Skip to content
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

Full resolution mode #1572

Merged
merged 67 commits into from
Jun 12, 2022
Merged

Full resolution mode #1572

merged 67 commits into from
Jun 12, 2022

Conversation

CapnBry
Copy link
Member

@CapnBry CapnBry commented May 16, 2022

⚠️ USERS ⚠️

I'm confident enough that I've started doing real flight testing with it, but be very careful. If something doesn't look right, don't fly! Plug in with caution in case I've messed something up and your motors spin up somehow, and check your LQ to verify 100 before taking off.

Two new modes

  • 100Hz Full - Lora SF7 (-112dBm sensitivity) Range = 150Hz LoRa mode. Ideal for general use servo output where every channel is full resolution. Great for new users coming from traditional systems, and the go-to mode for fixed wing with or without a flight controller.
  • 333Hz Full - Lora SF5 (-105dBm sensitivity) Range = 500Hz LoRa mode. For aerobatic helicopters, 3D fixed wing pilots, and surface RCers who want to get the most of their insanely expensive servos.
  • 100Hz Full Team900 mode - Lora SF6 (-112dBm sensitivity) Range = 200Hz LoRa mode. Team900 only gets one mode because they're not very fast to begin with. There's enough air time to use CR_4_8 vs CR_4_7 so there's a little better error correction.

333hz

Selectable channel count

  • 8ch mode - 8ch (CH1-CH4 and CH6-CH9) are sent 10-bit at the selected rate, along with AUX1 in 1-bit for arming
  • 12ch mode - CH1-CH4 are sent 10-bit at the selected rate, along with AUX1 in 1-bit. CH6-CH13 are sent 10-bit at half rate
  • 16ch mode - CH1-CH16 are sent 10-bit at the half rate

8ch and 12ch modes actually has one extra channel, AUX1 which is still 1-bit, but I didn't want to include that in the channel count because everyone hates AUX1 anyway. 8.1ch and 12.1ch?! 😏

Improved telemetry bandwidth

At our standard rate (4Hz telemetry) the current modes only have 10 bytes of telemetry bandwidth per second. The fatter packet means we can send ~2.4x more data per second at the basic rates. These modes also do not require a dedicated telemetry slot for LinkStats. When sending LinkStats, the advanced telemetry bandwidth is just slightly reduced for that packet.

Faster telemetry for all

In addition to that! The Stubborn telemetry transfer system currently always has to send a blank packet at the end of every item it transfers as the final ACK message to complete the transfer. Sending the typical VBAT telemetry of 12 bytes this goes in chunks 5-5-2-0. The code has been changed to accept the final ack on the "2" packet instead of requiring a separate "0" packet. For the VBAT example this transfers 25% faster. However, no single packet transfers are possible, as the receiving end needs to be able to discriminate a new transfer from the tail of a multi-packet resend that wasn't acked.

  • Current 500Hz 1:128, full Betaflight telemetry with GPS -- 7.6s
  • This PR's 500Hz 1:128 -- 5.6s
  • 333Hz Full Res 1:128 -- 3.5s

Due to changes being needed in the StubbornSender/Receiver classes that require them to be fully revalidated, this works its way in to this PR too.

Improved precision for original modes

Our current OTA we call "10 bit", however the way the ~10.9 bit CRSF data is decimated down to 10 bit is simply to divide it by 2. But! CRSF supports partial E.Limits i.e. beyond the 988 to 2012 range. For quads, this precision is wasted, meaning we only ever fill 80% of our 10 bit range with data. I'm changing that so our normal 4ch OTA uses all 10 bits to store 988-2012 and clamps any input outside that range, thus increasing our precision for our standard modes by 25%. Fullres mode is unaffected and supports the full CRSF range (885us - 2115us).

Always includes TPwr

The TX power level is included in the uplink packet in all Full modes, not just limited to Wide switch mode any more.

Why half/third/quarter rate small packets sucks

The simplest way to add more fullres channels is to just to halve our effective rate and just rotate though all the channels, right?!

  • Our current OTA packet is 60% payload, 40% overhead
  • Each packet transmission has also has overhead on our side, requiring around 200-400us of turnaround time between packets
  • The smaller payload is reduced even further by our own protocol overhead, meaning an 8 byte OTA packet is only 5-6 bytes of payload

0.6 OTA x 0.8 tock slack x 0.625 protocol overhead -> 30% of each period is spent actually transferring usable data and the 70% is overhead. If we want to send 8 full channels, we'd have to split that in half, and 12 channels would reduce it to a third.

A fatter packet just works better since all those overheads listed above are fixed per packet.
0.75 OTA x 0.8 tock slack x 0.77 protocol overhead -> 46% of time used actually transferring data or more than a 50% improvement in raw bandwidth. For transferring 8 full channels, that's a 100% improvement over 500Hz with round robin!

Downside

There's no denying that a packet that can be transferred more quickly is slightly more robust to random interference than a larger one. The technical term is putting all your eggs in one basket. In practice this doesn't appear to be significant, and nobody is complaining that our current lower rate modes perform worse, right?

Y U NO make rate slower

This is not about range, this is about precision. What good is a 10-bit channel going to do for smoothness when it is operating at 30Hz or less? SF7 is a good balance between update rate and range-- 4dBm more sensitive than our 250Hz mode that has done 40km. The encoding was chosen to give 12ch at 100Hz+50Hz, or 8ch all at 100Hz. Upping to SF8 would mean 12ch at 34Hz for just 3dBm more sensitivity.

Superseded PRs and Implementation Stuff

Built on 1421

#1421 "OTA packets to use structure instead of direct byte handling" was used as the base for this PR.

Prevent switch mode change while connected

It makes sense to put a check on the RX to prevent a switchmode change while connected, so this PR supercedes #1540 "More switchmode guard" and prevents the change. This PR also contains the Ota changes from #1540, which were then resubmited as #1567 "common 5 main channel same encoded". Confused at all?

The code now only allows a switchmode change on the RX if it is disconnected. It does this now by pushing the change to the main thread and blocking all RC channels packets until the mode changes.

Lua field peer refresh

Changing the packet rate now changes what the switch mode values are, so the Lua needs to reload to some degree. On master it is also broken that changing the Packet Rate changes the TLM bandwidth, but that doesn't update because it is a different field.

Now when a field value is changed, all of the same-level editable items are reloaded. For example, changing Packet Rate reloads Packet Rate first, then TLM Ratio, then Switch Mode, then Model Match. This is implemented as a loading queue, replacing the older sequential field ID counter. Somehow the loadQ system ends up being smaller too because it is simpler.

I've also removed the float type Lua handlers and the ability to edit strings. We do not use these so it makes the script smaller to not include them and save the RAM.

Unintentional Fixes

  • The optimized Stubborn unveiled a bug in our "Enable Rx Wifi", as the Rx goes to wifi without ever ACKing the transaction, so the TX just sends it over and over forever until the Rx comes back. Thanks to the optimization, the RX now has enough data to know it needs to go to Wifi even after the reboot, so it goes back into Wifi. On master, the last packet that is being repeatedly sent is blank, so it all works out to not go into Wifi after reboot. I've added a 500ms delay before the wifi switch, but the right way to fix this is to wait until the MSP ack bit is correct. I think this is close enough.

TODO

  • The SYNC packet going TX -> RX has 4 bytes left over in it. I can't think of what to put here since the sync packet won't have more data on the 4ch modes, and the sync packet isn't even guaranteed to be on during sync so it would have to be something the others could live without, like maybe the TlmResult flag for one. I'm open to ideas.
  • Provide updated OTA specs for spi-dev
  • We are so out of RATE colors for the haz RGB crowd, so I dunno what we can do about that because Jye's got two other rates too with DVDA. Maybe color them by sensitivity instead?
  • Because changing the packet rate now changes what the switch mode values are, the Lua should update everything at the same level when a value is changed. This will also fix the TLM ratio bandwidth not updating when you change the packet rate which is currently broken in master
  • Add 100hz fullres mode for Team900. This is easy just haven't done it yet.
  • The StubbornSender seems to always start out of sync with the other end and takes a bunch of packets to reset. I'll figure that out.
  • Now that I am doing more rigorous testing, it is clear that the old modes aren't working any more. They were earlier yesterday! I'll fix this
  • 16ch mode
  • Changing switchmode on the RX needs to probably be moved out of the received-packet ISR, as the function it calls is not IRAM and I don't want it to be. I might split the switchmode setter out? Or maybe have it update in the main loop instead like the rate change

@CapnBry
Copy link
Member Author

CapnBry commented May 19, 2022

