MSP2 via CSFR/SmartPort telemetry

[error414]

User description

PR adds support for MSP2 inspired by implementation from betaflight. PR does not change manupulation with buffers and keep INAV style. In PR is fix for mspRequestOriginID originally made Pawel

1. from MSP and MSP2 via CSFR has been removed MSP CRC, because CRSF contains itself CRC, so no need to duplicate
2. CRSF MSP frame <receiver address 8b><sender address 8b><csfr flags 8b><length 8b><cmd 8b><payload ....>
3. CRSF MSP2 frame
   <receiver address 8b><sender address 8b><csfr flags 8b><MSP flags 8b><cmd 16b LE><length 16b LE><payload ... >
4. JUMBO frames are not supported
5. simple test LUA files for MSP and MSP2 are attached, scripts has no any GUI, only send data over VSP from transmitter
   example:

RF2: Sending MSP cmd 1
RF2: TX (6 B)
RF2:   [1]:  0xC8
RF2:   [2]:  0xEA
RF2:   [3]:  0x30
RF2:   [4]:  0x0
RF2:   [5]:  0x1
RF2:   [6]:  0x1
RF2: -----
RF2: cmd:0x7B
RF2:   data length: 60
RF2:   [1]:  0xEA
RF2:   [2]:  0xC8
RF2:   [3]:  0x32
RF2:   [4]:  0x3
RF2:   [5]:  0x1
RF2:   [6]:  0x8
RF2:   [7]:  0x9
RF2:   [8]:  0x9

Tasks:

• CRSF MSP1
• CRSF MSP2
• CRSF MSP1 >60B
• CRSF MSP2 >60B
• Smartport MSP1
• Smartport MSP2
• create unit test (optional)
• apply fix , I don't know what the fix fixed and if it's needed (optional)

---

---

Testing:

CRSF (ready for test):

• test MSP1 simple frame REQUEST
• test MSP1 simple frame RESPONSE
• test MSP1 frame <60B REQUEST
• test MSP1 frame <60B RESPONSE
• test MSP2 simple frame REQUEST
• test MSP2 simple frame RESPONSE
• test MSP2 frame <60B REQUEST
• test MSP2 frame <60B RESPONSE

---

SmartPort:

• test MSP1 simple frame REQUEST
• test MSP1 simple frame RESPONSE
• test MSP1 frame <60B REQUEST
• test MSP1 frame <60B RESPONSE
• test MSP2 simple frame REQUEST
• test MSP2 simple frame RESPONSE
• test MSP2 frame <60B REQUEST
• test MSP2 frame <60B RESPONSE

---

MSP-test-lua.zip

---

PR Type

Enhancement, New Feature

---

Description

• Add MSP2 protocol support over CRSF and SmartPort telemetry

• Implement chunked frame handling for payloads exceeding 60 bytes

• Fix MSP request origin ID tracking for proper response routing

• Refactor MSP frame parsing with improved version detection and error handling

---

Diagram Walkthrough

flowchart LR
  A["MSP Request<br/>CRSF/SmartPort"] --> B["Parse MSP Frame<br/>v1 or v2"]
  B --> C["Handle Chunked<br/>Payloads"]
  C --> D["Process MSP<br/>Command"]
  D --> E["Generate MSP<br/>Response"]
  E --> F["Send Response<br/>with Origin ID"]

Loading

File Walkthrough

	Relevant files
