Fix MSP-over-telemetry multi-frame request assembly

[b14ckyy]

Summary

One-line fix in initSharedMsp() to initialize the response buffer as empty, preventing the response-pending check in handleMspFrame() from permanently blocking multi-frame MSP request assembly.

Root Cause

initSharedMsp() initialized the response buffer with buf.end pointing to the end of the full buffer:

responsePacket->buf.end = responseBuffer + sizeof(mspTxBuffer);  // e.g. 512 bytes

The response-pending check in handleMspFrame() uses sbufBytesRemaining() (end - ptr) to detect unsent response data:

if (sbufBytesRemaining(&mspPackage.responsePacket->buf) > 0) {
    mspStarted = 0;
}

After initSharedMsp() runs, sbufBytesRemaining() returns the full buffer size (512) rather than 0, because ptr and end span the entire buffer. The check misinterprets this as a pending response, resets mspStarted to 0, which triggers initSharedMsp() again on the next frame — creating a permanent cycle that drops every continuation frame.

Fix

-    mspPackage.responsePacket->buf.end = mspPackage.responseBuffer + sizeof(mspTxBuffer);
+    mspPackage.responsePacket->buf.end = mspPackage.responseBuffer;

Initialize the response buffer as empty (ptr == end) so sbufBytesRemaining() returns 0 when no response is actually pending. This is safe because processMspPacket() already sets up the response buffer with full capacity before writing any response.

Impact

All MSPv1/MSPv2 requests requiring more than one telemetry frame are silently dropped over SmartPort, CRSF, and FPort MSP passthrough. Only zero-payload single-frame requests (e.g. MSP2_INAV_STATUS) work because they complete in a single frame without needing continuation.

Testing

Verified on MATEKF765 with SmartPort MSP passthrough using diagnostic logging in handleMspFrame():

• Before fix: Every continuation frame was dropped — sbufBytesRemaining() returned 512 after each initSharedMsp() call, causing mspStarted to be reset before the continuation could be processed
• After fix: Multi-frame MSPv2 requests are correctly reassembled and dispatched to mspFcProcessCommand()

[github-actions]

Branch Targeting Suggestion

You've targeted the master branch with this PR. Please consider if a version branch might be more appropriate:

• maintenance-9.x - If your change is backward-compatible and won't create compatibility issues between INAV firmware and Configurator 9.x versions. This will allow your PR to be included in the next 9.x release.

• maintenance-10.x - If your change introduces compatibility requirements between firmware and configurator that would break 9.x compatibility. This is for PRs which will be included in INAV 10.x

If master is the correct target for this change, no action is needed.

---

This is an automated suggestion to help route contributions to the appropriate branch.

[qodo-code-review]

Review Summary by Qodo

Fix MSP-over-telemetry multi-frame request assembly deadlock

🐞 Bug fix

Walkthroughs

Description

• Remove response-pending check causing multi-frame MSP deadlock
• Fixes MSP requests over SmartPort, CRSF, FPort telemetry
• Prevents silent failures for payloads exceeding single frame
• Eliminates redundant buffer reinitialization logic

Diagram

flowchart LR
  A["Multi-frame MSP Request"] --> B["START Frame Arrives"]
  B --> C["mspStarted = 1"]
  C --> D["CONT Frame Arrives"]
  D --> E["Response-pending Check Removed"]
  E --> F["Frame Processed Successfully"]
  F --> G["Request Reassembled"]

Loading

File Changes

1. src/main/telemetry/msp_shared.c 🐞 Bug fix +0/-4

Remove response-pending check causing assembly deadlock

• Removed 4-line response-pending check in handleMspFrame() function
• Check was resetting mspStarted flag on every incoming telemetry frame
• Eliminated deadlock cycle that prevented continuation frames from processing
• Redundant logic already handled by initSharedMsp() buffer reinitialization

src/main/telemetry/msp_shared.c

---

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

Great, no issues found!

Qodo reviewed your code and found no material issues that require review

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

[github-actions]

Test firmware build ready — commit b29f60c

Download firmware for PR #11478

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.

[error414]

caused by #11369

[KeithCoreDumped]

Hi @b14ckyy, thanks for catching this and for the fix.

I tested this change against the original TCP -> CRSF/MSP scenario that led to #11369, and it behaves correctly there as well. The crash / stability issue fixed in #11369 remains resolved without the initSharedMsp() change.

The mistake on my side came from conflating two different buffer states around sbufBytesRemaining():

• for a reader, it means unread data remaining
• for a writer, it means writable space remaining

I mistakenly applied the "writer" initialization to initSharedMsp(), which (as you found) tricked the state machine into seeing a pending response.

That said, the same full-capacity initialization should still stay in processMspPacket(). At that point the response buffer is intentionally being prepared as a write buffer for mspFcProcessCommand(). If end is not moved to the end of mspTxBuffer there, the writable space is effectively zero, which is what caused the response construction / stability problem I originally fixed in #11369.

So reverting only the initSharedMsp() response-buffer initialization is the right fix here. LGTM.