Gyro sensor implementation incorrect?

[dsharlet]

According to the documentation at http://www.ev3dev.org/docs/sensors/lego-ev3-gyro-sensor/, the GYRO-G&A mode should have the angle in value0, and rate in value1. However, the ev3dev.h implementation is:

// The number of degrees that the sensor has been rotated
// since it was put into this mode.
int angle() {
  set_mode(mode_gyro_ang);
  return value(0);
}

// The rate at which the sensor is rotating, in degrees/second.
int rate() {
  set_mode(mode_gyro_rate);
  return value(0);
}

(from https://github.com/ddemidov/ev3dev-lang-cpp/blob/master/ev3dev.h#L473)

Isn't this going to make it impossible to use the GYRO-G&A mode? Any attempt to call angle() and rate() will change the mode away from G&A mode. Aside from that, calling angle() and rate() repeatedly and alternating will cause the gyro to reset and recalibrate, correct?

It seems like a workaround is to use value(0) and value(1) directly... but it seems like it should be possible to use the more descriptive functions in all modes. At best (if working as intended), the API is misleading.

[WasabiFan]

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?

[dlech]

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.

[dsharlet]

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...

[ddemidov]

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?

[dlech]

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?

[ddemidov]

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

[ddemidov]

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?

[dlech]

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.

[ddemidov]

@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.

[WasabiFan]

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.

[ddemidov]

I think there are two issues here:

1. G&A mode is not accessible. Convert types and indices in special sensor specification to lists ev3dev/ev3dev-lang#169 should solve that.
2. When mode is set too often, there is a std::system_error: Connection timed out exception. Make call to set_mode optional in special sensor getters #12 is for this case.