Add rover message to common.xml

[LorenzMeier]

The WHEEL_DISTANCE is the first of a set of rover messages intended to allow easier integration of ground vehicles.

[pavloblindnology]

I think this part of the description of distance field "The first wheel is the front left, 2nd is the front right, and then row-by-row." can confuse. Because it depends on wiring and cannot be detected by software, but people can think that it's true no matter which wheel is connected to which input of FCU.
Concerning 100,000m limit - it's not implemented in Ardupilot.

[hamishwillee]

Remove local thousand's markers: e.g. 100'000m becomes 100000m. Everyone can infer the range without the markers, but to a non-native they look horrible

[LorenzMeier]

@pavloblindnology How is the wheel mapping then done? The reason I ask is that setting this will avoid weird errors for users of the message in the future.

[hamishwillee]

99'999 to 99999

[hamishwillee]

@pavloblindnology @LorenzMeier Outside of this immediate discussion, can we have a call sometime to discuss more general ideas around Rover and MAVLink. For example, frame types, providing great integration with other robotics libraries (e.g. ROS), etc?
I don't want to drive the discussion, because I'm not fully aware of what rover needs, so would be good to get more input.

This could be done in a dev call, or probably even better in a specific meeting.

FYI there is an RFC in progress that may be relevant https://github.com/mavlink/rfcs/blob/2b5bc9dbea620fc15e8e3a241168850297b3d8ff/text/0002-local-frames-revision.md

[LorenzMeier]

@pavloblindnology I forgot to answer on the wrap-around of the number: It seems a prudent thing to do to not run into this later on. You're not going to run into this any time soon, but at least the spec lays out clear suggestions how things should be done.

[hamishwillee]

@LorenzMeier
Where using the words "should" and "must" is better for defining a standard.

Perhaps:

Distance traveled by each wheel. The distance must wrap around after 100000m ...

[pavloblindnology]

@pavloblindnology How is the wheel mapping then done? The reason I ask is that setting this will avoid weird errors for users of the message in the future.

Mapping is done experimentally by rotating each wheel and seeing which distance array value is changing and then by specifying displacement for each wheel in corresponding mavros plugin.

[hamishwillee]

@LorenzMeier Any comments in response to #1032 (comment) ?

Based on that post and this thread, IMO:

