development.xml: RADIO_RC_CHANNELS message

[jlpoltrack]

This PR adds the RADIO_RC_CHANNELS message, and the related enum bitfield. It exactly mirrors the upstream PR, mavlink#1919.

This PR is required for the proposed implemention in the ArduPilot code in this PR: ArduPilot/ardupilot#24577

Comment: This PR is done by @jlpoltrack on behalf of @olliw42 (since olliw42 already does have a fork of the upstream mavlink repo and thus can't fork this repo here in addition).

[peterbarker]

Looking good now IMO.

[peterbarker]

Suggested change

<extensions/>

No reason to make these extensions AFAICS.

trailing-zero-in-payload-trimming happens regardless of extension/not extension.

[olliw42]

IMHO incorrect. The sequence of fields is reordered, such that the larger fields come first. So the unit8 fields would be at the back, and zero trim would not work for the channel fields

[peterbarker]

Good call!

Could we get a comment instead, please?

[olliw42]

done

[hamishwillee]

If we do this we're saying we will never extend this message. Is that a shared understanding?

[tridge]

yes

[peterbarker]

Suggested change

	<field type="int16_t[32]" name="channels">RC channels. Channels above count should be set to 0, to benefit from MAVLink's zero padding.</field>
	<field type="int16_t[32]" name="channels">RC channels. Channels above count should be set to 0, to benefit from MAVLink's trailing-zero trimming.</field>

[olliw42]

THX, will be done

[peterbarker]

Do we need both of these? A "channels are valid" flag might be sufficient? Could we clarify or rename entry 2 to mean exactly that, with any other bits set jsut being additional information, perhaps?

The ArduPilot implementation was only looking at one of the two existing bits, but it seems to me we should be ignoring input if either is set. I'd prefer a singlet bit to indicate whether we should be trusting the channel values ata all.

[olliw42]

we sure don't necessarily need FRAME_MISSED, but I considered it usefull (even if it may not be used in AP code)
it's kind of mirroing sbus
we could also rename it to CHANNELS_INVALID, even though I'm not sure I see what it improves.

[olliw42]

per dev call decission, flag will be removed

[tridge]

should mention that this is an set of RC channel inputs for a flight controller, it does not replace the RC_CHANNELS output message

[olliw42]

Thx, will be done

[tridge]

this can't be filled in accurately without the sender knowing if any mavlink frames were lost, as we can't know if this is new data or not.
Much better to have an 8 bit number of how many 10s of milliseconds since we received the last valid message from the receiver

[tridge]

could also be a 16bit or 32 bit millisecond timestamp in the senders time domain

[tridge]

decided on call we will use a 32 bit ms timestamp since boot on the senders time domain, and remove the FRAME_MISSED flag

[olliw42]

THX
will be done

[olliw42]

@peterbarker @tridge the updated xml file should reflect the results of the dev call and incorporate the above suggestions.
Once merged I'll update the associated PR in the ardupilot main repo.
Thx to @jlpoltrack for helping out.

[hamishwillee]

If we can conceivably want more than 32 channels, would we be better to use an index and a count for this message? And in that case you could have a smaller array if you wanted.

Or if we're agreeing that truncation is actually important enough here to force reordering and never extending, why don't we make this a full length array (i.e use up the rest of the packet).

[olliw42]

I thought about this. We could add a field with the first channel in the message, which would allow sending several. Also, sure one could make it as large as possible (and I increased already from 24 to 32).
But: I really think we should not overdo it ...

My first impulse: REALLY? I would argue if really more than 32 RC channles are needed really something is wrong with the setup and what this person wants to do. There should then be a more appropriate "microservoce". I.e., the only reason I can see one may want more than 32 channels is to missuse it for something which is not existing. Just put 32 switches on your radio ... I won't believe you that you will know them all just 2 weeks later.

This message can be emitted by the receiver at relatively high rate, let's say 100 Hz (depending on the receiver)! Again, I doubt one ever will need that many rc channels at high rate, and all it tells is that oen wants to do something this message isn't for.

I VERY STRONGLY advice to not make this a message which is supposed to do everything and cure all issues in this world.

[hamishwillee]

All very logical, but we've said "that'll be all we need" so many times I don't believe it any more. We recently had someone come up with a joystick with more than 32 widgets on it, and history tells me that this will grow.

But anyhow, the main reason I want this it to use up all the remaining bits so that no one can extend it - that's for the same reason you have - to stop it being reused and extended at will.

[tridge]

I don't think more than 32 R/C channels is useful

[hamishwillee]

My assumption here is that these reflect full range of an input centred on 0 - effectively a normalized input - i.e. no direct concept of mapping to a particular PWM range? If so, it should say that here.

[hamishwillee]

Ah, I see you have done that above "Channel values are in centered 13 bit format. Range is -4096 to 4096, center is 0. Conversion to PWM is x * 5/32 + 1500."

Would there be any benefit to normalizing this and completely moving away from PWM? I'm kind of hoping we might head towards a single mechanism for setting manual control input ranges across joystick and RC so there is just one handling path.

[olliw42]

I'm very against it, since the whole ecosystem around it is just not like that. All radios I'm aware off think in terms of us or %. All receivers I'm aware of do so too. You just would introduce yet another unit system and create confusion as regards how they scale, and relate to the old units. Also it's hard to truncate, it's just too common to set the radio to e.g. +-120% or even +-150%. And what is the benfit at all?

I personally am also VERY against making this a "eierlegende Wollmilchsau" (google translates to egg-laying jack of all trades LOL). More on this below.

[hamishwillee]

Normalizing to a whole range is not a complicated rescaling, but I do understand that if this is single-purpose it is unnecessary.

If there is no interest I have no objection.

[hamishwillee]

Worth saying perhaps that this might allow targeting of what is effectively a second RC controller to command a a specific mavlink component - i.e. allow having separate RC for gimbal and autopilot flight control?

What if we want to target a vehicle attached gimbal using a separate RC?

[olliw42]

IMHO that's what targets are generally for in a MAVLink system

[hamishwillee]

Right, but since you've explicitly said it can normally go everywhere, my thought was that it makes sense to say what case would make you not send it everywhere. Had you said nothing I would probably have left it. I do like spelling things out.

[hamishwillee]

Shorter or longer? What is doing the "considering invalid".

What is the sender here - GCS? What is the receiver - Autopilot?

I "think" from a year ago that you were talking about a system where this is like a normal RC Rx/Tx setup but instead of talking a common RC protocol such as sbus you would use this message. So probably when you say "sender" you mean the Rx unit that is sending this mavlink message. But you might not. Should be spelled out.

Once that is clear, then I can interpret how the timeout and RC loss/ and data invalid work.

[olliw42]

You make a very valid observation, that with regards to targets and this timestamp quite something has changed here, and I will comment to this big picture below.

As regards the mechansim, I think it's pretty simple and common (even though maybe not totally trivial to implement). First note that we have 3 situations: 1. frame with valid rc data received, 2. a frame is missed (which isn't equal to a failsafe!), ergo no fresh rc data in that time slot, 3. failsafe. So we need "something" to indicate case 2. In sbus language it would be called "missed frame". Instead of a 2nd flag (which was the original proposal) ,this field was introduced upon @tridge 's request, and is supoposed to help in case of a lossy network. So, the basic is the assumption that not all messages are receiveed but some are lost.

