Skip to content
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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

IAssemble
Copy link
Contributor

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.

@dlech
Copy link
Member

dlech commented Jun 27, 2014

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.

@IAssemble
Copy link
Contributor Author

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.

@dlech
Copy link
Member

dlech commented Jun 27, 2014

If you are in mode 4, isn't InLength always 8? InLength is read from the sensor when getting the sensor info, but after it switches to data mode, it does not change. The CRC hack/check is under case UART_DATA:, so I think it is save to assume that we don't need to check InLength.

@IAssemble
Copy link
Contributor Author

That sounds reasonable. Any idea why their code gets the mode from the .Cmd rather than directly from .Mode?

@dlech
Copy link
Member

dlech commented Jun 27, 2014

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.

@IAssemble
Copy link
Contributor Author

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?

@dlech
Copy link
Member

dlech commented Jun 27, 2014

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.

@IAssemble
Copy link
Contributor Author

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! ;))

@dlech
Copy link
Member

dlech commented Jun 27, 2014

Would it be safer to only ignore the CRC error if both .Mode == 4 and the mode from .CMD is 4?

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.

(I did not expect such a small workaround would provoke such an interesting discussion! ;))

😁

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants