-
Notifications
You must be signed in to change notification settings - Fork 32
feat: Adds frame type 0x26 RC Extended Channels Packed Payload #28
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Extends the CRSF protocol specification to transport (optionally) up to 32 proportional 11-bit wide channels.
…PR#28 tbs-fpv/tbs-crsf-spec#28 ). To be paired with EdgeTX PR <TODO! fill number>
…PR#28 tbs-fpv/tbs-crsf-spec#28 ). To be paired with EdgeTX PR#6504 EdgeTX/edgetx#6504
…PR#28 tbs-fpv/tbs-crsf-spec#28 ). To be paired with EdgeTX PR#6504 EdgeTX/edgetx#6504
|
This would be very valuable to all the function-/ship-modelists. There is an even growing community that uses CRSF-based RF-links. These people are eagerly looking for solutions transporting 32 rc-channels. As a side not: they do not only need more than 16 rc-channels, they also need up to 64 (or more) binary switch states to be transported over the rf-link (this is a clear benefit of the rc-systems that use e.g. SumDV3 protocol like Gr/SJ Hott). So, it would be also very helpful to add a package type for that purpose. The channel data and the switch data may be transported in OTA packages with different update rates (like Hott does). |
|
fantastic to see efforts into this direction. Extending beyong 16 channels is certainly much desired for ground based vehicles. however, I'm not convinced the proposed message is the right step forward, and I'm not convinced it even is needed. There is the message 0x17 Subset RC Channels Packed which is quite flexible and - as much as I can see - perfectly can do that. It is also already implemented in e.g. ArduPilot, so no additional mess also on this side. |
|
Here is the direct link to the For the reference, here the (IMHO suboptimal) ArduPilot implementation of the CRSF 0x17 frame: https://github.com/ArduPilot/ardupilot/blob/ceb5ad848a5c41820c2bf37b1908a7b54600a346/libraries/AP_RCProtocol/AP_RCProtocol_CRSF.h#L246-L257 In addition, note the incomplete handling of CRSF 0x17 frame by ArduPilot: https://github.com/ArduPilot/ardupilot/blob/ceb5ad848a5c41820c2bf37b1908a7b54600a346/libraries/AP_RCProtocol/AP_RCProtocol_CRSF.h#L253-L256 |
|
That would be a great addition for INAV as well. The change to support CRSF with 32CH could be easily done since INAV supports up to 32+2 channels total already for Futaba SBUS2. There are a lot of applications that can require more than 16 available channels. |
I don't want to disturb this discussion, but afaik the SBUS2 proto is a bidi extension of SBUS to query telemetry data, but sticks to 16 channels. Maybe I'm wrong here? |
My understanding of that flag is, that it indicates the meaning of the following data: channel-values (with the specified resolution) or binary switches state (each bit resembles a binary switch). |
|
My point was that the specs should make it clear, without a shadow of a doubt, what each bit/byte does or is it for. Presently the usage of |
Absolutely. |
|
@tbs-trappy / @Dazza0 Can we have an executive decision please, either to rework/update |
|
|
Folks, pinging people over and over again will not make it go faster or make it more likely for anything to accepted, so I kindly ask to refrain from doing that. 0x17 Is discouraged because we are aware of ambiguities and because most of the additional functionality there was never implemented both on the producing and receiving side. However as the CRSF spec now expands far beyond TBS product line, I am of the opinion that changing it will only add to the mess.
As much as I appreciate that you are actively trying to improve the situation here, I don’t think this is a „either or“ type of situation. Seems to me there is many more possible solutions to this problem and I also don’t see any reason to be rushing this, especially considering this Proposal has not even made it through the 2 weeks grace period we usually impose. I will bring this PR to the attention of the team today and get back to you as soon as time permits. TLDR: good shit, but daddy chill. |
|
first, many thanks for responding, much appreciated
many thanks for the insight. I may not know all these ambiguities but it seems to me it should be possible to fix them in a backwards compatible way. The fact that the additional functionality was never implemenetd IMHO does not imply that it's a badly designed message. And, let's be fair, it may also have to do with that the specs wasn't official. We at mLRS have discussion of the >16 topic since long and I recall having suggested it as a natural way to go 2 years ago. As regards the functionality itself, I think it's a good idea. For instance, the native resolution of EdgeTx is 12 bits (if I'm not wrong), so it would make perfect sense to send with that resolution from radio to tx module. The link could then do what it wants, inlcuding using that full 12 bit or not, and send it out on the receiver as it wants. Why always the restriction to 11 bits. Mavlink can handle 13 bits.
point is that I see this dicussion for long floating through the different places, and if "no reason to rush" means to give it a good thought then I guess we are all with you but if it means not so important then I guess we would want to disagree. |
i think that is for the receiving end to judge :)
i don't necessarily disagree, but that will need much more careful consideration than a new frame and i wonder if that is "worth it" considering we could just as easily add a new frame that has similar functionality. i am purposely leaving that up discussion. i thought i made it more than clear that i meant to state my personal opinion on the matter.
that's just puzzling to me. yes, of course that means to say "good thought". |
…-crsf-spec#28) (cherry picked from commit ea78554)
|
@tbs-trappy |
|
Hi all, Just wanted to add us to the list of interested parties, we have received requests for 32-channel support also. Thanks! |
|
I would love to see an upgrade to the channel support in CRSF! @rotorman I'm not a huge fan of this extension. The CRSF 11 bit channel data format is not great, as it takes data from EdgeTX which has a larger range and higher precision then dumbs it down to the same fixed, non-extensible format of the existing channels packed message. CRSF doesn't even use the full 11 bits of the 11 bits per channel in the CHANNELS_PACKED message. I would much rather see is an upgrade to the SUBSET_PACKED definition, or I guess rather a new channels packet entirely since this no longer is a "subset". Ideally the protocol would take the full precision, full range that exists in EdgeTX. A CRSF packet can hold 32x 13 bit values, so there's no reason to "subset" anything, other than to set the format of a section of channels.
The structure repeats as many times as you need and fits in a CRSF packet, so you could have something as weird as this
There's not a ton of usefulness added here, but can cut down on transfer time? In the EdgeTX case, they can just send the number of channels the user has configured, in 4ch 12 bit blocks that use the proper format code if the user has extended limits on or not. e.g. for a user with up to 8 channels configured in the handset there would be 2x 4ch blocks, or 3+1+6+1+6+1=18 bytes. 24 channels would just be 6x 4ch blocks. The existing 11 bit 16ch would be 2x 8ch 11 bit blocks, just 2 bytes longer than the existing channels_packed for the 2 format specifiers, but would either support a wider range of values or a higher precision depending on the format. The formats are up for debate, but I do not see the point in adding a second channels packet with the exact same format and interleaving it with other channels packets, when there's plenty of room in a CRSF packet for all the channels at once at full rate even at 13 bit precision that nobody does currently, right? I understand the desire for backward compatibility, the proposal would allow older devices to still get those first 16 channels, but if you're adding a new packet type FOR THE LOVE OF ALL THAT IS HOLY do not make it CRSF channel encoding that still only covers 880-2120us and only has 10.6 bits of precision in the 1000-2000 range. 🤣 Everything could be better, not just more channels! |
|
I really like this idea of a flexible rc-channels packet format: the possibility the pack e.g. 32 low-resolution and some 1-bit channels together is of special interest to function/ship-modelists. So, I really support this idea! |
|
Indeed my suggestion in this PR was explicitly targeting for maximum compatibility with existing systems. If we are talking about devising a brand new CRSF frame, then I would suggest to go directly with 16-bit/channel (int16) and let the RF module discard the (least significant) bits it is not transmitting in the chosen OTA protocol.
How the int16 value is converted to the PWM output (and even if it is converted at all, consider e.g. DroneCAN), is then up to the receiver (configuration) to decide. Also various manufacturers can implement differing "servo" centers (e.g. some proprietary systems use 1520µs, whereas most open-source systems use 1500µs). |
|
|
If anyone is interested: I modified ELRSV4 (master) to transport 32 rc channels. You can find the modification here: https://github.com/wimalopaan/ExpressLRS/tree/wmextensions You have to use that version both on TX as well as on RX because the OTA format was modified and there were no more free bits (well, at first) to make that run-time selectable. You'll also this EdgeTX mod: https://github.com/wimalopaan/edgetx/tree/wmcrsfchanext This mod is based on @rotorman 's work, but adds the ability to detect the modified ELRS. Without this EdgeTx mod the ELRS TX is flooded with packages. P.S. be aware that this ELRS mod contains as well other modfications like the SumDV3, MultiSwitch and RGBLed support. The 32 channels support can be switched on/off at compiletime. |
|
Since ELRS already uses a flexible
This wouldn't require a new packet type (and should be compatible or could be easily made compatible) |
EdgeTX supports internally 32 channels across all radio types:
This PR extends the CRSF protocol specification to transport (optionally) up to 32 proportional 11-bit wide channels in order for the CRSF protocol not to be a limiting factor.
In the proposal PR here, the 11-bit proportional channels 17 to 32 are transported in a dedicated 0x26 frame. This results that the frame rate of the channels 17 to 32 could easily differ from the std. 0x16 RC Channels Packed Payload (channels 1 to 16) framerate.
Tested successfully with rotorman/CyberBrick_ESPNOW#8 paired with EdgeTX/edgetx#6504.