Fix MSP_RESULT_NO_REPLY ignored in MSP-over-telemetry passthrough

[b14ckyy]

Problem

processMspPacket() in msp_shared.c ignores the MSP_RESULT_NO_REPLY return value from mspFcProcessCommand(). When a client sends an MSPv2 command with the MSP_FLAG_DONT_REPLY flag (bit 0 of the flags byte), the FC correctly evaluates the result as MSP_RESULT_NO_REPLY, but processMspPacket() unconditionally calls sbufSwitchToReader() on the response buffer — causing a reply to be sent anyway.

This wastes bandwidth on half-duplex telemetry links (SmartPort, CRSF, FPort) and violates the MSPv2 protocol contract. In constrained passthrough scenarios (e.g. Lua scripts sending fire-and-forget SET commands), the unnecessary reply frames can cause congestion and slow down subsequent requests.

Root Cause

The serial MSP path in msp_serial.c already handles this correctly:

if (status != MSP_RESULT_NO_REPLY) {
    // ... send reply
}

The telemetry passthrough path in msp_shared.c was missing this check — the return value of mspFcProcessCommand() was discarded.

Fix

Store the return value of mspFcProcessCommand() and check for MSP_RESULT_NO_REPLY. When the result is NO_REPLY, set the response buffer to empty (buf.ptr = buf.end) instead of calling sbufSwitchToReader(), so no reply is transmitted.

Affected Paths

• SmartPort MSP passthrough
• CRSF MSP passthrough
• FPort MSP passthrough

[qodo-code-review]

Review Summary by Qodo

Fix MSP_RESULT_NO_REPLY ignored in MSP-over-telemetry passthrough

🐞 Bug fix

Walkthroughs

Description

• Fix MSP_RESULT_NO_REPLY being ignored in telemetry passthrough
• Store mspFcProcessCommand() return value for proper status checking
• Skip reply transmission when MSP_FLAG_DONT_REPLY flag is set
• Prevents unnecessary bandwidth waste on half-duplex telemetry links

Diagram

flowchart LR
  A["mspFcProcessCommand()"] -- "returns status" --> B["Check status value"]
  B -- "MSP_RESULT_NO_REPLY" --> C["Empty response buffer"]
  B -- "Other status" --> D["Send reply via sbufSwitchToReader"]
  C --> E["No reply transmitted"]
  D --> E

Loading

File Changes

1. src/main/telemetry/msp_shared.c 🐞 Bug fix +8/-2

Handle MSP_RESULT_NO_REPLY in processMspPacket

• Store return value from mspFcProcessCommand() in status variable
• Check if status equals MSP_RESULT_NO_REPLY before sending reply
• When NO_REPLY, set response buffer to empty instead of calling sbufSwitchToReader()
• Aligns telemetry passthrough behavior with serial MSP path handling

src/main/telemetry/msp_shared.c

---

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX Issues (0)

1. NO_REPLY still sends reply 🐞 Bug ≡ Correctness

Description

processMspPacket() empties the response buffer for MSP_RESULT_NO_REPLY, but handleMspFrame() still
returns true, causing SmartPort/CRSF to call sendMspReply(), which always emits at least a telemetry
header chunk and calls responseFn(). This means MSP_FLAG_DONT_REPLY still produces an on-wire reply
frame (size=0), so the bandwidth/protocol goal is not achieved.

Code

src/main/telemetry/msp_shared.c[R95-100]

+    if (status == MSP_RESULT_NO_REPLY) {
+        mspPackage.responsePacket->buf.ptr = mspPackage.responseBuffer;
+        mspPackage.responsePacket->buf.end = mspPackage.responseBuffer;
+    } else {
+        sbufSwitchToReader(&mspPackage.responsePacket->buf, mspPackage.responseBuffer);
+    }

Evidence

Even with an empty tx buffer (ptr==end), sendMspReply() still constructs and transmits a response
chunk because it always writes the telemetry header before checking txBuf bytes. Since
SmartPort/CRSF schedule replies based on handleMspFrame()'s boolean return (which is always true
once a full request is assembled), a NO_REPLY request still results in sendMspReply() being invoked
and sending a header-only frame.

src/main/telemetry/msp_shared.c[79-101]
src/main/telemetry/msp_shared.c[199-205]
src/main/telemetry/msp_shared.c[207-264]
src/main/telemetry/smartport.c[448-476]
src/main/telemetry/crsf.c[108-130]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
Telemetry MSP-over-telemetry still sends a response frame for `MSP_RESULT_NO_REPLY` because callers trigger `sendMspReply()` whenever `handleMspFrame()` returns `true`, and `sendMspReply()` always serializes a header even when the response buffer is empty.

### Issue Context
`processMspPacket()` now empties `responsePacket->buf` on `MSP_RESULT_NO_REPLY`, but this does not prevent reply scheduling/transmission.

### Fix Focus Areas
- src/main/telemetry/msp_shared.c[114-205]
- src/main/telemetry/msp_shared.c[207-264]
- src/main/telemetry/smartport.c[448-476]
- src/main/telemetry/crsf.c[108-130]

### Suggested fix
Implement an explicit NO_REPLY gate so no telemetry frame is emitted:
- Option A (preferred): Change `handleMspFrame()` to return `false` when `mspPackage.responsePacket->result == MSP_RESULT_NO_REPLY` after `processMspPacket()`, so SmartPort/CRSF never set `replyPending`.
- Option B: Add an early return in `sendMspReply()`:
 - if `mspPackage.responsePacket->result == MSP_RESULT_NO_REPLY`, ensure any internal chunking state (e.g. `headerSent`) is reset appropriately, then return `false` without calling `responseFn()`.

Either option should ensure that `MSP_FLAG_DONT_REPLY` produces no on-wire reply at all (not even a header-only chunk).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

ⓘ The new review experience is currently in Beta. Learn more

[github-actions]

Test firmware build ready — commit c74edd0

Download firmware for PR #11481

228 targets built. Find your board's .hex file by name on that page (e.g. MATEKF405SE.hex). Files are individually downloadable — no GitHub login required.

Development build for testing only. Use Full Chip Erase when flashing.

[sensei-hacker]

The fix in c74edd0 correctly addresses the bot's concern. Here is the trace:

processMspPacket() now returns false when mspFcProcessCommand() returns MSP_RESULT_NO_REPLY, and handleMspFrame() now propagates that return value directly (return processMspPacket()).

Both callers gate sendMspReply() on the return value of handleMspFrame():

• SmartPort (smartport.c:452): smartPortMspReplyPending = handleMspFrame(...) — with false returned, the if (smartPortMspReplyPending) block at line 472 is never entered, so sendMspReply() is never called.
• CRSF (crsf.c:124): if (handleMspFrame(...)) — the body is skipped entirely when false is returned, replyPending stays false, and sendMspReply() is never called.

This is the bot's preferred Option A: sendMspReply() is not called at all for NO_REPLY requests — no header-only frame emitted, no responseFn() invoked, no bandwidth consumed. The fix is solid.