Optimised TX + Continuous mode RX #3353
Conversation
100% broken other radio types
# Conflicts: # src/lib/SX127xDriver/SX127x.cpp # src/lib/SX1280Driver/SX1280.cpp
# Conflicts: # src/lib/LR1121Driver/LR1121.cpp
# Conflicts: # src/lib/LR1121Driver/LR1121.cpp
|
To also cover the why of this PR, it is due to the exceptionally long time the LR1121 takes to go from STANDBY_RC to TX. This causes the transmit following an LBT CCA (which was before this PR is done with a receive + timeout) to be delayed up to 150us longer, and can cause the RX's timer sync to be thrown off enough to jump into another packet period. This was observed by myself and Mickey in master on the 2.4GHz K1000 mode and 3.x.x (although possible in other modes) as telemetry dropping off and coming back as the two desync. Observations of the RX PFD timing was showing large deviations of -10us to +150us from the expected packet reception time. The SX1280 and SX127x were also modified to remove their RX timeouts as it unifies the code between all RF regions-- if we're saying it is safe for LBT users, then we stand behind it for other regions as well. There is a slight benefit for SX1280 in reducing the packet jitter, although it is only maybe 5us in my measurements. Both SX1280 and LR1121 appear to not go to STANDBY_FS following a receive timeout (and the datasheet says this), and the transition from RX-to-TX is always faster than StbyRC-to-TX. |
CapnBry
left a comment
There was a problem hiding this comment.
This was a great team effort on the optimizations and root cause analysis of the jitter 😍
I'd like to also take this opportunity to clean up the LBT interface with some clearer naming and more NOOP implementations to remove #ifdefs in rx_main/tx_main
- getRXWaitTime() -> remove, no longer needed
- EnableLBT() -> LbtEnableIfRequired()
- SetClearChannelAssessmentTime() -> LbtCcaTimerStart()
- Add NOOP implementation for non-LBA to remove #ifdefs
- ChannelIsClear() -> LbtChannelIsClear()
- Add NOOP implementation
- LBTEnabled -> LbtIsEnabled
- Add constexpr for non-LBT
- Remove ICACHE_RAM_ATTR from SpreadingFactorToRSSIvalidDelayUs(). Not called from ISR
- Move POWERMGNT.h to implementation
- Add static to all ISM LBT implementations
- transmittingRadio &= LbtChannelIsClear(transmittingRadio) & not needed, it always returns a subset of the passed radios
I have tested with these changes for both ISM and LBT. There's no code changes apart from the last item, just renames, and can push if you agree.
Yes, please push |
CapnBry
left a comment
There was a problem hiding this comment.
I just see those two macros left to remove if you want to grab them.
Tested before the PR with:
3hr with the first TX on 50Hz gemini 2.4GHz, no issues
2hr TX switched to K1000 2.4 Gemini, no issues
2hr TX switched to 500Hz Lora 2.4 SwitchAnt, no issues (15 minutes on gemini as well)
1hr TX switched to K1000 Full 900 SwitchAnt, terrible reception (~70-80LQ), but no issue
Then about an hour of me trying random packet rates, telemetry rates, antenna modes, all no obvious issues
Tested again after the PR with the same combos, but only about an hour each. I changed the packet rate and forced the connection to 1:2 TLM a lot on each handset and had no issues (after I disabled DEBUG_LOG). Also tested for an hour with SX1280 single TX (boxer) to LR1121 dual RX (BAYAK).
mha1
left a comment
There was a problem hiding this comment.
Thanks for time, collaboration and testing. Great work!
Two things in this PR.
This optimises LBT on the transmitter side slightly