Skip to content

Fix false Ground Station activation during Blackbox download - #2690

Open
mart1npetroff wants to merge 2 commits into
iNavFlight:maintenance-10.xfrom
mart1npetroff:fix/ltm-blackbox-routing
Open

Fix false Ground Station activation during Blackbox download#2690
mart1npetroff wants to merge 2 commits into
iNavFlight:maintenance-10.xfrom
mart1npetroff:fix/ltm-blackbox-routing

Conversation

@mart1npetroff

Copy link
Copy Markdown

What changed

  • Route LTM decoding only until a valid MSP stream is detected.
  • Prevent Ground Station auto-activation when MSP is active.
  • Reset MSP and LTM decoder state between connection sessions.
  • Handle incomplete LTM telemetry values safely in the Ground Station UI.
  • Add regression coverage for a checksum-valid LTM frame embedded in arbitrary data.

Root cause

The serial connection fed every received byte to both MSP and LTM decoders. Raw MSP_DATAFLASH_READ payloads contain arbitrary Blackbox bytes, so they could accidentally form a valid LTM frame and switch the UI into Ground Station mode.

Validation

  • yarn test — passed.
  • Windows x64 ZIP build completed and archive integrity was verified.

@qodo-code-review

Copy link
Copy Markdown
Contributor

Qodo reviews are paused for this user.

Troubleshooting steps vary by plan Learn more →

On a Teams plan?
Reviews resume once this user has a paid seat and their Git account is linked in Qodo.
Link Git account →

Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center?
These require an Enterprise plan - Contact us
Contact us →

@sensei-hacker sensei-hacker added this to the 10.0 milestone Aug 3, 2026
@sensei-hacker
sensei-hacker changed the base branch from maintenance-9.x to maintenance-10.x August 3, 2026 15:28
@github-actions

github-actions Bot commented Aug 3, 2026

Copy link
Copy Markdown

Configurator test build ready — commit 1cfc1f2

Download build artifacts for PR #2690

Available platforms (scroll to the Artifacts section at the bottom of the run page):

  • Windows x64 (ZIP, MSI) and x32 (ZIP, MSI)
  • macOS arm64 (ZIP, DMG) and x64 (ZIP, DMG)
  • Linux x64 (DEB, RPM, ZIP) and aarch64 (DEB, RPM, ZIP)

A GitHub login is required to download artifacts. Build is for testing only.

@sensei-hacker

Copy link
Copy Markdown
Member

Thanks for the fix — this is a clean solve for a tricky bug (Blackbox bytes coincidentally checksumming as valid LTM). A couple of small things I noticed while reading through:

It looks like ltmDecoder.reset() in read_ltm fires on every subsequent receive event once MSP.wasEverReceiving() becomes true, rather than just once at the point MSP is first detected. Since that branch runs on the exact hot path this PR is targeting (Blackbox download can mean many receive events per second), could it be adjusted to only reset once at the transition, rather than on every chunk afterward?

Also, for the second test in ltm-decoder.test.mjs — it reads serial_backend.js and regex-matches the source text rather than exercising the gating logic directly. Would it be possible to test this behaviorally instead, e.g. by feeding a valid MSP frame followed by bytes that happen to checksum as a valid LTM frame, and asserting groundstation.activate never fires? That would catch a logic regression even if the surrounding code shape changed.

One more minor thought: the !CONFIGURATOR.connectionValid check alongside !MSP.wasEverReceiving() in both read_ltm and the ltm-connection-check interval — from tracing the paths that set connectionValid, it looks like wasEverReceiving() always becomes true first, so the extra check may never do anything. Could be wrong about this (maybe there's a BLE-specific path I'm missing) — just flagging in case it's worth a second look.

@mart1npetroff

Copy link
Copy Markdown
Author

Thanks — addressed all three points.

  • LTM routing now uses a session-level MSP detection gate. The decoder is reset once when the first valid MSP frame is observed, then subsequent receive chunks are ignored by the LTM route for the rest of the connection.
  • Replaced the source-regex test with a behavioural test that feeds a valid MSP frame followed by checksum-valid LTM-shaped bytes and verifies that LTM is not read and Ground Station is never activated.
  • Removed the redundant connectionValid checks from the LTM gate. In the current connection flow, MSP is observed before connectionValid is set, so the explicit per-session MSP state is the relevant condition.

@sonarqubecloud

sonarqubecloud Bot commented Aug 5, 2026

Copy link
Copy Markdown

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants