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

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

Closed
WasabiFan opened this issue Nov 25, 2015 · 7 comments

Comments

@WasabiFan
Copy link
Member

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
Copy link
Member

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
Copy link
Member Author

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
Copy link
Member

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
Copy link
Member Author

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
Copy link
Member

dlech commented Nov 25, 2015

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
Copy link
Member Author

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 WasabiFan changed the title Add "isPressed" to touch sensor? Add sensor-specific helper properties (such as "isPressed") to classes that wrap a single sensor type? Nov 26, 2015
@WasabiFan
Copy link
Member Author

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.

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

No branches or pull requests

3 participants