Skip to content

CANModule to fit CAN into app restructure #407

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
upadhyaydhruv wants to merge 12 commits into
base: master
Choose a base branch
from
Draft

CANModule to fit CAN into app restructure #407

Changes from 1 commit
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
b489be4
Create modules skeleton for new app structure (#382)
cindyli-13 Jun 22, 2021
20010f1
Create watchdog module (#389)
jahnavithota2011 Aug 18, 2021
1a488c7
Create PDB monitoring module (#392)
jahnavithota2011 Aug 26, 2021
4de2cff
fix folder structure
cindyli-13 Oct 17, 2021
3f052b1
fix common modules not found
cindyli-13 Oct 17, 2021
d1c8689
created CANModule and fixed txProcessor function
upadhyaydhruv Jan 15, 2022
5de8de9
fixed rxClient function for modular use
upadhyaydhruv Jan 15, 2022
2c23625
made CMake linking changes for CANModule
upadhyaydhruv Jan 15, 2022
541583d
implemented most changes as requested by Melvion and Cindy
upadhyaydhruv Feb 3, 2022
6947dd6
added all changes requested, hw bridge issue persists
upadhyaydhruv Feb 4, 2022
a667ffa
fixed issues preventing compilation
upadhyaydhruv Feb 10, 2022
a0d658b
added wrapper for other CANInterface functions
upadhyaydhruv Feb 14, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
fixed rxClient function for modular use
  • Loading branch information
@upadhyaydhruv
upadhyaydhruv committed Feb 4, 2022
commit 5de8de916d48e52de07aa3ed81a63be17285984a
8 changes: 2 additions & 6 deletions libs/can/src/CANInterface.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,12 +55,8 @@ void CANInterface::rxPostman(void) {
void CANInterface::rxClient(void) {
while (true) {
Copy link
Member

@wmmc88 wmmc88 Jan 16, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should no longer be while true if its a 1ms periodic function

CANMsg *mail = nullptr;

// Wait for a message to arrive
do {
mail = m_rxMailbox.try_get(); // using try_get() because try_get_for() was crashing
ThisThread::sleep_for(1ms); // TODO: Check for object in mailbox every ms
} while (mail == nullptr);
// Check if a message has arrived:
mail = m_rxMailbox.try_get();

MBED_ASSERT(mail != nullptr);
Copy link
Member

@wmmc88 wmmc88 Jan 16, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with the above logic change on lines 58 and 59, but now it results in try_get sometimes returning nullptr if the mailbox is empty. should change this assert to maybe a debug print and early return(or just an if block to block execution of rest of this function. personally, i prefer single return statements from functions in fw code)

Copy link
Contributor Author

@upadhyaydhruv upadhyaydhruv Feb 3, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would the MBED_ASSERT not account for that?

Copy link
Member

@cindyli-13 cindyli-13 Feb 3, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MBED_ASSERT would actually break the program, I agree that we should remove it and just wrap the logic in an if block


Expand Down