Hotfix: Flags were not being propery set on transmitted messages. #31
Conversation
|
How has this been tested? Are there test cases to cover the changes? |
|
@jurlaubAStuff - I tested it on my local machine using |
|
@jurlaubAStuff - So, what I'm realizing is that there is no way to test these scenarios on CI because of Docker. Docker uses the host's kernel. Because of this, you can't install a kernel module in Docker without some very specific options passed to it when running the Docker container (which we don't have access to pass). I'm going to have to find a way to validate the API without hardware and without virtual channels - unless we stick to running tests only on local machines, which seems to defeat the point. Thoughts? |
|
@jurlaubAStuff - Nevermind. I think I found a way. Stay tuned. |
JWhitleyWork
force-pushed
the
fix/extended_id_transmission
branch
from
6b949a6 to
495e847
Compare
|
See #33 for tests. In that PR, the tests for extended CAN IDs are disabled due to the bug that this PR fixes. I merged that branch into this one and re-enabled the tests to verify that the bug is fixed. |
|
Actually, #33 is going to take longer than expected as we are going to write and attempt to implement some more detailed test diagnostics. I have verified that this works on Extended CAN IDs and it is passing CI. Can @zoakesAStuff and @jurlaubAStuff please approve if you think it looks alright at this stage so I can get a new release out and fix the bug? |
|
@JWhitleyAStuff This looks okay, but do we know for sure that it fixes 30? |
|
We will now (just merged CI tests from |
|
@zoakesAStuff / @jurlaubAStuff - Please review/approve #37 prior to this PR. That will keep the conflicts to a minimum. Sorry for the confusion. |
|
Approved previously by @zoakesAStuff - merging. |
Fixes #30.
I was converting the flags returned int in the read() function into a CanMsg but was not setting the flags passed to the write() function from a CanMsg.