Fix MSPv2 over SmartPort: repeated START frames after header-only first chunk #14749

robthomson · 2025-11-04T13:38:23Z

Summary

This PR fixes an issue where MSPv2 telemetry over SmartPort would continuously resend the first “START” frame and never send continuation frames, preventing MSPv2 replies from completing.

Root Cause

SmartPort limits each telemetry frame to 6 bytes.
When sendMspReply() sends the first MSPv2 reply frame, that frame often contains only the 6-byte header (no payload bytes).

The function previously used:

if (responsePacket.buf.ptr == responseBuffer)

to detect whether the current chunk is the first frame.
Because the header-only case doesn’t advance the buffer pointer, subsequent calls still satisfy the condition — every chunk is treated as “first”, so the firmware keeps setting the MSP_STATUS_START_MASK bit.

Fix

A minimal fix introduces a small static flag (headerSent) to track whether the header has already been emitted:

static bool headerSent = false;

if (!headerSent) {
    // First frame: add START flag + header
    ...
    headerSent = true;
} else {
    // Continuation: no START flag
    ...
}

...

// Reset flag after final chunk
headerSent = false;

This ensures only the first frame of a response has the START flag, even if it carried no payload bytes.

Verification

Transport	MSP Version	Result
SmartPort	MSPv1	✅ Unchanged - checked
SmartPort	MSPv2	✅ Fixed – now sends continuation frames and completes replies
CRSF	MSPv1	✅ Unchanged - checked
CRSF	MSPv2	✅ Unchanged - checked

Logs now show:

RXv2 start … continuation expected
RXv2 continuation … collected=3/3
RXv2 complete

Impact

Fixed: MSPv2 over SmartPort telemetry now functions correctly
No change: MSPv1 or CRSF behavior
Low risk: The fix only affects chunk sequencing logic

Summary by CodeRabbit

Bug Fixes
- Improved telemetry response frame handling to ensure proper header emission and frame sequencing.
- Enhanced reliability of telemetry data transmission by refining frame state management.

github-actions · 2025-11-04T13:38:32Z

Do you want to test this code? You can flash it directly from the Betaflight App:

Simply put #14749 (this pull request number) in the Select commit field in the Firmware Flasher tab (you need to Enable expert mode, Show release candidates and Development).

WARNING: It may be unstable. Use only for testing!

coderabbitai · 2025-11-04T13:38:45Z

Walkthrough

Modifies sendMspReply function to track MSP response header emission using a new static headerSent state variable instead of pointer comparison. Replaces first-frame detection logic and adds proper state reset after response completion.

Changes

Cohort / File(s)	Change Summary
MSP Response Header State Tracking `src/main/telemetry/msp_shared.c`	Introduces static `headerSent` variable to replace responseBuffer pointer-based frame detection. Updates first-frame logic to set `headerSent` flag after writing header and use start flag. Subsequent frames write non-start headers with sequence/version. Resets `headerSent` to false after response completion. Adjusts comment formatting.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Review the state machine logic for header emission to ensure headerSent correctly tracks first-frame state across responses
Verify that the reset of headerSent to false after response completion prevents frame sequencing issues in subsequent responses
Confirm the start flag behavior and sequence/version combination for first vs. subsequent frames aligns with MSP protocol requirements

Suggested labels

RN: BUGFIX

Suggested reviewers

haslinghuis
nerdCopter
blckmn
freasy
ledvinap

Pre-merge checks and finishing touches

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately describes the primary fix: it specifies the exact issue (repeated START frames after header-only first chunk) and the affected component (MSPv2 over SmartPort).
Description check	✅ Passed	The description provides comprehensive information including root cause analysis, the fix with code examples, verification across multiple transport/protocol combinations, and impact assessment.

✨ Finishing touches

📝 Generate docstrings

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

coderabbitai

Actionable comments posted: 0

🧹 Nitpick comments (1)

src/main/telemetry/msp_shared.c (1)
327-327: Consider resetting headerSent in processMspPacket and sendMspErrorResponse for robustness.

The flag is correctly reset here after completing a response. However, both processMspPacket() (line 144) and sendMspErrorResponse() (line 162) reset the response buffer pointer without resetting headerSent. While the protocol assumes sequential request-response cycles, defensively resetting the flag when initializing a new response would prevent potential state corruption if a new request arrives unexpectedly.

