Gyro sensor implementation incorrect? about ev3dev-lang-cpp HOT 16 CLOSED

The angle and rate getters set the mode to the one that is specific to that property, so yes, you can't use them if you want to be in GYRO-G&A mode. For that you'd need to set the mode yourself and then read from the two value attributes.

@dlech Is there any detriment to performance or angle data if you are in the G&A mode instead of the property-specific one? i.e. would it be bad if both of those getters used the G&A mode and read their respective values?

from ev3dev-lang-cpp.

Dunno, you would have to do an experiment to find out. Whether it is bad or not depends on if you care that the getters also set the corresponding mode.

from ev3dev-lang-cpp.

Coincidentally, I think that setting the mode on each call to angle or rate is triggering a bug.

I'm currently not attempting to use the G&A mode, just RATE mode. I have a robot that is trying to read the rate of two gyros in a tight control loop (100 Hz).

When I use the rate method, between ~100 and ~1000 iterations of the control loop, I get a std::system_error: Connection timed out exception. If I use the value(0) method, I seem to be able to run my robot indefinitely (no exceptions). The only difference is the call to set_mode...

from ev3dev-lang-cpp.

Sorry for being silent on this, was away on vacation.

When I use the rate method, between ~100 and ~1000 iterations of the control loop, I get a std::system_error: Connection timed out exception.

A possible workaround for this would be to cache the current mode in a private class field and only set the mode when necessary. This could fail if mode is changed outside of the current process or from a different class instance referencing the same sensor. I think both of these should be extremely rare, and the latter is bad practice anyway.

We could also read the current value of mode just before reading the value and update the mode if necessary. That could be even faster than setting the mode every time and may be less error-prone.

@dsharlet, @dlech, what do you think?

from ev3dev-lang-cpp.

It is rare, but in some cases, you actually do have to set the mode to get an updated value, for example the US-SI-CM of the LEGO EV3 Ultrasonic sensor. So, I think always setting the mode before reading the value is the correct thing to do for a general use case.

However, having an alternate way to read the value without setting the mode for advanced users that want to read the value as fast as possible makes sense too.

For this particular issue though, I think the question is there are rate() and angle() functions, but why is there not a rate_and_angle() function that corresponds to the GYRO-G&A mode of the LEGO EV3 Gyro sensor?

from ev3dev-lang-cpp.

but why is there not a rate_and_angle() function that corresponds to the GYRO-G&A mode of the LEGO EV3 Gyro sensor?

Good question and good idea

from ev3dev-lang-cpp.

Good question and good idea

After some thought I don't see a clear way to implement this. The problem is, we only have types for int, string, and set of strings so far in the specification. And in order to define the GYRO-G&A mode we need a tuple (or its equivalent in a specific language). I see three options for doing this:

1. Only change the definition of the gyroSensor, so that its sensorValueMapping for the GYRO_G&A mode has list of integers for sourceValue, and list of types for type. Something like this:

                 {
                     "name": "Rate and Angle",
                     "sourceValue": [0, 1],
                     "type": ["int", "int"],
                     "requiredMode": "GYRO-R&A",
                 },
   

   The problem here is I don't see a way in liquid template syntax to test if a value is a scalar or a list. (@WasabiFan, do you?) So there is no way to process this specification.

2. Change all the definitions in specialSensorTypes to have lists for both sourceValue and type. Then, if the list is of length one, do the current thing and return a scalar, and otherwise construct and return a tuple. This I think is doable but requires too much change for a single use-case (so far?).

3. I could rewrite liquid template for special classes to leave the class declaration unclosed (omit the closing brace) and add the implementation of gyro_rate_and_angle() method manually. This will localize the change to this repo, but the source file will look slightly uglier.

@WasabiFan, @dlech, what do you think?

from ev3dev-lang-cpp.

I think it would make sense to be able to handle sensors with multiple values in a general case. There are a number of sensors that can return multiple values. So, make all sensors have "sourceValue" array. Most will just have length of 1, but that is OK.

from ev3dev-lang-cpp.

@dsharlet, what do you think of ev3dev/ev3dev-lang#169 and #11? It looks like this is not going to help much for this specific issue, especially since you are just using RATE mode. What about the following idea: add an optional parameter set_mode=true to the generated special sensor helper methods. So that

int r = s.rate();

will set the required mode and return the value, while

int r = s.rate(false);

would return the value immediately, without touching the mode. This way the user would be responsible for setting the correct mode, and the existing code would not need to be changed.

from ev3dev-lang-cpp.

That seems like a good idea, but then we need to make sure that it's clear what mode must be used. In this case, we would still need to change the getters to use the G&A mode to make this work, I think.

from ev3dev-lang-cpp.

I think there are two issues here:

1. G&A mode is not accessible. ev3dev/ev3dev-lang#169 should solve that.
2. When mode is set too often, there is a std::system_error: Connection timed out exception. #12 is for this case.

from ev3dev-lang-cpp.

There are two issues, but they're closely related; each requires the other to be fixed in order to be of use AFAIK. If the getters here still use their individual modes, you won't be able to get both rate and angle in a loop, regardless of whether or not you are telling it to set the mode: each call would require that you switched it to make it function. Am I misunderstanding?

from ev3dev-lang-cpp.

If you need both rate and angle, you would use the rate_and_angle() function. This does not require changing modes between calls:

int rate, angle;
std::tie(rate, angle) = s.rate_and_angle();

this would also work with s.rate_and_angle(false) given that you called this function at least once with set_mode=true.

from ev3dev-lang-cpp.

Sorry for the delayed reply, I was missing the notifications for updates to this issue.

I think the mentioned PRs are an improvement, and they definitely solve both problems I had. Thanks for addressing them!

from ev3dev-lang-cpp.

Merged #12, waiting for @WasabiFan's approval on ev3dev/ev3dev-lang#169 to merge #11.

from ev3dev-lang-cpp.

Resolved by #11 and #12

from ev3dev-lang-cpp.