I'll provide more concise details about the exact changes to the OTA layouts in this post. This is in progress!

OTA Changes

Switch mode changed to 1 bit

The OTA for even our regular modes has been changed to only be 1 bit for the switch mode, down from 2. We have so many RF modes that we needed more space in the sync packet and honestly I don't even think we need two switch modes on regular ELRS. Wide switch mode is now the default too. The 8ch/12ch flag is sent in every packet in full mode, so switch mode is irrelevant for full mode.

Encoding of existing rates CH0-CH3 changed

It made sense to make all the 4x 10-bit channels use the same OTA format so a single function could be used. Rather than using our current (8 + 8 + 8 + 8 + 2:2:2:2) encoding, I've made them all use the CRSFv3 format. Values are packed little-endianish such that bits A987654321 -> 87654321, 000000A9. See OTA::PackUInt11ToChannels4x10() for details.

To expand the 10-bit value back to CRSF values:
Standard modes : map(ch10bit, 0, 1023, 172, 1811) // 0=988us 1023=2012us
Fullres modes: ch10bit << 1 // CRSF "legacy" scale

Sync channel moved to (N/2)+1

The sync channel is at FHSS Hops / 2 currently, but our OTA structure is a different so I wanted to break compatibility with 2.0 systems and reduce the chance of them connecting. This can be removed if we can come up with something better, since it is possible that a 2.0 system can see a 3.0 syncspam packet and misinterpret it still.

Air rate mode indexes changed

SPI can just ignore any air rate it does not support

Team 900

Index Prev. Index Air Rate
0 0 200Hz
1 x 100Hz Full
2 1 100Hz
3 2 50Hz
4 3 25Hz

Team2.4

Index Prev. Index Air Rate
0 x 1000Hz FLRC
1 x 500Hz FLRC
2 x DVDA 500
3 x DVDA 250
4 0 500Hz
5 x 333Hz Full
6 1 250Hz
7 2 150Hz
8 x 100Hz Full
9 3 50Hz

Misc

  • Air rate in SYNC packet now 4 bits
  • Switch mode = 0 Wide (was 2), Switch mode 1 = Hybrid (was 1)
  // Air rate and switch mode in SYNC packet
    uint8_t switchEncMode:1, // low bits
            newTlmRatio:3,
            rateIndex:4;  // high bits
  • Wide switch mode RC channels packet CRC uses FHSS+1 as the seed instead of FHSS. Packet 0 still would pass CRC without the +1
  • StubbornSender sends data in PackageIndex 0 now
  • LinkStats mspConfirm moved to high bit of LQ field, LQ 7 bits

Copy link
Collaborator

@pkendall64 pkendall64 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Theres nothing really to complain about here! Nice job.

src/include/common.h Outdated Show resolved Hide resolved
src/lib/CRC/crc.cpp Show resolved Hide resolved
src/lib/SCREEN/display.cpp Show resolved Hide resolved
src/src/common.cpp Outdated Show resolved Hide resolved
@CapnBry
Copy link
Member Author

CapnBry commented Jun 8, 2022

I'll fix the remaining merge conflicts tomorrow morning as I'll have a little time then.

@JyeSmith
Copy link
Member

JyeSmith commented Jun 9, 2022

I've taken this out of draft status, it is up to date with master and has all the tests working, but I have one more trick up my sleeve for the OTA encoding that will apply to everyone. It should be usable in it's current state but if we're changing the OTA formatting, I think I can squeeze more precision out of the same 10 bits

Since we are changing OTA encoding, lets go all the way for FLRC and just send the 11b channel 😃

This means we get the absolute most out of the current crsf version, double the resolution from existing releases, and remove any weird rounding going from the crsf 10.5b, to 10b OTA, back to 10.5b, then BF scaling to the 10b range but as a float.

The change fits the topic of this PR, but we can do it in another since this is already a monster to review/test?

@CapnBry
Copy link
Member Author

CapnBry commented Jun 9, 2022

Since we are changing OTA encoding, lets go all the way for FLRC and just send the 11b channel 😃

I hope you're joking. I spent months with this kicking around in my head, drafting ideas and running math trying different encoding parameters and OTA layouts for all the packets. Over two full time weeks hand tweaking code, comparing size and performance, and validating with dozens of hours of bench testing on top of the development time.

Why not just throw together a third whole OTA format for some existing air rates a week before the feature freeze?

  • Expanding the packet size by 4 bits leaves 4 remainder bits unused. Are you going to redo switch modes to consume those? One new Hybrid with 8/9 bit resolution? What about the blank space in the sync packet?
  • A longer FLRC packet time severely degrades performance, you said this. And now (still) without any rigorous testing, you're abandoning that and saying make the packet bigger.
  • What advantage does would the increased precision bring? Are you suggesting we support full ±120% CRSF in FLRC, or 11bit representation of 988-2012? The latter does nothing (can't pull more precision out of the data coming from than this already does) and the former provides no benefit over the reduced range 10bit in this PR.

It is shocking to me how absurd an idea this is just a few days from the feature freeze, with the careful consideration that needs to be given to such a change. I dunno 11bit would be more. 🤪 It would be more but I'm certain it is worse*

Asterisk- unless you need 885-2135us for your quad. Do I need to find your quote from last week we're.you said longer OTA duration was a nonstarter for FLRC because of how sensitive it is to interfere.

@JyeSmith
Copy link
Member

JyeSmith commented Jun 9, 2022

FLRC has 14 free bits from the CRC.

@CapnBry
Copy link
Member Author

CapnBry commented Jun 9, 2022

FLRC has 14 free bits from the CRC.

Ah that's right. I misunderstood what you intended and I didn't have much time to try to get there. I guess what we'd do is

  • Put the Nonce (or at least 1 or 2 bits of it) in every channels packet for FLRC
  • Add new Pack/Unpack/CRC (nonce check in this case) functions
  • The Advanced TLM packet should have its length updated to carry another byte.
  • Linkstats will now have 2 free bytes which seems a shame to waste so it should a similar implementation to fullres where 5 bits of the header becomes the package index, and one becomes "contains linkstats". That frees an additional byte in the telemetry packet, so the advanced telemetry would change from 5 to 6, and linkstats telemetry would carry 3 bytes instead of 0.
  • MSP also gets length +1, but also should move its package index to the header to get a +2.
  • Wide switch mode can be expanded to at least 8-bit always as well for that mode.
  • Hybrid I don't want to try to do anything with because adding more resolution there doesn't do anything except for the 16-pos being more pos, 2/3/6 position switches will still be 2/3/6 position. Both switch implementations will need entirely new functions though because the whole packet layout is probably going to be different (weeps).
  • I feel like there's something else I'm missing maybe, or did I think of everything?

It's just throwing more rx/tx packet handler complexity and a whole new set of OTA serializers, and now really this PR shouldn't be reviewed yet because all air rates and switch modes are going to have to run through code paths that touch all that new code. I'm converting this back into draft status until all that's done.

@CapnBry CapnBry marked this pull request as draft June 9, 2022 17:59
@wvarty
Copy link
Collaborator

wvarty commented Jun 10, 2022

@CapnBry my preference would be to get this in as-is, especially since @pkendall64 has given it the thumbs up.
We can then look at iterative improvements / changes to the OTA modes, and channel bit-width, as part of a subsequent release.
I get that changes to the OTA modes requires a major version bump, but there is a lot to be said for an "agile" delivery approach, where we deliver value incrementally in smaller releases, rather than a waterfall approach.
Thoughts on taking this back out of draft and getting it merged?

@JyeSmith
Copy link
Member

JyeSmith commented Jun 10, 2022

* Put the Nonce (or at least 1 or 2 bits of it) in every channels packet for FLRC

My only thoughts are for the RC packet. Reserve space for 12b channels even though only 11b is needed for current crsf channels. And as you mentioned a bit to describe the nonce slot.

@CapnBry
Copy link
Member Author

CapnBry commented Jun 10, 2022

Thoughts on taking this back out of draft and getting it merged?

I'd be happy with that, as this is a pretty big change. Not only would there be new packers/unpackers, but the core code can no longer use the packet size to determine the encoding, and the main code needs separate codepaths to divert to writing to the proper place in the structures. It might be better saved for the next OTA version we'll probably want to do ~6 months from now for other reasons not yet discovered.

The "10-bit" from this PR already has much improved precision compared to "10-bit" in OTA 2.x, now having a 1-to-1 mapping from 988-2012, vs around 1-to-1.2 for the latter.

My only thoughts are for the RC packet.

If we're going to change the Channels packet, then there's no reason to not do the other changes as well. The bit allocation for the channels in that packet are up to you, 12-bit reduced range CRSF (988-2012) is fine by me as the switch channels aren't really that important after fullres is available. Going from 5 bytes of payload to 7 for MSP and Adv.Telem, plus 2 extra bytes in the Linkstats Telem is huge though for things like F1000 Airport (more than 40% more throughput) so I feel like if we're going to make an OTA4F, it definitely should do all the packet types.

With the benefits of the increased precision this PR already adds to the regular OTA4, would you be on board with delaying these changes to the next OTA version? We could also use that time to actually get 11 full bits and 12 bit reduced range from the handset, by working with EdgeTX to implement CRSFv3's arbitrary resolution packed channels.

@JyeSmith
Copy link
Member

Sounds good

@CapnBry CapnBry marked this pull request as ready for review June 11, 2022 10:50
Copy link
Collaborator

@wvarty wvarty left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Flew both 333hz and 100hz using the new "Std" tlm mode...
Tested switching modes on the bench and the older modes still worked (just flicking sticks in BF configurator)
Tested 12ch and 16ch modes on the bench
Tested a pot on a non stick channel, and noted the resolution
Any defects we find can be fixed on master as we discover them, since were committed to full res for 3.0 anyway

@wvarty wvarty merged commit 521e214 into ExpressLRS:master Jun 12, 2022
bkleiner pushed a commit to bkleiner/ExpressLRS that referenced this pull request Jun 12, 2022
* OTA: packets to use structure instead of direct byte handling

* Add be32toh

* Fix OTA for 3 bit rate index / broken wide support

* Break OTA compatibility and config
-- Rate changed to 4 bits
-- Switch mode just one bit
-- AUX1 always in high bit of switch byte

* First working refactored for fullres

* Put BLE back in, fix errant paren

* CrC != cRc

* Fix dbg_linkstats decoding for 8ch

* Remove debug cruft

* Stubborn no longer requires blank packet to complete

* Update tests for new sender behavior

* Fix original modes not working

* Delay jump to wifi on MSP

* Fix adv.telem on 4ch rates

* Add Team900 100Hz fullres mode

* Use FHSS+1 for Wide switch mode init

* Add 16ch mode

* Refactor PackedRCdataOut to ChannelData

* Remove IsArmed from RX/TX

* Forgot about my PWMPs

* Push switchmode change to main thread

* Prevent rate change that changes switchmode when connected

* No servo jumps on 16ch mode as the mode changes

* Replace statusComplete

* Fix automatic switchmode change

* Lua sequential loader => queue for reloadRelatedFields

* Add missing OLED/TFT screen labels for modes

* RGB rate hue is now just a index * 255 / MAX

* PayloadLength to determine OTA format instead of Rate

* Fix DEBUG_FREQ_CORRECTION overflowing the SNR field

* You dummy, you know MAXINT7 is -64 to 63

* Fix calls to OtaUpdateSerializers after criteria change

* Revert allowing single packet Stubborn transfers

If the entire transfer takes place on the 0th packageIndex
the receiver can't discriminate a resend of a packet tail
from a new send, which flips the confirm bit and starts
a cascading failure until RESYNC

* collectgarbage after each string build

And some other incredibly minor code reduction

* Typo in field type bitmask

* Slightly more realistic TLM bandwidth for fullres

* Fix tests now that they are working again WOOHOO.exe

* Tests for OTA8

* Fix UINT10_to_CRSF only going to 1999 for 2000 input

* Smaller RcPacketToChannelsData using common bitpacker

* Unncessary volatiles

* Remove unitended change from 79de904

* TX MSP functions only if !CRITICAL_FLASH

* Clarifying comment

* Fix DEBUG_CRSF_NO_OUTPUT for RCframes

* Return of F500

* More efficient byte packing of rates/mod settings

* Use full 10-bits for 988-2012 for OTA4 main channels

* SNR uses the whole OTA field now, is signed

* Fix DEBUG_RCVR_LINKSTATS compile errors

* You miss ONE merge problem, github all up in your business

* Fix failed merge attempt

* Return F500 to display

* RATE_DEFAULT back to 0

* Return F500/F1000 to 2 hops

* Missed the TLM differences

Co-authored-by: cruwaller <cruwaller@gmail.com>
Co-authored-by: Paul Kendall <pkendall64@gmail.com>
@CapnBry CapnBry deleted the 8ch-mode branch June 23, 2022 11:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

5 participants