Deprecate and drop old MAVLink messages

[WickedShell]

Issue details

There are several old messages/interfaces that we had with MAVLink which have been eclipsed by later messages we are sending for a variety of reasons. I'm proposing to make a list of messages we want to deprecate that we will deprecate, and continue to send until we have both shipped at least one version of Copter/Plane/Rover after the deprecation announcement, and a year has gone by.

Proposed messages to be dropped:

1. BATTERY2 this is already marked as deprecated in the XML, and is thoroughly eclipsed by BATTERY_STATUS which we are already sending.
2. RANGEFINDER this message is sent in conjunction with DISTANCE_SENSOR which provides a lot more information (what sensor is reporting, the orientation, sensor type)
3. RAW_IMU this is identical to SCALED_IMU except it has a 64 bit timestamp. Moving to SCALED_IMU would allow a reduction in duplicate code on the aircraft side (see #8287) and provides GCS's with a more consistent message set. We do not currently send the proposed replacement.
4. RC_CHANNELS_SCALED We use this to send the scaled outputs, rather then the inputs which is what the message definition indicates it should be, it also does not follow the RCMAP parameters, and generally is malformed to the spec. The raw data is already transmitted in SERVO_OUTPUT_RAW
5. RC_CHANNELS_RAW this overlaps (and is inferior to) RC_CHANNELS. It's noted in the code as being sent for MAVLink1 compatibility to not break OSD's. RC_CHANNELS is a MAVLink 1 message as well, it was just added more recently. The OSD breakage case is more painful as fewer people work on the firmware for them, but I think it's worth looking at if we want to go down this road. (It's already only sent if the channel is MAVLink 1 which means it will unreliable to expect it if you shift to MAVLink2)

Proposed interfaces to be dropped:

1. Accelerometer calibration by status text messages. #5200 provided a set of commands to interact with the accel calibration routine, and allows for much more robust reporting of accel cal then the statustext/snooping version. The command version can handle a lossy data link, and removing the snooping code would allow us to clean up the accel calibrator, allows the GCS to send ACK's to the aircraft (currently this is assumed to always be an accel cal interaction) for future MAVLink use cases, and removes a very error prone process from the user experience. The command version has been shipping since Nov, 2016 so there is a lot of existing compatibility.

@DonLakeFlyer @meee1 @peterbarker are there any of proposed changes you object to, or additional messages you want us to consider deprecating/moving away from at this point to ease GCS/external application maintenance? (The rally points doing mission protocol will have to wait, unless someone has more time then me to do it at the moment)

[night-ghost]

new try to break everything around, as usual (GPS, OSD, 3DR RSSI)? ;)

[WickedShell]

The goal isn't to break everything (I don't know any GPS that this breaks), the OSD affecting one is only an idea, and one I can be convinced isn't worth pursuing. At some point OSD's do need to be maintained though rather then supporting legacy items for ever. I'm not sure how hard it is to update them though, that might make it a deal breaker for removing that message at the moment.

[night-ghost]

But it will, as it was many times. Because there are a lots of unmaintained code, that used in ecosystem. So if Ardupilot uses MAVlink1, it should be TRUE MAVlink1, as it was in time of development. Any changes should be in MAVlink2 only.

PS. Last years you broke NMEA GPS compatibility, OSD by removing RC_Channels (was restored after my bugreport) and 3DR RSSI translation from modem to OSD (still broken).

@Tridge says "we can't do anything which breaks compatibility because there are thousands of system thiat we should respect", and I can't understand how this changes can be.

[WickedShell]

Well first of all the alternate message is a true MAVLink 1 message, so why can't the OSD's handle it? (Answer their firmware is unmaintained).

Yeah, the GPS one I will cop to haven broken. I do strongly disagree with the characterizations that UBlox units cause flyaways, and that this is a commonly used GPS unit. I had missed the response from you and admit this slipped off my radar. #8364 is an attempt to resolve that as described in the open MTK issue.

At some point hardware needs to update and be maintained. I'm not urging us to break stuff over night, that is the point of having a process and timeline.

[auturgy]

@ShikOfTheRa and @night-ghost are maintainers for the two most popular (and actively maintained) forks of minimosd

[night-ghost]

besides that OSD projects, there are some another projects, eg. PlayUAV osd, which stoped development years ago - it becomes broken, so people which bought will got a garbage instead OSD.

Two ways: There was OpenPilot time ago, which forced users to change full ecosystem on version change because each version had own UAVtalk dialect. Where it now?

And there is CleanFlight which uses the same MWII as it was born many years ago in MultiWII.

Which way you like?

[peterbarker]

On Thu, 10 May 2018, night-ghost wrote:

There was OpenPilot time ago, which forced users to change full ecosystem on version change because each version had own UAVtalk dialect. Where it now?

Something not explicitly stated by @WickedShell, I think: we'd be phasing this in over a year or two, not up-and-breaking things on a particular day!

[night-ghost]

I do strongly disagree with the characterizations that UBlox units cause flyaways

I do not sell any GPS, so I'm not want to prove anything if the screencast is not enough. I will consider this as a matter of faith :)

[night-ghost]

in over year or two

The only difference is that the PlayUAV and the rest will turn into a pumpkin in a year, and not now.

Just imagine that GRE protocol will be announced as deprecated and will start to be dropped on all routers in a year 8-)

[night-ghost]

