Automatic protocol version handshake #5233

LorenzMeier · 2017-06-03T09:37:47Z

This extension allows to negotiate with all connected vehicles the highest possible MAVLink version supported by all. It is compatible with legacy vehicle software (interprets NACK and timeouts as v1.0 requests). This allows to have the best possible version enabled without any user configuration.

Before connecting QGC:

instance #0:
	mavlink chan: #0
	no telem status.
	flow control:	OFF
	rates:
	tx: 23.215 kB/s
	txerr: 0.000 kB/s
	rx: 0.000 kB/s
	rate mult: 1.000
	accepting commands: YES
	MAVLink version: 1
	transport protocol: UDP (14556)

After connecting QGC with this extension:

instance #0:
	GCS heartbeat:	988721 us ago
	mavlink chan: #0
	type:		GENERIC LINK OR RADIO
	flow control:	OFF
	rates:
	tx: 30.495 kB/s
	txerr: 0.000 kB/s
	rx: 0.021 kB/s
	rate mult: 1.000
	accepting commands: YES
	MAVLink version: 2
	transport protocol: UDP (14556)

LorenzMeier · 2017-06-03T09:49:18Z

Matching firmware PR: PX4/PX4-Autopilot#7344

LorenzMeier · 2017-06-04T08:15:20Z

@DonLakeFlyer Firmware PR is through. If you can review / cross-test we can get this in (but its not needed for v3.2).

LorenzMeier · 2017-06-04T11:45:22Z

@DonLakeFlyer Full documentation: https://mavlink.io/en/mavlink-version.html#version-handshaking

DonLakeFlyer · 2017-06-04T19:16:01Z

This isn't quite right. It treats all links as a single group. Either mavlink 1 or 2 based on lowest common denominator vehicle reporting. It should really be on a link by link basis based only on the vehicles on the specific link. That said, I think that gets way more complicated. This should be fine for 3.2. And maybe forever.

I still don't get how if there is a radio that doesn't support mav 2 in the middle how this works. Mavlink defaults to mav 1 for status flags doesn't it? So initial send of MAV_CMD_REQUEST_PROTOCOL_VERSION will be over mavlink 1 protocol. And it will go through. Is the link supposed to start out as mav 2 and then fall back? If so I don't see the code for that.

LorenzMeier · 2017-06-04T19:22:52Z

@DonLakeFlyer The reason I don't want to treat it link-by-link, vehicle-by-vehicle is that managing that configuration space is super error prone (imagine that you can send geofence over one link, but not over another - how do you want to deal with that?). Anyone who knows what he is doing will use MAVLink 2 exclusively. The only reason to have detection and fallback is to ensure that random users don't get bitten when they upgrade to a new QGC version and their old plane is not yet supporting MAVLink 2 throughout.

But really, the main focus here is to enable a safe upgrade path - I'm not really trying to support mixed setups.

In terms of detection: The trick is that the PROTOCOL_VERSION message is sent in MAVLink 2 framing. That ensures that the link supports 2.0. But the message will in the future be also important for cases where we have v2.1 or v2.5.

LorenzMeier · 2017-06-05T20:47:06Z

This is good to go. If the remote vehicle does not support discovery but sends MAVLink 2 it will stick with that as default.

LorenzMeier · 2017-06-17T11:24:43Z

@DonLakeFlyer I haven't tested this yet on hardware, but wanted to see if it fits the flash. This branch teaches the SiK radio to respond properly to PROTOCOL_VERSION, which in turn would allow QGC to detect wether or not the radio can do MAVLink 2. It likely will require adjustments to this pull, but this would be the watertight solution.

LorenzMeier/SiK#1

LorenzMeier · 2017-07-11T09:14:44Z

I will test this one round more and then get it in. @bkueng This is needed for polygons and stuff as it switches the system automatically.

LorenzMeier · 2017-07-30T00:16:34Z

Re-tested: With just USB connected the link auto-switches to MAVLink 2, with a 3DR Radio it stays at MAVLink 1. And with forced MAVLink 2 it also seems to work:

nsh> mavlink status