• we (you) should update to doubles (or whatever). @pavloblindnology is OK with this, and in any case will be no worse off with this (and other systems need not have his system's limitation so this will be useful).

• @pavloblindnology Re

  "The first wheel is the front left, 2nd is the front right, and then row-by-row."

  • Just to be clear, can you confirm that the reason this does not work is that the mapping of encoders to vehicle wheels is not necessarily 1:1 or predictable. So you might have a 4 wheel vehicle, but only report 1 or 2 wheels (and which wheels is up to you)?
  • If so, I agree, we definitely cannot do it this way. To do this we'd need to know how many wheels are present, which ones have assigned encoders and the geometry (which we might have from the vehicle type). Possible, but a lot more data to send. Doubt it is worth it.

[pavloblindnology]

@hamishwillee

• Just to be clear, can you confirm that the reason this does not work is that the mapping of encoders to vehicle wheels is not necessarily 1:1 or predictable. So you might have a 4 wheel vehicle, but only report 1 or 2 wheels (and which wheels is up to you)?
• If so, I agree, we definitely cannot do it this way. To do this we'd need to know how many wheels are present, which ones have assigned encoders and the geometry (which we might have from the vehicle type). Possible, but a lot more data to send. Doubt it is worth it.

Yes, absolutely. It's up to user to decide how many, at which wheels and in what order to install encoders. E.g., Ardupilot Rover firmware uses up to two encoders (link) with the next prescription in case of two-wheeled config:
Connect motor encoder’s A and B outputs to the flight controller (i.e. Pixhawk’s) AUX OUT 3,4,5 and 6 pins. Normally 3,4 should be used for the left motor’s encoder, 5,6 for the right’s.
This means that the left wheel is reported through the 1st value of distance array and the right through the 2nd. But it's up to user to guarantee this. And user is free to change the config on his own in case he uses his own firmware or uses the wheel distances in mavros plugin, etc.

[hamishwillee]

@LorenzMeier in the source ArduPilot issue I suggested that we might change name="time_usec" to name="time" (or name="timestamp").
@amilcarlucas Suggested here ArduPilot#55 (comment) that it should be kept the same for consistency and code simplicity reasons.

Don't know if you have an opinion.

[hamishwillee]

forgot to answer on the wrap-around of the number: It seems a prudent thing to do to not run into this later on. You're not going to run into this any time soon, but at least the spec lays out clear suggestions how things should be done.

Had a chat to @auturgy about this. Wrapping provides clear guidance to what the autopilot should do, but this doesn't help the consumer of the message if this is exceeded. IF it is likely that 100km could be exceeded in future and there are alternatives, we should not constrain to this.
What sort of precision do we need? Perhaps use something other than a float?

[pavloblindnology]

Well, with my current configuration, wrapping of int32 encoder tick counter in Ardupilot leads to a distance limit of ~75km which is even less than 100km, while 1 tick measures ~3e-5m (= 75km / 2147483647) - accuracy not achievable by float, though I'm not sure it's needed.
So, switching to double would definitely help to improve accuracy, but bandwidth would suffer.
And in any case 100km seems not achievable on some configurations because of shorter range of int32 encoder tick counter.

[LorenzMeier]

Given this message gets probably consumed by an onboard computer, is bandwidth a real concern here?

I can tell from experience that the loss of precision with float will come back to haunt us as it’s going to affect absolute precision.

I’m ok changing the wheel assignment to a recommendation, but please be aware that it means you loose compatibility.

[pavloblindnology]

Given this message gets probably consumed by an onboard computer, is bandwidth a real concern here?
I can tell from experience that the loss of precision with float will come back to haunt us as it’s going to affect absolute precision.

Yes, message is consumed by on-board computer. If there is no tough limitations on bandwidth then i'm ok with changing floats to doubles. Though, again, with my current configuration int32 encoder tick counter is the limiter, and not the floats.

I’m ok changing the wheel assignment to a recommendation, but please be aware that it means you loose compatibility.

Well, some robots (like my current configuration for example) just don't have any encoders at front wheels, or have single one at some other wheel, so any assignment like The first wheel is the front left just doesn't make sense.

[hamishwillee]

@LorenzMeier Any comments in response to #1032 (comment) ?

[hamishwillee]

@LorenzMeier @pavloblindnology

1. I made the distance an array of doubles. Note that this will never be truncated because the count field is a smaller value type.
2. Updated text to:
   • remove the mapping between index and wheel position (does not make sense given arbitrary encoder:wheel mappings)
   • make it clear that not every wheel is reported
   • remove the text about wrapping around - I assume the limit would now be great enough that it can be ignored?

Will we ever need more than 16 wheels?
If so, one option is to get rid of the count field, encode the distances in an integer, and have 0 as the not-used value. This would allow the empty values to be truncated.
It is possible because both ends of the communication already have to know the encoder/wheel mapping (and hence how many encoders there are) for the message to be useful.

If not, does anything else need to be done?

[pavloblindnology]

@hamishwillee LGTM. Doubt I'll ever need more than 16 wheels. Usually they are used for odometry and 16 is more than enough to do it, even 2, 4 is enough for the most of applications.

[pavloblindnology]

@hamishwillee

• Just to be clear, can you confirm that the reason this does not work is that the mapping of encoders to vehicle wheels is not necessarily 1:1 or predictable. So you might have a 4 wheel vehicle, but only report 1 or 2 wheels (and which wheels is up to you)?
• If so, I agree, we definitely cannot do it this way. To do this we'd need to know how many wheels are present, which ones have assigned encoders and the geometry (which we might have from the vehicle type). Possible, but a lot more data to send. Doubt it is worth it.

Yes, absolutely. It's up to user to decide how many, at which wheels and in what order to install encoders. E.g., Ardupilot Rover firmware uses up to two encoders (link) with the next prescription in case of two-wheeled config:
Connect motor encoder’s A and B outputs to the flight controller (i.e. Pixhawk’s) AUX OUT 3,4,5 and 6 pins. Normally 3,4 should be used for the left motor’s encoder, 5,6 for the right’s.
This means that the left wheel is reported through the 1st value of distance array and the right through the 2nd. But it's up to user to guarantee this. And user is free to change the config on his own in case he uses his own firmware or uses the wheel distances in mavros plugin, etc.

[hamishwillee]

Thanks @pavloblindnology.

I think this is good to go. @LorenzMeier @auturgy - we've all be around the loop on this a few times. I believe it does what is expected. If there are no comments by Monday I plan to push this in.

[TSC21]

@pavloblindnology since this is merged can you please rebase and update the MAVROS pull request so we can bring it in? Thanks!

[hamishwillee]

I also somewhat feel we need a party to celebrate getting this in :-) Thanks @pavloblindnology for your patience.

[pavloblindnology]

@hamishwillee Thanks, but I'll consider the story ended when mavros and ardupilot parts are merged :)

[pavloblindnology]

@TSC21 @hamishwillee Mavros and Ardupilot part has been updated.