[hamishwillee]

Thanks.

If this time is significantly shorter than the expected update time for this message, then the RC channels data can be considered invalid, and the RC channels data should carry the last valid data."

The timestamp would be small if the sender rebooted or similar - so is this to catch the case where the sender rebooted and you can no longer use the delta between two timestamps to determine the time between the last two updates?

[hamishwillee]

What rates do we think this should be sent at?

[olliw42]

that's really up to the receiver ... it can be "anything" the link can or wants to offer.

[hamishwillee]

I've added some questions. My main concerns are the ones outlined by @auturgy here mavlink#1921 (comment) - can we at least consider a path where this approach handles RC and Joystick through one input.

[hamishwillee]

What is failsafe? When is it triggered? What should receiver do?

[olliw42]

that's really up to the receiver. All is - IMHO - relevant here is that such a thing like a failsafe can happen and that the recipient wants to react on it. "Failsafe" has here the same meaning as with every rc receiver too. Typically it would be connection lost, whatever this means to the particular link.

[hamishwillee]

The problem here is #343 (comment) - nowhere do you say this is "like a typical RC receiver" so this could mean anything. Perhaps that's correct, but either way I like to spell out what this might mean and how the recipient should react.

From above "1. frame with valid rc data received, 2. a frame is missed (which isn't equal to a failsafe!), ergo no fresh rc data in that time slot, 3. failsafe. ". So failsafe means "connection lost" right? If it always means "connection lost" can we name it that?

