Check requested message before trying callbacks#14
Merged
samuelsadok merged 1 commit intoodriverobotics:masterfrom Apr 24, 2025
Merged
Conversation
|
|
samuelsadok
reviewed
Apr 24, 2025
Member
samuelsadok
left a comment
samuelsadok
left a comment
There was a problem hiding this comment.
Intent and implementation looks good to me.
Technically it could be a breaking change for someone but the previous behavior seems a bit quirky to rely on and we can bump the library version to 0.11.
Do you mind doing a rebase & force-push to trigger the new CI test? (see comment on #12)
BrechtSerckx
force-pushed
the
requested-cyclic-messages
branch
from
7c4f624 to
c88f8a4
Compare
Contributor
Author
I've rebased on master and force-pushed, but nothing seems to have been triggered. Could it be that the workflows need approval? |
samuelsadok
approved these changes
Apr 24, 2025
Member
samuelsadok
left a comment
samuelsadok
left a comment
There was a problem hiding this comment.
Thanks! Yes the workflows need approval to run.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Hi,
I encountered this while working on #13.
Here is an example program to reproduce it: https://gist.github.com/BrechtSerckx/e682a30fb946292be58ab2be64ed49e3.
When no feedback handler is registered, calls to
getFeedbackfail. When a feedback handler is registered, calls still fail, but the callback is called.getFeedbackshould not conflict with registered callbacks.This patch first checks if a CAN message has been requested. Only if it wasn't, the callback handlers are checked.