instance #0:
	GCS heartbeat:	810149 us ago
	mavlink chan: #0
	type:		3DR RADIO
	rssi:		219
	remote rssi:	219
	txbuf:		100
	noise:		56
	remote noise:	45
	rx errors:	0
	fixed:		0
	flow control:	ON
	rates:
	tx: 0.138 kB/s
	txerr: 0.000 kB/s
	rx: 0.059 kB/s
	rate mult: 0.058
	accepting commands: YES
	MAVLink version: 2
	transport protocol: serial (/dev/ttyS1 @57600)

instance #1:
	mavlink chan: #1
	no telem status.
	flow control:	OFF
	rates:
	tx: 0.308 kB/s
	txerr: 0.000 kB/s
	rx: 0.014 kB/s
	rate mult: 0.050
	accepting commands: YES
	MAVLink version: 2
	transport protocol: serial (/dev/ttyS2 @57600)

instance #2:
	GCS heartbeat:	877322 us ago
	mavlink chan: #2
	type:		USB CDC
	flow control:	OFF
	rates:
	tx: 1.518 kB/s
	txerr: 0.000 kB/s
	rx: 0.021 kB/s
	rate mult: 1.000
	accepting commands: YES
	MAVLink version: 2
	transport protocol: serial (/dev/ttyACM0 @2000000)
nsh>

From QGC output: Switching outbound to mavlink 2.0 due to incoming mavlink 2.0 packet

LorenzMeier · 2017-07-30T00:52:18Z

This still needs work - I still need to test with the upgraded radio.

DonLakeFlyer · 2017-07-30T00:57:16Z

Looking through this I can't quite see if it delays the initial plan request until it knows the mavlink version?

…and v2

This extension introduces a complete version handshake mechanism which will detect wether the vehicle AND the complete link / routing infrastructure support MAVLink 2. If they do, QGC will switch to the highest protocol version supported by all connected vehicles. If a new vehicle connects, it will re-negotiate the highest possible version. This means that we error on the side of compatibility, which later can be easily changed.

…two links to the same vehicle are connected and one can only do v1

This is important to avoid having QGC hijack links that are active (we read 2000 bytes!) but are definitely not MAVLink. This can have nasty side-effects, e.g. if we would talk to an LTE modem in a laptop or alike.

…ect command.

…nabled, including radio status.

LorenzMeier · 2017-07-30T16:26:57Z

@DonLakeFlyer Yes it does, because it queues the command requesting the protocol version and only fully initializes when that is received.

I've also taken care of the SiK / 3DR radio: This PR now contains an updated link to a stable build that is a) MAVLink 2.0 enabled and b) reacts correctly to the handshaking mechanism. Code for that is here: LorenzMeier/SiK#1

If a user has not yet updated his 3DR radio QGC will tell him now. It will still work as a link but fall back to MAVLink 1 automatically with a pop-up warning the user. With this addition this should be watertight and good upgrade path for the user base. It's properly documented, takes care of the corner cases and does not leave existing users behind.

@dogmaphobic You will want to upgrade your ESP8266 bridge too. Here are the handshaking docs: https://mavlink.io/en/mavlink-version.html

From my side this is now all good to go.

…s in tests

DonLakeFlyer · 2017-07-31T04:07:13Z

Yes it does, because it queues the command requesting the protocol version and only fully initializes when that is received.

I still don't see how this causes _startPlanRequest to not be called until the protocol version is known. The code in Vehicle still triggers _startPlanRequest on capability bits known and parameters ready. I don't see any checked for protocol being known as well.

LorenzMeier · 2017-07-31T06:29:37Z

Let me re-check it. I was relatively certain that the remaining load could not happen until the first commands were handled, but I might have missed a parallel code path. I will update here.

LorenzMeier · 2017-07-31T07:42:58Z

@DonLakeFlyer I made the dependency explicit.

DonLakeFlyer · 2017-07-31T20:25:41Z

@LorenzMeier Yup. Looks good. Any reason to not merge now. Then I can use this in my GeoFence stuff as well.

LorenzMeier · 2017-07-31T21:35:17Z

Ok, merged.

LorenzMeier requested review from dogmaphobic and DonLakeFlyer June 3, 2017 09:37

