Can bus fifo selection #5035
Can bus fifo selection #5035
Conversation
|
build needs to pass |
|
Yeah sorry, I created an issue with more information. Probably shouldn't have bothered with that and just put it all in the PR description. I was looking for some feedback on the idea before I nailed down all the tests and examples. |
|
It seems reasonable to me, so I will probably merge it if you fix the build. The only possible is concern is that some users may want to read from any fifo--the old behavior? |
|
Personally I'm not a fan of this change as is. Sure it is possible to starve fifo1, but in that situation, things are kind of broken anyway. You will get overruns. This change is also quite unfriendly for many users who just want the next message without having to worry about the two FIFO's. Many users will just try to set up filters to split traffic between two to make the best use of the FIFO memory. Also, it becomes a breaking change. If this change is really required, I'd prefer to not change the existing functions. We could add a family of *_fifo or *_from_fifo variants. In this case, one approach could be to add 'Any' to the enum and then the 'old' functions just wrap the new. |
|
@cschuhen that seems reasonable to me; making it a breaking change is definitely not required. I still think it's useful to have access to the individual FIFOs so I'll probably do as you suggested and wrap the original commands while exposing a more specific method in case the user desires finer control. I haven't had time to work on this recently so I'll finish it up this upcoming weekend. |
3 participants
Closes #5034