the only way to blood-less deprecate any protocol feature is a version negotiation between FC and device, and use of the highest version confirmed by device.This will allow to reduce channel throughput.

But there is no such mechanism in MAVlink AFAIK.

[WickedShell]

the only way to blood-less deprecate any protocol feature is a version negotiation between FC and device, and use of the highest version confirmed by device.This will allow to reduce channel throughput.

This requires you to carry forward a compat layer. This builds up into unmaintained cruft quickly, and at somepoint you will always turn off the oldest compat layer.

But there is no such mechanism in MAVlink AFAIK.

Message interval setting is the MAVLink mechanisim for this (where you just say "send me this ID at X rate" and if the FC can send it it does. Peter is doing some work on adding this, but it's not present yet. And even if it was, none of the older devices know how to do it anyways, so we still have to deprecate items at somepoint.

[night-ghost]

none of the older devices know how to do it anyways

so those devices should to use original MAVlink1 dialect, isn't it?

[WickedShell]

The messages and features exist in MAVLink1. Please realize that MAVLink is always changing. MAVLink 1/2 is primarily an encoding spec. The set of messages grows over time. There is no "orginal MAVLink 1 dialect"

[night-ghost]

The messages and features exist in MAVLink1. Please realize that MAVLink is always changing.

NO! After any change this is NOT a MAVlink1, but may be MAVlink1.01 and need to be negotiated.

And I now see that we have the opposite opinion about changes in the published interfaces, so I, for my part, end this useless discussion.

[peterbarker]

@night-ghost Have you done a git log on common.xml?! That's the PX4 team's dev playground!

[amilcarlucas]

@night-ghost Mavlink 1.0 is changing a lot, and doing it fast. And we (ardupilot) have our own fork where we strive to only merge "stable" changes that do not break things for our users.

That said, we can improve on that ... so what deprecated, duplicated message should we keep to preserve OSD compatibility ?

[DonLakeFlyer]

Ok, to remove from a QGC perspective. May require simple code changes in QGC: RC_CHANNELS_RAW, RC_CHANNELS_SCALED, BATTERY2.

RAW_IMU - Used when QGC falls back to old style compass cal on older firmwares (not sure how old version is that doesn't support onboard).

RANGEFINDER - ArduSub code in QGC uses this message.

Status text based accel cal: This is still what QGC uses. Will be work to cut over to new mechanism. Not a huge deal, but non-trivial. Need to fit that into my other work I have going now.

Then the last thing is that you'll need to work through with PX4 as well if you ever want to merge back. Keep in mind that QGC is built against the ArduPilot dialect which is from the main mavlink repo.

[WickedShell]

@DonLakeFlyer I'm not suggesting we delete the messages, I'm just indicating we should stop sending them :) EDIT: IE this isn't proposing change MAVLink specs at all, just what messages we send/handle.

RAW_IMU - The proposed change would mean that anyone who needs old style compass cal's would already have it, since onboard has shipped for quite awhile now and should be preferred.

The status text one is admittedly a small weight to carry forward on the autopilot. I'm mostly advocating dropping it so we get people to be using the much more reliable, better user experience form.

Sub is also sending the distance sensor which is reporting the same value. Is it doing something more complicated with it in QGC that I'm missing?

[DonLakeFlyer]

I'm not suggesting we delete the messages, I'm just indicating we should stop sending them

Ah, simple then.

Sub is also sending the distance sensor which is reporting the same value. Is it doing something more complicated with it in QGC that I'm missing?

Don't know. I just did a search. YOu'd have to ask those guys. They have code in the plugin implementation that would need to change.

The status text one is admittedly a small weight to carry forward on the autopilot.

I'll try to get this changed over in this version of QGC I'm working on.

[WickedShell]

@jaxxzer any issues with using the DISTANCE_SENSOR message in the Q rather then RANGEFINDER? As far as I can tell both are already sent and DISTANCE_SENSOR contains all the information that RANGEFINDER does plus more.

[jaxxzer]

For mavlink rangefinders, it's a bit of a concern that the same message is used for input/output of the sensor data at the autopilot. If there were some application where the autopilot was routing mavlink from another mav, the distance_sensor info could be accepted as valid input to the routing autopilot. (edge case)

Other than that, stable is using RANGEFINDER. So we will have to handle both in QGC.

[ShikOfTheRa]

Fine with me. Very minor impact to MWOSD.

Just need to migrate to BATTERY2 and RANGEFINDER. Others are already incorporated

RANGEFINDER is only one that is an issue for me – mainly because I am not so familiar with how to get same info from DISTANCE SENSOR.

It would be nice to know what version I can deprecate the older message support from. And even better if had same info for PX4. I would like to deprecate older ones when possible as memory is a challenge on the atmega chip used.

[amilcarlucas]

Please take a look at mavlink/mavlink#910 and comment it there. It provides a way to deprecate messages in a machine readable way. Each vendor can then decide to remove messages after a defined time-frame of 18 moths for example.

[IamPete1]

Looks like we have made no progress on any of these messages.

[peterbarker]

We could start to soft-deprecate these by removing them from the streams that send them - at least in some cases.

That would break some things - but they could be fixed by installing a script to set a streamrate for the message, or the application moving to a newer message (or setting the message streamrate)

We should definitely provide the option of compiling these messages out with a HAL_MAVLINK_MSG_FOO_ENABLED define.