Otherwise maybe

Suggested change

	<description>Failsafe is active.</description>
	<description>Failsafe is active. This is sent when the receiver has lost connection (is not receiving or able to parse data from the sender).</description>

[hamishwillee]

@olliw42 Is this correct ^^^. If so, merge? I don't have rights to merge here.

[auturgy]

I've added some questions. My main concerns are the ones outlined by @auturgy here mavlink#1921 (comment) - can we at least consider a path where this approach handles RC and Joystick through one input.

I'm actually quite happy with the logical separation, but perhaps we can clarify in the message description that this is not intended to be injected into the telemetry link, but is a mavlink-native alternative to sbus/csrf wire protocols, and doesn't attempt to describe an OTA format.

[hamishwillee]

See #343 (comment) > This is not sufficient for "most people" to understand that this is the output of an Rx receiver, replacing other protocols.

[hamishwillee]

@olliw42 ArduPilot (Peter, Tridge) are happy with the design of this message - 32 channels, using PWM values. Let's take that as given.

This assumes that the channels are packed right - i.e. that the array is fully filled from the lowest index without gaps? Is that a problem? I mean that on a normal RC receiver the channels on the Rx and Tx match - so you'd set channel 5 on the transmitter (say) and that would show up as channel 5 on QGC. Is that still the case here - i.e. I am assuming that if you have an 8 channel RX then the count would be 8 even if you're not mapping a particular channel to a widget on the RC control.

The description does not capture the fact that is actually the interface for a MAVLink RC receiver and not an OTA format. That needs to change.

I don't understand how you'd use the timestamp - or to be more precise, I understand it indicates when the sender sent the data in channels. I also understand the Rx might lose or be unable to decode a packet or two. But I don't understand what this meant/what causes this problem " If this time is significantly shorter than the expected update time for this message, then the RC channels data can be considered invalid, and the RC channels data should carry the last valid data."

I've rewritten this as perhaps it should be (for your discussion). The main changes are that it states that

