ev3dev / ev3dev-lang

(deprecated) language bindings for ev3dev sensors, motors, LEDs, etc.
GNU General Public License v2.0
56 stars 39 forks source link

Add sensor-specific helper properties (such as "isPressed") to classes that wrap a single sensor type? #126

Closed WasabiFan closed 8 years ago

WasabiFan commented 8 years ago

Currently, it looks like we don't have any special properties or methods on any of the sensor classes (such as the TouchSensor) to make it more intuitive to use. How 'bout an isPressed property that returns the value of value0 as a Boolean?

ddemidov commented 8 years ago

That sounds like a good idea. We could also provide some syntactic sugar to other sensors, e.g. distance or angle for infrared sensor, or rgb for color sensor in raw mode.

WasabiFan commented 8 years ago

We could also provide some syntactic sugar to other sensors, e.g. distance or angle for infrared sensor, or rgb for color sensor in raw mode.

How do you think we should handle the mode? Set the mode before reading the value every time, or check and throw an error if it isn't in the right mode? Both options will introduce some level of perf hit.

ddemidov commented 8 years ago

That's what stopped me from suggesting this :). We could also assume the user sets the mode himself before calling the methods (and document this carefully), but I am not sure this is the right way.

WasabiFan commented 8 years ago

I think that the main function of helper classes like this is to make it easier for the majority of people who are just looking for a simple robot interface for their code. From that lens, I'd say that reading from an extra property to confirm that it's in the right mode (or change the mode) is O.K., given that you can still access the value attributes manually if performance is important.

@dlech how much of a delay should we expect between when we set a new mode and when we can use new values? If we set a mode and immediately read from a value are we assured to get the value for the new mode? And if we set the mode every time before we read a property, is that slower than reading the mode and only setting it if needed?

dlech commented 8 years ago

Setting the mode depends on the sensor. With analog sensors, it will happen almost instantly since it is just changing a few pointers in memory. With I2C and UART sensors, it actually has to communicate with the sensor, so it takes longer. I haven't measured the time though. Writing the mode attribute blocks until the mode has actually changed, so as soon as the write function returns, it is safe to read the data.

Setting the mode every time you read a value is standard operating procedure for EV3-G and OpenRoberta and not a bad idea unless you want a really tight loop (~10ms). Just reading the mode will be generally faster since it doesn't actually have to poll the sensor but rather returns the last mode from memory.


I would shoot higher here. Why just add an isPressed property for touch sensors when you could autogen a device-specific class for each sensor from sensors.json?

WasabiFan commented 8 years ago

Good to know! So it looks like it should be reasonably easy to get this working so that it sets the mode and we can be sure that the values that we read are correct.

I would shoot higher here. Why just add an isPressed property for touch sensors when you could autogen a device-specific class for each sensor from sensors.json?

That's my hope, although I'd like to get a single sensor class working before extending it to others. I think that using these specialty classes is much easier to use for most people than having to read from the value attributes manually, so this is definitely important in my eyes.

I think after I get the touch sensor figured out and fix LEDs in my binding I'll tag a release there (as well as on the main repo) to get a recent, stable version available.

WasabiFan commented 8 years ago

Just to note in this issue to keep track, PR #121 adds the autogen info discussed here so that these classes are generated by our scripts and include the helper properties.