-
Notifications
You must be signed in to change notification settings - Fork 178
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
Workaround to support LEGO EV3 Color Sensor in RGB-RAW mode #30
base: master
Are you sure you want to change the base?
Conversation
It looks like the fix from LEGO is here. I would say it is ever so slightly better because they use the TYPE_COLOR constant instead of hard coding 29. |
Thanks for linking to the ev3sources-xtended code. It looks like the code is functionally different too as it does not check the length of the data. My workaround was intended only to affect the specific cases observed to be incorrect. Perhaps the length check should not be included. |
If you are in mode 4, isn't |
That sounds reasonable. Any idea why their code gets the mode from the .Cmd rather than directly from .Mode? |
The mode is sent from the sensor with every DATA packet. I think .Cmd contains the mode from the data packet whereas .Mode gets its value from the sensor INFO. So, it does make sense to read the mode from .Cmd instead of .Mode. |
If I understand correctly, .Cmd is data received from the sensor. In this case it is subject to corruption during transmission (which is why the CRC is included in the protocol). What if the color sensor were being used in a mode other than 4 where the CRC is generated correctly but the .Cmd value happened to be corrupted to read as 4 instead of the actualy mode? The LEGO code would cause the CRC to be ignored even though the sensor was not actually being used in mode 4 and an error that could have been caught by the CRC would be missed. I would argue that only ignoring the CRC when .Mode is 4 is safer than using .Cmd. Thoughts? |
That is a good point (even though the likelyhood of it happening is very small). Looking at the code, it looks like it will see that .Mode != GET_MODE(....Cmd) and try to change back into the mode specified by .Mode. So, I don't think anything bad would happen in this scenario. If you look at .Mode instead of .Cmd, when you change sensor modes (via a command from the VM), there is the possibility of getting one or two more DATA packets before the mode in the sensor actually changes. So, if you change change from Mode 4 to another mode, if you are looking at .Mode to ignore the CRC and get one of these straggler packets from mode 4, chances are it will have a bad checksum. But since you can have up to 6 bad DATA packets before actually failing, the errors are ignored and this scenario works as well. |
I hadn't realized that there could be more DATA packets received from the previous mode after a mode change. Would it be safer to only ignore the CRC error if both .Mode == 4 and the mode from .CMD is 4? (I did not expect such a small workaround would provoke such an interesting discussion! ;)) |
Yes, I think this would be better when changing from another mode into mode 4. Changing from mode 4 to another mode, it would would be the same as just testing .Mode == 4.
😁 |
This was the workaround used for MindCub3r since the LEGO firmware v1.03H did not work. It appears that the issue has been resolved in LEGO firmware v1.06H so if someone has access to the source for that it may be worth checking whether there is a better fix than this.