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

pybricks.ev3devices.ColorSensor: Use blocking read calls #273

Merged
merged 1 commit into from
Dec 5, 2024

Conversation

JakubVanek
Copy link
Contributor

Other EV3 sensors in this module use the pb_type_device_get_data_blocking() method. I think that using pb_type_device_get_data() as it is used (only) in the color sensor class the sensor will not work for reading non-default modes.

I have found this by accident - I wanted to know if Pybricks waits for a sensor to change its mode. I think this is what happens for most EV3 sensors and it is only disabled for the EV3 color sensor.

However, I unfortunately have no way to test this and also I'm not entirely sure the bug is there.

Other EV3 sensors in this module use the pb_type_device_get_data_blocking()
method. I think that using pb_type_device_get_data() as it is used (only)
in the color sensor class the sensor will not work for reading non-default
modes.

Fixes: 8a52c95 ("pbdrv/legodev: Refactor LEGO device abstraction.")
@coveralls
Copy link

Coverage Status

coverage: 56.81%. remained the same
when pulling 1bccc1a on JakubVanek:bugfix/ev3-color-sensor
into 4d457a1 on pybricks:master.

@laurensvalk
Copy link
Member

laurensvalk commented Nov 10, 2024

Thank you. The EV3 build hasn't been tested much lately, and won't be continued in its current form. The pybricks.ev3devices has not been updated to be async compatible.

We're currently working on a bare metal version of Pybricks for EV3. It will probably get a rewrite of pybricks.ev3devices to be a bit more like pybricks.pupdevices and be async compatible.

It will use the same legodev_pup_uart.c driver for the EV3 UART sensors (which is why that driver needs to be made compatible with EV3 sensors at some point).

@JakubVanek
Copy link
Contributor Author

Oh, I see. In that case, feel free to close this PR.

@laurensvalk
Copy link
Member

I think we can still merge it to make sure the legacy EV3 version is still complete. Thanks!

@laurensvalk laurensvalk merged commit 1d1d850 into pybricks:master Dec 5, 2024
20 checks passed
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.

3 participants