Add the reset at the beginning of processMspPacket:
static void processMspPacket(void)
{
    responsePacket.cmd = 0;
    responsePacket.result = 0;
    responsePacket.buf.ptr = responseBuffer;
    responsePacket.buf.end = ARRAYEND(responseBuffer);
+   headerSent = false;  // Reset header state for new response
And similarly in sendMspErrorResponse:
static void sendMspErrorResponse(uint8_t error, int16_t cmd)
{
    responsePacket.cmd = cmd;
    responsePacket.result = 0;
    responsePacket.buf.ptr = responseBuffer;
+   headerSent = false;  // Reset header state for new response
Note: headerSent would need to be moved outside sendMspReply() or declared at file scope for these functions to access it.

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between af70928 and 8dcf5b6.

📒 Files selected for processing (1)

src/main/telemetry/msp_shared.c (3 hunks)

🧰 Additional context used

🧠 Learnings (1)

📓 Common learnings

Learnt from: ledvinap
Repo: betaflight/betaflight PR: 14578
File: src/main/telemetry/jetiexbus.c:547-556
Timestamp: 2025-08-12T18:26:17.016Z
Learning: For Betaflight Jeti ExBus backport PR #14578: ledvinap prefers to keep the PR focused on backporting fixes from iNavFlight rather than expanding into broader architectural improvements. Additional improvements to TX/RX state machine should be handled in separate PRs.

🧬 Code graph analysis (1)

src/main/telemetry/msp_shared.c (1)

src/main/common/streambuf.c (2)

sbufInit (29-34)

sbufSwitchToReader (153-157)

🔇 Additional comments (5)

src/main/telemetry/msp_shared.c (5)

274-274: LGTM: Static flag appropriately tracks header state.

The static boolean flag correctly maintains state across chunks and is properly initialized to false.

281-281: Core fix correctly addresses the root cause.

Replacing the pointer-based detection with an explicit headerSent flag properly handles the case where the first SmartPort chunk contains only the 6-byte header and no payload. The previous pointer comparison failed because the buffer pointer wasn't advanced when no payload bytes were written.

306-309: Correct flag management and continuation header logic.

Setting headerSent = true after emitting the header ensures subsequent calls generate continuation frames without the START flag, properly implementing the chunk sequencing protocol.

312-312: LGTM: Minor formatting cleanup.

Also applies to: 324-324

271-329: Well-structured fix with minimal scope.

The implementation correctly addresses the SmartPort MSPv2 header-only first chunk issue while maintaining backward compatibility with MSPv1, MSPv2 over CRSF, and other telemetry protocols. The logic properly handles partial sends and response completion.

robthomson · 2025-11-04T13:48:04Z

This code has been derived and tested on rotorflight

rotorflight/rotorflight-firmware#344

The entire RFsuite stack works perfectly with this fix using mspv2 as the transport - on crsf and sport.

Its also about to make it into the inav tree.

Fix msp

8dcf5b6

coderabbitai bot reviewed Nov 4, 2025

View reviewed changes

coderabbitai bot approved these changes Nov 4, 2025

View reviewed changes

haslinghuis assigned robthomson Nov 4, 2025

haslinghuis added the RN: BUGFIX label Nov 4, 2025

haslinghuis added this to 2025.12.0 Nov 4, 2025

haslinghuis moved this to Bugfix in 2025.12.0 Nov 4, 2025

haslinghuis added this to the 2025.12 milestone Nov 4, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

Fix MSPv2 over SmartPort: repeated START frames after header-only first chunk #14749

Fix MSPv2 over SmartPort: repeated START frames after header-only first chunk #14749

robthomson commented Nov 4, 2025 •

edited by coderabbitai bot

Loading

Uh oh!

github-actions bot commented Nov 4, 2025

Uh oh!

coderabbitai bot commented Nov 4, 2025 •

edited

Loading

Uh oh!

coderabbitai bot left a comment

Uh oh!

robthomson commented Nov 4, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Fix MSPv2 over SmartPort: repeated START frames after header-only first chunk #14749

Are you sure you want to change the base?

Fix MSPv2 over SmartPort: repeated START frames after header-only first chunk #14749

Conversation

robthomson commented Nov 4, 2025 • edited by coderabbitai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Root Cause

Fix

Verification

Impact

Tags

Summary by CodeRabbit

Uh oh!

github-actions bot commented Nov 4, 2025

Uh oh!

coderabbitai bot commented Nov 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Suggested labels

Suggested reviewers

Pre-merge checks and finishing touches

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

robthomson commented Nov 4, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

robthomson commented Nov 4, 2025 •

edited by coderabbitai bot

Loading

coderabbitai bot commented Nov 4, 2025 •

edited

Loading