• this is the interface for Mavlink receiver
• the timestamp and channels data are always last valid data - even in failsafe (though that doesn't matter). My assumption is that we get to say what it should do.
• it pushes some of the description down to the fields.

    <message id="420" name="RADIO_RC_CHANNELS">
      <description>
        RC channel outputs from a MAVLink RC receiver for input to a flight controller or other component (allows an RC receiver to connect via MAVLink instead of some other protocol such as PPM-Sum or S.BUS).
        Note that this is not intended to be an over-the-air format, and does not replace RC_CHANNELS and similar messages that override the output from the flight controller.
        The target_system field should normally be set to the system id of the system to control, typically the flight controller.
        The target_component field can normally be set to 0, so that all components of the system can receive the message.
        The `channels` array field can publish up to 32 channels; these should be densely packed (without gaps) and the number channels used specified in the `count` field.
        Each channel item must contain the last valid data received (by the receiver) for indexes above `count`.
        The time_last_update_ms must contain the timestamp of the last valid data received in the receiver's time domain (i.e. of the data in `channels`).
        The RADIO_RC_CHANNELS_FLAGS_FAILSAFE failsafe flag is set by the RC receiver if the connection is lost, and the receiver must treat the timestamp and channels data as invalid.
        Note: The RC channels fields are extensions to ensure that they are located at the end of the serialized payload and subject to MAVLink's trailing-zero trimming.
        </description>
      <field type="uint8_t" name="target_system">System ID (ID of target system, normally flight controller).</field>
      <field type="uint8_t" name="target_component">Component ID (normally 0 for broadcast).</field>
      <field type="uint32_t" name="time_last_update_ms" units="ms">Time when the data in the channels field was last updated (time since boot in the receivers time domain).</field>
      <field type="uint16_t" name="flags" enum="RADIO_RC_CHANNELS_FLAGS" display="bitmask">Radio RC channels status flags.</field>
      <field type="uint8_t" name="count">Total number of RC channels being received. This can be larger than 32, indicating that more channels are available but not given in this message.</field>
      <extensions/>
      <field type="int16_t[32]" name="channels" minValue="-4096" maxValue="4096">RC channels. Channel values are in centered 13 bit format. Range is -4096 to 4096, center is 0. Conversion to PWM is x * 5/32 + 1500. Channels that are above the field count should be set to 0, to benefit from MAVLink's trailing-zero trimming.</field>
    </message>

You might hate this. That's OK. Mostly I want to make clear where I have been confused and what I think you mean. It's for discussion.

[peterbarker]

Ping @olliw42 - are you happy with Hamish's suggestions?

I'll merge in a couple of days if no response on the basis we should get this in. It's development.xml, after all!

[hamishwillee]

If this merges here it will be modified upstream, because @auturgy shares at least some of my concerns about the description. Better for it to be agreed here but up to you.

[olliw42]

@hamishwillee

many thx for your many comments, suggestions, and questions. Besides having been very busy the last two weeks, I however mainly have to confes that your expansion of the discussion to "infinity" has made me realize how little - if not to say no - energy I have left for this discussion. The path to here has exhausted me. I also think that you make the story more complicated than it really is. So I won't address the points which are related to extending this message (and associated concept) to beyond the system the receiver is connected to. I'm sorry for this, I really just have no energy left over, and if you guys want to see the message in that broader context, which I would fully understand, I rather would want to skip out.

I very much like the explicit statement of a MAVLink RC receiver, and that it's for that and not for OTA.

There are no messages which "override the output from the flight controller".

I really don't understand your concerns with the "dense packing". The very same construct is used in RC_CHANNELS, and I've never heard of any confusion, and I find it just obvious and clear. The rewriting is incorrect; "must contain the last valid data received (by the receiver) for indexes above count" would impair with the zero padding mechanism. Might be just a mixup with below.

Concerning the timestamp's time domain I have a feeling that it was mistakenly considered to be in the RC radio's time domain, while it is in the RC receiver's time domain. The original "sender's time domain" was refering to the sender/emitter of the message - now specified to be the MAVLink RC receiver.

Your rewriting doesn't - in my reading - reflect your 2nd bullet "the timestamp and channels data are always last valid data - even in failsafe". It is inconsistent; it first says that the timestamp must contain the time when the last valid data was received followed by that in failsafe it must be considered invalid.

I just can try to repeat what I think how it should be.

If the failsafe flag IS NOT raised:

• the timestamp must hold the time of the last valid channel data (in the RC receiver's time domain)
• the channels data (up to count) must hold the last valid channel data

If the failsafe flag IS raised:

• the timestamp could be specified to be invalid, but I would suggest to also require that it must hold the time of the last valid channel data; is easier and is at least some data which might be useful in some cases
• the channels data (up to count) IS NOT specified by the protocol, but is determined by the implementation of the RC receiver.

EDIT: for a updated suggestion for the description pl see #343 (comment)

[hamishwillee]

@olliw42 I also have fatigue on this one. My version of the text really settles around one point:

the channels data (up to count) IS NOT specified by the protocol, but is determined by the implementation of the RC receiver.

WHY leave it up to the protocol: why not specify that the data in the channels is "always the last valid data received by the receiver, even in failsafe"? As a recipient the FC should/can be using its own failsafe values if the flag is set so where's the benefit in not fixing this? The benefit in my version is really just simpler documentation. If there is a reason then fine.

The packing stuff may not be a concern. I just want to be sure that you never get an array where a channel less than count can be invalid, and that an X channel receiver will always have count of X.

The timestamp in the receiver's domain is much better - I wondered at that.

[hamishwillee]

Your rewriting doesn't - in my reading - reflect your 2nd bullet "the timestamp and channels data are always last valid data - even in failsafe". It is inconsistent; it first says that the timestamp must contain the time when the last valid data was received followed by that in failsafe it must be considered invalid.

The data is the last data parsed by the receiver. It can't be treated as valid by the FC in failsafe or if it is too old.

We're both using "valid data" to refer to this in timestamp. That's a problem because it's valid to the receiver, but not to the FC.

[hamishwillee]

There are no messages which "override the output from the flight controller".

Whoops. I was thinking of RC_CHANNELS_OVERRIDE which overrides the input, including from this message. So not just "RC_CHANNELS and similar messages reported by the flight controller.

[olliw42]

WHY leave it up to the protocol: why not specify that the data in the channels is "always the last valid data received by the receiver, even in failsafe"? As a recipient the FC should/can be using its own failsafe values if the flag is set so where's the benefit in not fixing this? The benefit in my version is really just simpler documentation. If there is a reason then fine.

why not! There is a good reason why many existing RC receivers allow to set their failsafe behavior. Also do not forget that the channels are possibly not only for the fc but also other components. And they may or may not do what you want to enforce. In traditional RC world it is e.g. quite common to split the sbus and wire it to both the fc and e.g. a gimbal, and to use some higher channels for gimbal control. You may even want to add a separate MAVLink RC receiver to the gimbal, or camera controller. All I see is that you are taking out lots of options and flexibility for no technical reason.

I'm btw not aware one could set user specific failsafe values in ArduPilot, I may have overlooked and may need to double check, but it seems it's not even possible what you suggest.

[olliw42]

Whoops. I was thinking of RC_CHANNELS_OVERRIDE which overrides the input, including from this message. So not just "RC_CHANNELS and similar messages reported by the flight controller.

not sure I understand what you are trying to say. RC_CHANNELS_OVERRIDE, and also MANUAL_CONTROL, are in contrast OTA messages and kind of "excluded" from the topic by that. But one can extend the sentence to also include and comment on these.

Unfortunately it seems to have become practice to missue RC_CHANNELS_OVERRIDE for the purposes of what this new message is for, and in THIS case, the new message DOES replace RC_CHANNELS_OVERRIDE :)

I'm not sure this should be commented on in the description. I guess one rather should improve the description for RC_CHANNELS_OVERRIDE. It is my general strong impression that RC_CHANNELS_OVERRIDE, MANUAL_CONTROL, etc. pp., need some modernization anyhow - which gladly is not the concern of this PR.

[olliw42]

The data is the last data parsed by the receiver. It can't be treated as valid by the FC in failsafe or if it is too old.

first part correct, 2nd part incorrect IMHO.

• "It can't be treated as valid by the FC in failsafe": correct. That's why there is a failsafe flag, and the fc implementation can decide which of the several options to follow.
• "It can't be treated as valid by the FC ... if it is too old": incorrect. The fc implementation can e.g. decide to trust the receiver implementation and continue to use the channels values - until the more severe failsafe kicks in. I bet this is going to be the most common behavior in fc implementations. The fc implementation could however also decide to work out his own idea of "too old" from the timestamps and act accordingly, in which case it's irrelevant what the receiver's opinion on the channels data is.

my point is: the receiver decides what it considers valid or last or failsafe or whatever data. You try to specify the precise method it should do that, e.g. upon loss of connection, or last parsed data, or whatever, but that's totally not needed nor relevant to the flight controller to know. All the fc needs to know is if - in the receiver's opionion - the data are fresh and shall be used by the fc, or are not, in which case the fc should be provided with some info to know a bit better what it may want to do.

As said, it looks to me you are overcomplicating the situation with too many details which are not needed for the integrity and functioning of the protocol.

As a fun fact, note that a common failsafe behavior might be for the receiver to just stop sending the message, in which case the fc never would see the failsafe flag ...
... ArduPilot gladly supports this and a number other failsafe options seamlessly. The mLRS link e.g. implements 4 different failsafe behaviors, and they all are easily handled by ArduPilot with just minimal code for the MAVLink RC receiver.

[olliw42]

having said that, I re-introduced the INVALID flag (now called OUTDATED flag to help avoide missinterpretations). It is specified to be set whenever the timestamp does not match the current time of the message but lags behind, and the channels data are not fresh. This makes interpreting/using the timestamp much easier.

see latest pushes.

[hamishwillee]

is specified to be set whenever the timestamp does not match the current time of the message but lags behind, and the channels data are not fresh. This makes interpreting/using the timestamp much easier.

So to be clear, this is when the receiver has detected data from the transmitter but can't parse it. So Rx is sending channels and data that is known to be old. old data. This is more a hint to the recipient since the data in the channels is still all valid at the timestamp.

Suggested change

	<description>Content of the RC channels field in the RADIO_RC_CHANNELS message does not contain the current/latest RC data but the last RC data received.</description>
	<description>Channel data may be out of date (recipients should check the timestamp to decide if the data can still be used). This is set when the receiver is unable to parse incoming data from the transmitter and has therefore resent the last data it was able to process.</description>

[hamishwillee]

This makes relatively minor modifications for English, not for the behaviour.

1. Sentence line breaks - make it easier to read (for me) than arbitrary line breaks.
2. Reworded the text around RADIO_RC_CHANNELS_FLAGS_OUTDATED - meaning is the same.
3. Restructured to say "The count field indicates the first index of the channel array that holds invalid data.". I've assumed MAVLink uses array indexing from 0 but perhaps we should never assume and just say "this is the count of items that are used" because different programming languages take different conventions.

Suggested change

	<description>RC channel outputs from a MAVLink RC receiver for input to a flight controller or other components
	(allows an RC receiver to connect via MAVLink instead of some other protocol such as PPM-Sum or S.BUS).
	Note that this is not intended to be an over-the-air format, and does not replace RC_CHANNELS and similar
	messages reported by the flight controller. The target_system field should normally be set to the system id of
	the system to control, typically the flight controller. The target_component field can normally be set to 0, so
	that all components of the system can receive the message.
	The channels array field can publish up to 32 channels; the number of channel items used in the array is
	specified in the count field.
	The time_last_update_ms field contains the timestamp of the last received valid channels data in the
	receiver's time domain, and the channel items up to count holds the last valid data.
	If the channels data are not up-to-date and do not hold the current/latest data, the RADIO_RC_CHANNELS_FLAGS_OUTDATED
	flag is set by the receiver.
	The RADIO_RC_CHANNELS_FLAGS_FAILSAFE failsafe flag is set by the receiver if the receiver's failsafe condition
	is met (implementation dependent, e.g., connection to the RC radio is lost). In this case time_last_update_ms
	still contains the timestamp of the last valid channels data, but the content of the channels data is not
	defined by the protocol but is up to the implementation of the receiver. For instance, the channels data could
	contain failsafe values configured in the receiver; the default is to carry the last valid data.
	Note: The RC channels fields are extensions to ensure that they are located at the end of the serialized payload
	and subject to MAVLink's trailing-zero trimming.
	</description>
	<description>RC channel outputs from a MAVLink RC receiver for input to a flight controller or other components (allows an RC receiver to connect via MAVLink instead of some other protocol such as PPM-Sum or S.BUS).
	Note that this is not intended to be an over-the-air format, and does not replace RC_CHANNELS and similar
	messages reported by the flight controller.
	The target_system field should normally be set to the system id of the system to control, typically the flight controller.
	The target_component field can normally be set to 0, so that all components of the system can receive the message.
	The channels array field can publish up to 32 channels; the number of channel items used in the array is specified in the count field.
	The time_last_update_ms field contains the timestamp of the last received valid channels data in the receiver's time domain.
	The count field indicates the first index of the channel array that holds invalid data.
	The RADIO_RC_CHANNELS_FLAGS_OUTDATED is set by the receiver if the channels data is not up-to-date (for example, if new data from the transmitter could not be decoded so the previous data is resent).
	The RADIO_RC_CHANNELS_FLAGS_FAILSAFE failsafe flag is set by the receiver if the receiver's failsafe condition is met (implementation dependent, e.g., connection to the RC radio is lost).
	In this case time_last_update_ms still contains the timestamp of the last valid channels data, but the content of the channels data is not defined by the protocol (it is up to the implementation of the receiver).
	For instance, the channels data could contain failsafe values configured in the receiver; the default is to carry the last valid data.
	Note: The RC channels fields are extensions to ensure that they are located at the end of the serialized payload
	and subject to MAVLink's trailing-zero trimming.
	</description>

[hamishwillee]

@olliw42 You're coming from the point of view where you want to change as little as possible about the existing ecosystem. To me the receiver interfaces look like an unstandardized mess that makes the implementation for the recipient harder.

No one else seems to find this a problem, and what you have will work, so why not.

I've added some English changes in #343 (comment) and https://github.com/ArduPilot/mavlink/pull/343/files#r1490175917 to make a couple of points unambiguous (I hope)

Other than that we discussed in the dev call last night and happy to merge into development.

[olliw42]

@hamishwillee

You're coming from the point of view where you want to change as little as possible about the existing ecosystem

that's maybe somewhat unfair, I think I have waved in to quite some changes compared to the initial suggestion - and each little change came with substantial work in the ecosystem where it is used (every little change had to go into three projects, and a flurry of testing)

To me the receiver interfaces look like ... that makes the implementation for the recipient harder.

well, to the contrary. Have you looked at the PR to ArduPilot, ArduPilot/ardupilot#25838 ? I find it hard to imagine how it possibly could be simpler. As a matter of fact, it is simpler than essentially all other receiver implementations in ardupilot ... at least you have to give it that :D:D:D.

now seriously.

I think the great thing is that it looks like that the structure of the message has been settled and "everyone" seems to be happy with it. It's only the text in the discriptions which is under discussion. That's really great.

So, I will happily go with your text, want to leave my opinion however.
Language like "parse", "decode", "to process", etc. pp I find strange. It appears to reflect a mental model of an rc link which doesn't quite match its actual functioning. I can't recall having heard anyone using language like this in the context of rc link. The "count field" sentence I find awkward. It is actually also wrong, since the channels above count should hold 0 and not invalid data. Which is even more confusing give that in the sentence before the notion of "valid channels data" is used. Etc. pp. So, in my humble opinion, the text is rather confusing and inconsistent. I'm happy to leave the verdict however to the next generation.

That's just my opinion, and irrelevant. Hopefully you are happy with the description you've suggested. So, I took it over, with two modifications:

• I've deleted the part "(recipients should check the timestamp to decide if the data can still be used)" in the description for the RADIO_RC_CHANNELS_FLAGS_OUTDATED flag. It's the purpose of this flag to convey this information without having to go through the hazzles of using the timestamp (using the timestamp is not straightforward).
• I've added the word "flag" in the RADIO_RC_CHANNELS_FLAGS_OUTDATED sentence in the main description.

I hope you find that ok, and that you can give your ok to merge to Peter.

Once this message is merged here, I will ensure to update the PR in the main mavlink repo, so that both are in sync.

[hamishwillee]

@olliw42 I've replaced parsed/decoded etc with "validated" and "unable to validate". If these are better aligned with the domain then great.

If you like those changes/think they are better I'm happy to merge. Of course upstream may have further comments outside of me, but I suspect not as we have talked about this a bit.

[hamishwillee]

THanks for making the changes @jlpoltrack . I'm good for this to be merged and pushed upstream.

[olliw42]

@peterbarker: seems @hamishwillee has given an ok for merging :)

[olliw42]

any reason to not merge?

[peterbarker]

Thanks everyone, that was an epic discussion.

Is someone going to freshen upstream now?

[olliw42]

THX!

I promised to do, so I'll do :)

I first will do the ArduPilot PR however, I guess with a separate PR to update the submodule (to learn how to do this)