ev3dev-lang-cpp icon indicating copy to clipboard operation
ev3dev-lang-cpp copied to clipboard

Fix gyro_sensor on BrickPi

Open alexdewar opened this issue 6 years ago • 9 comments

On the BrickPi, we have to explicitly set the sensor type as it cannot be automatically detected, as it is with ev3dev running on the Mindstorms computer.

This PR adds an extra constructor to the sensor class, which allows for explicitly setting the sensor type, and makes use of it in the gyro_sensor class.

alexdewar avatar Jun 04 '19 17:06 alexdewar

Is gyro sensor the only sensor that is affected by this on the brickpi?

Also, I wonder if introducing a sensor constructor that takes lego_port instead of address_type would work better. That is, something like this would work:

gyro_sensor s(lego_port(address).set_mode(mode).set_set_device(device));

lego_port is constructible from address_type, so sensor s(address) should still work, but this could also be another constructor.

ddemidov avatar Jun 04 '19 17:06 ddemidov

No, I believe that sensors on the BrickPi all need to be explicitly defined. The reason that I didn't add this feature to the other sensors is because I don't have the other sensors to test this with (and while the gyro sensor uses UART, others use i2c etc. and I'm not confident I'd set the mode correctly).

I feel like the extra business of setting the mode and device type should be hidden from the user in the constructor somewhere, just because these parameters will always be the same for this sensor and it's a detail that users shouldn't have to worry about.

alexdewar avatar Jun 04 '19 20:06 alexdewar

I feel like the extra business of setting the mode and device type should be hidden from the user in the constructor somewhere, just because these parameters will always be the same for this sensor and it's a detail that users shouldn't have to worry about.

Ok, that makes sense. Could you put a comment for the new constructor so that people know what should it be used for (and fix other sensors as needed)?

About guarding new functionality with EV3DEV_PLATFORM_BRICKPI, I don't have a strong opinion. It does introduce new (unnecessary and strictly speaking expensive time-wise) operations on ev3dev, but those are only done in the constructor, which should be rare enough.

ddemidov avatar Jun 05 '19 04:06 ddemidov

I'll stick a comment in.

The thing is that I don't know what mode other sensors need to be put into. They're not all UART-based. If you happen to have other sensors and an EV3 we can figure it out though (just plug in the sensor and cat the mode sysfs file).

alexdewar avatar Jun 05 '19 10:06 alexdewar

All I have is a standard home edition mindstorms set 31313 with its sensors. But I haven't touched it for some time, so I guess I would need to update the ev3dev system first. We could also ask @dlech if he has any pointers.

ddemidov avatar Jun 05 '19 10:06 ddemidov

Have a look at http://docs.ev3dev.org/projects/lego-linux-drivers/en/ev3dev-stretch/sensor_data.html. It describes which communication type each sensor uses.

A couple of things to watch out for:

  • For I2C sensors, you also have to include the I2C address when writing to set_sensor.
  • Usually a short delay is need after writing to set_sensor before the device appears in /sys/class/lego-sensor.

dlech avatar Jun 05 '19 14:06 dlech

I'm happy to have a crack at this, though as I said, I don't have most of the other sensors so won't be able to test it.

How can I figure out what the I2C address will be? How long do you need to wait for after writing to set_sensor? Should we add this delay to the constructor or let the user add the delay?

alexdewar avatar Jun 05 '19 15:06 alexdewar

The default I2C address is also listed in the link I gave.

The most efficient logic (instead of a fixed time delay) is probably something like this:

  • Try to create the sensor object (the port might be configured already from a previous program)
  • If it fails:
    • configure the port and write to set_sensor
    • Loop until sensor exists in /sys/class/lego-sensor or a timeout is exceeded
    • If timeout was exceeded, stop the program
  • continue with the rest of the program

dlech avatar Jun 05 '19 16:06 dlech

That does sound like a better strategy. I'll have a crack at implementing this.

alexdewar avatar Jun 06 '19 09:06 alexdewar