Enhancement	crsf.c Improve CRSF timing and MSP frame handling                              src/main/rx/crsf.c Increased CRSF frame timing from 1100µs to 1750µs for ad-hoc requests Renamed crsfFrameStartAt to crsfFrameStartAtUs for clarity Changed time measurement from micros() to microsISR() for ISR context Updated MSP frame buffering to use actual frame length instead of fixed size Added crsfScheduleMspResponse() parameter for request origin ID tracking Added crsfRxIsTelemetryBufEmpty() function to check telemetry buffer state +13/-8    crsf.h Add telemetry buffer status check function                              src/main/rx/crsf.h Added crsfRxIsTelemetryBufEmpty() function declaration +1/-0      crsf.c Implement MSP response chunking and origin ID tracking      src/main/telemetry/crsf.c Implement reply pending state machine to handle chunked MSP responses Wait for telemetry buffer to empty before sending MSP reply Track MSP request origin ID for correct response addressing Update crsfScheduleMspResponse() to accept and store request origin ID Modify crsfSendMspResponse() to accept dynamic payload size parameter Add telemetry buffer check in processCrsf() to prevent frame collision +28/-9    crsf.h Update MSP response scheduling signature                                  src/main/telemetry/crsf.h Update crsfScheduleMspResponse() signature to include requestOriginID parameter +1/-1      msp_shared.h Update MSP callback and function signatures                            src/main/telemetry/msp_shared.h Update mspResponseFnPtr callback signature to include payload size parameter Update handleMspFrame() parameter naming for clarity +2/-2      smartport.c Add payload size handling to SmartPort MSP response            src/main/telemetry/smartport.c Update smartPortSendMspResponse() to accept payload size parameter Use MIN() macro to safely copy data within SmartPort payload bounds +2/-2
New feature	msp_shared.c Add MSPv2 protocol support and chunked frame handling        src/main/telemetry/msp_shared.c Add MSPv2 protocol support with 16-bit command and size fields Refactor frame parsing to handle both MSPv1 and MSPv2 formats Implement proper version detection and validation Add buffer overflow protection with size validation Remove CRC calculation (handled by CRSF/SmartPort layer) Implement chunked response handling with header sent tracking Add detailed byte position constants for frame structure Update error handling with new error types for version mismatch and oversized requests +121/-87

---

[MrD-RC]

Awesome work. Will SmartPort also get the MSPv2 treatment?

[error414]

it should work, the changes for MSP2 are in msp_share file, so theoretically i don't see any reasson why it should not work.

rob.thomson is going to test it, he has smarport capable gear.

[MrD-RC]

I did have a brief look. There may be some things that need changing in SmartPort regarding the packets. I believe there are examples of this in the BetaFlight or RotorFlight SmartPort.c file.

Awesome work though 👍🏻

[error414]

do you mean change regarding to skip few request after EEPROM_WRITE command? I will look at that closely :)

[MrD-RC]

Don’t listen to me, I was misremembering 🤣 The changes in msp_shared should hopefully get it work over SmartPort.

[error414]

:D, it seems you were right, still some issues with smartport, I'm waiting to rob.thomson for more info. And moreover I will have frsky gear for better smartport testing.

However, CRSF works much better, I was able to fetch VERSION frame, PID frame, and save new values for PID frame. there were some insidious bugs :D

[MrD-RC]

Awesome 🤘🏻 Rob mentioned that FrSky are sending you some hardware to get this working. Which is great news.

[error414]

current status is that CRSF is fully functional. I tested all scenarios from "Testing:" section. Rob.thomson confirmed that CRSF is OK,
So if anybody would like to test it as well, he is welcome :)

Smartport is still in progress, Maybe what about to create for smartport new pullrequest?

[error414]

Tested by rob.thomson

[qodo-code-review]

PR Compliance Guide 🔍

(Compliance updated until commit 26382bf)

Below is a summary of compliance checks for this PR:

Security Compliance
🟢	No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
⚪	🎫 No ticket provided Create ticket/issue
Codebase Duplication Compliance
⚪	Codebase context is not defined Follow the guide to enable codebase context checks.
Custom Compliance
🟢	Generic: Meaningful Naming and Self-Documenting Code Objective: Ensure all identifiers clearly express their purpose and intent, making code self-documenting Status: Passed
	Generic: Secure Error Handling Objective: To prevent the leakage of sensitive system information through error messages while providing sufficient detail for internal debugging. Status: Passed
	Generic: Secure Logging Practices Objective: To ensure logs are useful for debugging and auditing without exposing sensitive information like PII, PHI, or cardholder data. Status: Passed
⚪	Generic: Comprehensive Audit Trails Objective: To create a detailed and reliable record of critical system actions for security analysis and compliance. Status: No audit logs: Newly added MSP-over-telemetry handling and response scheduling introduce critical protocol actions without any logging of requests, origins, or outcomes to enable auditing. Referred Code { static uint8_t mspStarted = 0; static uint8_t lastSeq = 0; if (sbufBytesRemaining(&mspPackage.responsePacket->buf) > 0) { mspStarted = 0; } if (mspStarted == 0) { initSharedMsp(); } if (payloadLength < MIN_LENGTH_CHUNK) { return false; // prevent analyzing garbage data } mspPacket_t *requestPacket = mspPackage.requestPacket; const uint8_t status = frameStart[MSP_INDEX_STATUS]; const uint8_t seqNumber = status & TELEMETRY_MSP_SEQ_MASK; lastRequestVersion = (status & TELEMETRY_MSP_VER_MASK) >> TELEMETRY_MSP_VER_SHIFT; ... (clipped 70 lines)
	Generic: Robust Error Handling and Edge Case Management Objective: Ensure comprehensive error handling that provides meaningful context and graceful degradation Status: Partial edge handling: While new code validates lengths, versions, and sequence continuity, it removes CRC validation and relies on external CRC, which may be acceptable for CRSF but lacks explicit fallback or logging when frames arrive out of order or buffers are busy. Referred Code if (payloadLength < MIN_LENGTH_CHUNK) { return false; // prevent analyzing garbage data } mspPacket_t *requestPacket = mspPackage.requestPacket; const uint8_t status = frameStart[MSP_INDEX_STATUS]; const uint8_t seqNumber = status & TELEMETRY_MSP_SEQ_MASK; lastRequestVersion = (status & TELEMETRY_MSP_VER_MASK) >> TELEMETRY_MSP_VER_SHIFT; if (lastRequestVersion > TELEMETRY_MSP_VERSION) { sendMspErrorResponse(TELEMETRY_MSP_VER_MISMATCH, 0); return true; } if (status & TELEMETRY_MSP_START_MASK) { // first packet in sequence uint16_t mspPayloadSize; if (lastRequestVersion == 1) { // MSPv1 mspPayloadSize = frameStart[MSP_INDEX_SIZE_V1]; ... (clipped 36 lines)
	Generic: Security-First Input Validation and Data Handling Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent vulnerabilities Status: External input trust: The new MSPv2 handling trusts CRSF/SmartPort transport CRC and accepts payloads based on size checks without additional authentication or integrity checks at this layer, which may be acceptable but is not explicit in code. Referred Code mspPayloadSize = frameStart[MSP_INDEX_SIZE_V1]; requestPacket->cmd = frameStart[MSP_INDEX_ID_V1]; sbufInit(&mspPackage.requestFrame, frameStart + MSP_INDEX_PAYLOAD_V1, frameStart + payloadLength); } else { // MSPv2 if (payloadLength < MIN_LENGTH_REQUEST_V2) { return false; // prevent analyzing garbage data } requestPacket->flags = frameStart[MSP_INDEX_FLAG_V2]; requestPacket->cmd = *(uint16_t *) &frameStart[MSP_INDEX_ID_LO]; mspPayloadSize = *(uint16_t *) &frameStart[MSP_INDEX_SIZE_V2_LO]; sbufInit(&mspPackage.requestFrame, frameStart + MSP_INDEX_PAYLOAD_V2, frameStart + payloadLength); } if (mspPayloadSize <= sizeof(mspRxBuffer)) { // prevent buffer overrun requestPacket->result = 0; requestPacket->buf.ptr = mspPackage.requestBuffer; requestPacket->buf.end = mspPackage.requestBuffer + mspPayloadSize; mspStarted = 1; } else { // this MSP packet is too big to fit in the buffer. sendMspErrorResponse(TELEMETRY_MSP_REQUEST_IS_TOO_BIG, mspPackage.requestPacket->cmd); ... (clipped 6 lines)

Compliance status legend

🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

---

Previous compliance checks

Compliance check up to commit 779fccf

[qodo-code-review]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Correct MSP frame length calculation Correct the MSP frame length calculation in bufferCrsfMspFrame by changing crsfFrame.frame.frameLength - 4 to crsfFrame.frame.frameLength - 3 to prevent truncating MSP data. src/main/rx/crsf.c [177-179] -if (bufferCrsfMspFrame(frameStart, crsfFrame.frame.frameLength - 4)) { +if (bufferCrsfMspFrame(frameStart, crsfFrame.frame.frameLength - 3)) { crsfScheduleMspResponse(crsfFrame.frame.payload[1]); } Apply / Chat Suggestion importance[1-10]: 9 __ Why: The suggestion correctly identifies a bug in the MSP payload length calculation, which would cause the last byte of the MSP data to be truncated, leading to data corruption.	High
High-level	Refactor state management for chunked replies The state for chunked MSP replies is currently distributed across static variables in crsf.c and msp_shared.c. This should be refactored by encapsulating the entire response state into a single structure or state machine within msp_shared.c for better robustness. Examples: src/main/telemetry/crsf.c [110-116] static bool replyPending = false; if (replyPending) { if (crsfRxIsTelemetryBufEmpty()) { replyPending = sendMspReply(payloadSize, responseFn); } return replyPending; } src/main/telemetry/msp_shared.c [202-203] static uint8_t seq = 0; static bool headerSent = false; Solution Walkthrough: Before: // src/main/telemetry/crsf.c bool handleCrsfMspFrameBuffer(...) { static bool replyPending = false; if (replyPending) { if (crsfRxIsTelemetryBufEmpty()) { replyPending = sendMspReply(...); // in msp_shared.c } return replyPending; } // ... process request ... if (crsfRxIsTelemetryBufEmpty()) { replyPending = sendMspReply(...); } // ... } // src/main/telemetry/msp_shared.c bool sendMspReply(...) { static uint8_t seq = 0; static bool headerSent = false; if (!headerSent) { // ... write header ... headerSent = true; } // ... write payload chunk ... if (last_chunk) { headerSent = false; return false; // No more chunks } return true; // More chunks pending } After: // src/main/telemetry/msp_shared.h typedef struct { bool responsePending; bool headerSent; uint8_t seq; // ... other state ... } mspResponseState_t; // src/main/telemetry/msp_shared.c static mspResponseState_t responseState; void msp_shared_init_response() { /* ... */ } bool msp_shared_send_chunk(mspResponseFnPtr responseFn) { if (!responseState.responsePending || !is_transport_ready()) { return responseState.responsePending; } if (!responseState.headerSent) { // ... write header ... responseState.headerSent = true; } // ... write payload chunk ... if (last_chunk) { responseState.responsePending = false; responseState.headerSent = false; } return responseState.responsePending; } // src/main/telemetry/crsf.c void processCrsf() { // ... msp_shared_send_chunk(crsfSendMspResponse, crsfRxIsTelemetryBufEmpty); // ... } Suggestion importance[1-10]: 8 __ Why: The suggestion correctly identifies a significant design weakness where state for chunked replies is scattered across multiple files and static variables, leading to complex and fragile logic. Centralizing this state would greatly improve the maintainability and robustness of this core new feature.	Medium
Update

[error414]

Correct MSP frame length calculation

-4 is there becasue CRSF FRAME contains 4 bytes related to CRSF , rest is payload, do we need alocated extra one byte, just for sure?

 * Device address: (uint8_t)
 * Frame length:   length in  bytes including Type (uint8_t)
 * Type:           (uint8_t)
 * CRC:            (uint8_t)

Refactor state management for chunked replies

personaly I would keep static variables in each function for detection if header has been sent or not, seqnumber is used only in one function, and it should be last seq number. So no need to extract. Good suggestion from qodo but for beter debugging and keep it more readable I would keep it as it is. i think it does not have impact for performance.

[robthomson]

Yup. Tested crsf and sport from ethos.

All good.

Same work helped fix a sport issue with MSPv2 on betaflight and rotorflight.

[error414]

NOTE: in ERLS is bug and MSP2 does not work properly, it's fixed in ELRS 3.6.0, so for using MSP2 it's needed to update ELRS TX/RX