LorenzMeier force-pushed the proto_ver branch from c640644 to 9d8937f Compare July 11, 2017 09:09

bkueng mentioned this pull request Jul 20, 2017

New Mavlink GeoFence and Rally Point support #5474

Closed

LorenzMeier force-pushed the proto_ver branch from 9d8937f to e161cea Compare July 30, 2017 00:15

LorenzMeier force-pushed the proto_ver branch from e161cea to 47a0ac9 Compare July 30, 2017 12:17

LorenzMeier added 7 commits July 30, 2017 18:20

Remove stale MAVLink 1 submodule as the MAVLink 2 headers support v1 …

7d08bec

…and v2

Switch only to MAVLink 2 on first connected link. Fall back to v1 if …

bee2c95

…two links to the same vehicle are connected and one can only do v1

MAVLink: Disconnect link if it does not contain protocol data

18e4f8b

This is important to avoid having QGC hijack links that are active (we read 2000 bytes!) but are definitely not MAVLink. This can have nasty side-effects, e.g. if we would talk to an LTE modem in a laptop or alike.

MAVLink: Set v2 state also on incoming MAVLink 2 packets

a1ca0b9

MAVLink: Accept existing MAVLink 2 connection and accept link disconn…

fde8c91

…ect command.

Radio upgrade: Point updater to a binary which is fully MAVLink 2.0 e…

f6909fd

…nabled, including radio status.

LorenzMeier force-pushed the proto_ver branch from 47a0ac9 to f6909fd Compare July 30, 2017 16:20

MAVLink protocol: Increase disconnect holdoff to avoid false positive…

6148429

…s in tests

Start plan request: Wait until the protocol version is definite.

0593826

LorenzMeier merged commit db714c2 into master Jul 31, 2017

LorenzMeier deleted the proto_ver branch July 31, 2017 21:34

This pull request was closed.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Automatic protocol version handshake #5233

Automatic protocol version handshake #5233

LorenzMeier commented Jun 3, 2017

LorenzMeier commented Jun 3, 2017

LorenzMeier commented Jun 4, 2017

LorenzMeier commented Jun 4, 2017

DonLakeFlyer commented Jun 4, 2017 •

edited

Loading

LorenzMeier commented Jun 4, 2017

LorenzMeier commented Jun 5, 2017

LorenzMeier commented Jun 17, 2017 •

edited

Loading

LorenzMeier commented Jul 11, 2017

LorenzMeier commented Jul 30, 2017

LorenzMeier commented Jul 30, 2017

DonLakeFlyer commented Jul 30, 2017

LorenzMeier commented Jul 30, 2017 •

edited

Loading

DonLakeFlyer commented Jul 31, 2017

LorenzMeier commented Jul 31, 2017

LorenzMeier commented Jul 31, 2017

DonLakeFlyer commented Jul 31, 2017

LorenzMeier commented Jul 31, 2017

Automatic protocol version handshake #5233

Automatic protocol version handshake #5233

Conversation

LorenzMeier commented Jun 3, 2017

LorenzMeier commented Jun 3, 2017

LorenzMeier commented Jun 4, 2017

LorenzMeier commented Jun 4, 2017

DonLakeFlyer commented Jun 4, 2017 • edited Loading

LorenzMeier commented Jun 4, 2017

LorenzMeier commented Jun 5, 2017

LorenzMeier commented Jun 17, 2017 • edited Loading

LorenzMeier commented Jul 11, 2017

LorenzMeier commented Jul 30, 2017

LorenzMeier commented Jul 30, 2017

DonLakeFlyer commented Jul 30, 2017

LorenzMeier commented Jul 30, 2017 • edited Loading

DonLakeFlyer commented Jul 31, 2017

LorenzMeier commented Jul 31, 2017

LorenzMeier commented Jul 31, 2017

DonLakeFlyer commented Jul 31, 2017

LorenzMeier commented Jul 31, 2017

DonLakeFlyer commented Jun 4, 2017 •

edited

Loading

LorenzMeier commented Jun 17, 2017 •

edited

Loading

LorenzMeier commented Jul 30, 2017 •

edited

Loading