Skip to content
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.

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

CAN: Initialisation Timeout #47

Merged
merged 5 commits into from
Sep 19, 2024
Merged

Conversation

garethpotter
Copy link
Collaborator

Add an (optional) timeout on CAN initialisation.

include/thingset/can.h Outdated Show resolved Hide resolved
src/can.c Outdated Show resolved Hide resolved
@garethpotter
Copy link
Collaborator Author

garethpotter commented Aug 11, 2024

I've now finally had another crack at this, as you will see. I noticed as I was adding a new event that the existing ALREADY_USED event had a value of 0x03; as these are flags, it should surely have been BIT(3), so I have amended accordingly.

To preserve what looked like the existing behaviour (and what makes sense to me reading it), I now explicitly clear all three of the events, two of which were being implicitly cleared by the call to k_event_clear at line 663. I also pass the thingset_can_addr_claim_tx_cb callback to can_send, so that that callback can set the MSG_SENT event which is awaited at line 702. I think this all looks correct to me now but I welcome your thoughts.

@garethpotter garethpotter force-pushed the init-timeout branch 2 times, most recently from 2f3df3f to 68d36f5 Compare August 11, 2024 21:15
CAN: correct callback

CAN: move timer to context
Copy link
Contributor

@martinjaeger martinjaeger left a comment

Choose a reason for hiding this comment

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

Looks good, thanks for the update. Just got a very minor suggestion.

src/can.c Outdated Show resolved Hide resolved
src/can.c Outdated Show resolved Hide resolved
Copy link
Contributor

@martinjaeger martinjaeger left a comment

Choose a reason for hiding this comment

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

Looks good, thanks for the update.

@martinjaeger martinjaeger merged commit 425cfce into ThingSet:main Sep 19, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants