-
Notifications
You must be signed in to change notification settings - Fork 22
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
void CanHardware::HandleRx - more then one use of ID causes issues #20
Comments
Just to add meat to the bones, the issue is that if you add an RX map for an ID and the code also uses that ID, the handlers down the list will never receive it. |
That is intended but I see the problem. You could register the other handlers before CanMap and return false in them |
That is an odd design choice IMO. It runs counter to the publish-subscribe pattern of CAN messaging. If a CanHardware user has expressed an interest in a particular ID shouldn't it just get an opportunity to process it. Why does it work that way? |
All handlers are called on all ids.That is way easier to implement rather than remembering which handler is interested in which ID. So I thought this would prevent unnecessary handler calls. But I see the downsides are more severe. Feel free to remove it (and the return value of HandleRx) |
CanCallback::HandleRx no longer returns a bool indicating whether the message has been handled. The message is passed to all handlers allowing user defined handlers to see messages that have been handled by CanMap. Tests: - Build with stm32-sine for SINE and FOC configurations - Basic verification of CAN SDO and CAN RX map functionality
I've raised a PR that addresses your problem @Tom-evnut and @jamiejones85. Any chance you could test with the problem you were seeing? I don't have VCU hardware and environment to test currently. Thanks. |
When the main code and the CAN map both share ids it wont be sent to both.
Fix:
Verified working on STM32_VCU Zombie. This allows populating of info like HV bus measurement from devices already used.
The text was updated successfully, but these errors were encountered: