-
Notifications
You must be signed in to change notification settings - Fork 39
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
Comments
That sounds like a good idea. We could also provide some syntactic sugar to |
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. |
That's what stopped me from suggesting this :). We could also assume the |
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? |
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? |
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.
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. |
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. |
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 anisPressed
property that returns the value ofvalue0
as a Boolean?The text was updated successfully, but these errors were encountered: