CRSF: Allow future use of unused bits in the "Arming status byte" - #7022
Open
mha1 wants to merge 6 commits into
Open
CRSF: Allow future use of unused bits in the "Arming status byte"#7022mha1 wants to merge 6 commits into
mha1 wants to merge 6 commits into
Conversation
gagarinlg
reviewed
May 18, 2026
gagarinlg
left a comment
gagarinlg
left a comment
Member
There was a problem hiding this comment.
what is this part of the code for?
There was a problem hiding this comment.
Pull request overview
This PR updates CRSF channel frame generation so the status byte is always appended, enabling arming-mode signaling while leaving higher bits available for future status flags.
Changes:
- Always sends a 25-byte CRSF channel frame payload including the extra status byte.
- Encodes Switch-mode armed state in bit 0 and CH5 arming mode in bit 1.
- Updates CRC calculation to include the always-present status byte.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthrough
ChangesCrossfire Frame Status Byte Integration
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Replaces the "TODO check" placeholder with real coverage of the wire format: the spec-defined 0x16 channel packing (verified against independently computed expected bytes), and the ExpressLRS arming status-byte extension for both CH5 and Switch arming modes (length, status bits, CRC), matching the fixed 25-byte frame introduced in this PR. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
pfeerick
force-pushed
the
PR_use_more_status_bits
branch
from
August 27, 2026 00:56
debd129 to
44a5b59
Compare
Member
|
Is this truely compatible with ELRS V3? Or do you mean 3.6.3 and later - as this seems to cause an issue for at least 3.5.3 through 3.6.2? Since not everyone updates to the new version as soon as it comes out, we can not assume they will be running 3.6.3 or later... especially if this goes into 2.11.x, for example. If the following works, should we be looking at version gating it? Only the diff --git a/radio/src/pulses/crossfire.cpp b/radio/src/pulses/crossfire.cpp
index 1cd9259c40..e1cf9780e0 100644
--- a/radio/src/pulses/crossfire.cpp
+++ b/radio/src/pulses/crossfire.cpp
@@ -101,9 +101,21 @@ uint8_t createCrossfireChannelsFrame(uint8_t moduleIdx, uint8_t * frame, int16_t
// - arming mode Switch or CH5 (bit 1)
// - bits 2-7 spare
//
+ // Gate on the module's own ELRS version - older than 4.0 (#3470)
+ // don't mask individual bits: they assign the whole byte to a bool
+ // (any nonzero -> armed). Sending 0x02 unconditionally in CH5 mode
+ // would make those modules report a spurious armed state.
+ //
+ bool fixedStatusByte = CRSF_ELRS_MIN_VER(moduleIdx, 4, 0);
+
+ ModuleData *md = &g_model.moduleData[moduleIdx];
+ uint8_t armingMode = md->crsf.crsfArmingMode; // 0 = Channel mode, 1 = Switch mode
+ uint8_t lenAdjust = (fixedStatusByte || armingMode == ARMING_MODE_SWITCH) ? 1 : 0;
+
uint8_t * buf = frame;
*buf++ = MODULE_ADDRESS;
- *buf++ = 25; // 1(ID) + 22(channel data) + 1(extra status byte) + 1(CRC)
+ *buf++ = 24 + lenAdjust; // 1(ID) + 22(channel data) + (+1 extra byte if present) + 1(CRC)
uint8_t * crc_start = buf;
*buf++ = CHANNELS_ID;
@@ -126,22 +138,25 @@ uint8_t createCrossfireChannelsFrame(uint8_t moduleIdx, uint8_t * frame, int16_t
//
// assemble status byte
//
- ModuleData *md = &g_model.moduleData[moduleIdx];
+ if (fixedStatusByte) {
+ if (armingMode == ARMING_MODE_SWITCH) {
+ swsrc_t sw = md->crsf.crsfArmingTrigger;
- if (md->crsf.crsfArmingMode == ARMING_MODE_SWITCH) {
- swsrc_t sw = md->crsf.crsfArmingTrigger;
+ *buf = (sw != SWSRC_NONE) && getSwitch(sw, 0); // commanded armed status in Switch mode
+ } else {
+ *buf = 0x02; // flag arming mode CH5
+ }
+ buf++;
+ } else if (armingMode == ARMING_MODE_SWITCH) {
+ swsrc_t sw = md->crsf.crsfArmingTrigger;
- *buf = (sw != SWSRC_NONE) && getSwitch(sw, 0); // commanded armed status in Switch mode
- } else {
- *buf = 0x02; // flag arming mode CH5
+ *buf++ = (sw != SWSRC_NONE) && getSwitch(sw, 0); // commanded armed status in Switch mode
}
- buf++;
-
//
// add crc
//
- *buf++ = crc8(crc_start, 24);
+ *buf++ = crc8(crc_start, 23 + lenAdjust);
return buf - frame;
}
diff --git a/radio/src/tests/crossfire.cpp b/radio/src/tests/crossfire.cpp
index b904a454f8..5b0d037037 100644
--- a/radio/src/tests/crossfire.cpp
+++ b/radio/src/tests/crossfire.cpp
@@ -44,6 +44,10 @@ TEST(Crossfire, createCrossfireChannelsFrame)
pulsesStart[i] = -1024 + (2048 / MAX_TRAINER_CHANNELS) * i;
}
+ // channel packing doesn't depend on arming mode or module version, but
+ // reset explicitly rather than relying on running before the tests below
+ crossfireModuleStatus[EXTERNAL_MODULE] = {0};
+
createCrossfireChannelsFrame(EXTERNAL_MODULE, crossfire, pulsesStart);
ASSERT_EQ(crossfire[0], MODULE_ADDRESS);
--- a/radio/src/tests/crossfire.cpp
+++ b/radio/src/tests/crossfire.cpp
@@ -58,9 +58,10 @@ TEST(Crossfire, createCrossfireChannelsFrame)
// Status byte after the 0x16 payload is an ExpressLRS extension, not TBS CRSF
// spec (semantics per ExpressLRS's TXModuleEndpoint.cpp / crsf_protocol.h).
-// Frame is always 25 bytes (1 ID + 22 channel data + 1 status + 1 CRC);
-// bit 0 = commanded armed status (Switch mode only), bit 1 = arming mode is CH5.
-TEST(Crossfire, ExpressLRSArmingExtension_CH5Mode)
+// The fixed 25-byte frame is only sent once the module confirms (via
+// CRSF_ELRS_MIN_VER) it's new enough to mask individual status bits (#3470);
+// older/unknown modules get the legacy variable-length frame they understand.
+TEST(Crossfire, ExpressLRSArmingExtension_CH5Mode_LegacyModule)
{
MODEL_RESET();
@@ -73,6 +74,37 @@ TEST(Crossfire, ExpressLRSArmingExtension_CH5Mode)
}
g_model.moduleData[EXTERNAL_MODULE].crsf.crsfArmingMode = ARMING_MODE_CH5;
+ crossfireModuleStatus[EXTERNAL_MODULE] = {0}; // no/unknown module version
+
+ uint8_t len = createCrossfireChannelsFrame(EXTERNAL_MODULE, crossfire, pulsesStart);
+
+ // no status byte at all for CH5 mode pre-#3470: legacy 24-byte payload
+ ASSERT_EQ(len, 26);
+ ASSERT_EQ(crossfire[0], MODULE_ADDRESS);
+ ASSERT_EQ(crossfire[1], 24);
+ ASSERT_EQ(crossfire[2], CHANNELS_ID);
+
+ uint8_t crc = crc8(&crossfire[2], 23);
+ ASSERT_EQ(crossfire[25], crc);
+}
+
+TEST(Crossfire, ExpressLRSArmingExtension_CH5Mode_ModernModule)
+{
+ MODEL_RESET();
+
+ int16_t pulsesStart[MAX_TRAINER_CHANNELS];
+ uint8_t crossfire[CROSSFIRE_FRAME_MAXLEN];
+
+ memset(crossfire, 0, sizeof(crossfire));
+ for (int i=0; i<MAX_TRAINER_CHANNELS; i++) {
+ pulsesStart[i] = -1024 + (2048 / MAX_TRAINER_CHANNELS) * i;
+ }
+
+ g_model.moduleData[EXTERNAL_MODULE].crsf.crsfArmingMode = ARMING_MODE_CH5;
+ crossfireModuleStatus[EXTERNAL_MODULE] = {0};
+ crossfireModuleStatus[EXTERNAL_MODULE].isELRS = true;
+ crossfireModuleStatus[EXTERNAL_MODULE].major = 4;
+ crossfireModuleStatus[EXTERNAL_MODULE].minor = 0;
uint8_t len = createCrossfireChannelsFrame(EXTERNAL_MODULE, crossfire, pulsesStart);
@@ -101,6 +133,8 @@ TEST(Crossfire, ExpressLRSArmingExtension_SwitchMode)
g_model.moduleData[EXTERNAL_MODULE].crsf.crsfArmingMode = ARMING_MODE_SWITCH;
g_model.moduleData[EXTERNAL_MODULE].crsf.crsfArmingTrigger = SWSRC_NONE;
+ // Switch mode already implied a status byte pre-#3470 (bit 0 only), so it
+ // gets the fixed-length frame regardless of module version.
uint8_t len = createCrossfireChannelsFrame(EXTERNAL_MODULE, crossfire, pulsesStart);
ASSERT_EQ(len, 27);
|
Contributor
Author
Yes, can you elaborate on the issue or concern? V3 doesn't have any knowledge of the extra status byte and doesn't have code to act on it. It'll read the 25byte payload but ignore the last byte (= status byte) 
Version gating in pulses/crossfire.cpp is not necessary.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR allows future use of the currently unused status bits in the byte we use to communicate the commanded arming status to ExpressLRS modules if we ever have a need for them. The changes are fully backward compatible with the existing V2.11 and V2.12 implementation for communicating the commanded arming status to ExpressLRS modules. It is also compatible with modules using ExpressLRS V3 firmware.
Summary of changes:
This complements the changes made to V4 in ExpressLRS/ExpressLRS#3470 and is fully backwards compatible to the existing implementation
Summary by CodeRabbit