Skip to content

fix(lora): correct preset SNR floors and signal colors - #7110

Merged
jamesarich merged 1 commit into
mainfrom
fix/lora-snr-floors
Sep 10, 2026
Merged

fix(lora): correct preset SNR floors and signal colors#7110
jamesarich merged 1 commit into
mainfrom
fix/lora-snr-floors

Conversation

@jamesarich

@jamesarich jamesarich commented Sep 10, 2026

Copy link
Copy Markdown
Collaborator

LongTurbo links were rated against an SF9 SNR floor even though firmware uses SF11. NarrowFast and NarrowSlow also had incorrect floors, and traceroute colors ignored the selected preset.

🐛 Bug Fixes

  • Derive SNR floors from firmware spreading factors and apply preset-relative colors to traceroutes and neighbor info.
  • Use four RSSI text-color bands and add LongTurbo receiver sensitivity to the site planner.
  • Correct the signal-meter documentation and preset table.

Testing Performed

  • Regression tests cover every preset floor, bandwidth independence, and preset-relative traceroute/neighbor colors, including unknown and zero SNR.
  • Passed spotlessApply spotlessCheck detekt assembleDebug test allTests kmpSmokeCompile on Linux. Apple test execution is skipped on this host.
  • All four docs checks pass. Local CodeRabbit review: no findings.
  • Screenshot validation passed with -Dorg.gradle.isolated-projects=false --no-configuration-cache (CI also disables Isolated Projects for this task). No golden changes.

Constitution check: all seven principles evaluated; shared logic stays in commonMain, no new dependencies or privacy-sensitive data, and user docs are updated.

Summary by CodeRabbit

  • New Features

    • Signal quality ratings now account for the active modem preset and use separate quality bands for weak RSSI.
    • Traceroute and neighbor information displays now apply preset-relative SNR colors, including a red lowest-quality state.
    • Long Turbo radio settings now reflect updated sensitivity and signal thresholds.
  • Bug Fixes

    • Corrected SNR limits for Long Turbo, Narrow Fast, and Narrow Slow presets.
  • Documentation

    • Updated signal-meter, node, radio-settings, and node-metrics guidance to reflect the revised rating behavior.

Derive demodulation floors from firmware spreading factors. Use the active preset for traceroute and neighbor-info colors, restore four RSSI display bands, and add LongTurbo to the site planner sensitivity table.

Cover all preset floors and annotated signal colors with regression tests. Document the preset-relative rating.
@coderabbitai

coderabbitai Bot commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: 5947f9d2-7111-4a78-b7f8-816774492f77

📥 Commits

Reviewing files that changed from the base of the PR and between 5ba6c90 and b997bbc.

📒 Files selected for processing (13)
  • core/model/src/commonMain/kotlin/org/meshtastic/core/model/ChannelOption.kt
  • core/ui/src/commonMain/kotlin/org/meshtastic/core/ui/component/LoraSignalIndicator.kt
  • core/ui/src/commonMain/kotlin/org/meshtastic/core/ui/component/TracerouteAlertHandler.kt
  • core/ui/src/commonMain/kotlin/org/meshtastic/core/ui/util/AnnotatedStrings.kt
  • core/ui/src/commonTest/kotlin/org/meshtastic/core/ui/component/LoraSignalIndicatorTest.kt
  • core/ui/src/commonTest/kotlin/org/meshtastic/core/ui/util/AnnotatedStringsTest.kt
  • docs/en/user/node-metrics.md
  • docs/en/user/nodes.md
  • docs/en/user/settings-radio-user.md
  • docs/en/user/signal-meter.md
  • feature/map/src/commonMain/kotlin/org/meshtastic/feature/map/component/SitePlannerLaunch.kt
  • feature/node/src/commonMain/kotlin/org/meshtastic/feature/node/metrics/NeighborInfoLog.kt
  • feature/node/src/commonMain/kotlin/org/meshtastic/feature/node/metrics/TracerouteLog.kt

Included review availability: Your plan provides up to 4 included reviews per hour; 3 remain after this review.


📝 Walkthrough

Walkthrough

The change derives SNR limits from modem spreading factors, updates RSSI and SNR quality bands, applies preset-aware colors to traceroute and neighbor information, adds tests, and updates related documentation and map sensitivity handling.

Changes

Signal quality and modem presets

Layer / File(s) Summary
Derived modem preset limits
core/model/.../ChannelOption.kt, core/ui/.../LoraSignalIndicatorTest.kt, docs/en/user/settings-radio-user.md, feature/map/.../SitePlannerLaunch.kt
ChannelOption derives snrLimit from spreadingFactor. LONG_TURBO now uses SF11 and a −17.5 dB floor. Tests cover all presets and shared spreading factors. Map sensitivity and documented limits are updated.
Signal quality bands
core/ui/.../LoraSignalIndicator.kt, docs/en/user/node-metrics.md, docs/en/user/nodes.md, docs/en/user/signal-meter.md
RSSI thresholds now include a Quality.NONE band. Documentation describes preset-relative SNR evaluation and RSSI quality handling.
Preset-aware annotation flow
core/ui/.../AnnotatedStrings.kt, core/ui/.../TracerouteAlertHandler.kt, feature/node/.../NeighborInfoLog.kt, feature/node/.../TracerouteLog.kt, core/ui/.../AnnotatedStringsTest.kt
Traceroute and neighbor annotations receive the active modem preset and red status color. A shared helper maps quality tiers to colors. Tests cover preset differences, below-floor SNR, unknown SNR, and zero SNR.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Sequence Diagram(s)

sequenceDiagram
  participant TracerouteLogScreen
  participant LocalModemPreset
  participant AnnotatedStrings
  participant determineSignalQuality
  TracerouteLogScreen->>LocalModemPreset: Read current modem preset
  TracerouteLogScreen->>AnnotatedStrings: Pass SNR, colors, and modem preset
  AnnotatedStrings->>determineSignalQuality: Evaluate SNR against modem preset
  determineSignalQuality-->>AnnotatedStrings: Return quality tier
  AnnotatedStrings-->>TracerouteLogScreen: Return colored annotated text
Loading

Merge Risk: ⚪ Minimal · up to b997b

The PR recalibrates preset-relative signal quality, RSSI colors, annotations, and site-planner sensitivity. Supplied regression coverage and validation show no current merge-blocking risk.

🚥 Pre-merge checks | ✅ 6 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Tests Prove The Path, Not The End State ⚠️ Warning The added test every ModemPreset is covered by the firmware spreading-factor table in core/ui/src/commonTest/kotlin/org/meshtastic/core/ui/component/LoraSignalIndicatorTest.kt:71-78 does not exerc… Change or remove the coverage-only test. Make the test invoke the production mapping for every preset and assert the independent expected floor, while also verifying that each preset has a real ChannelOption mapping. Do not only compare t…
Regression Coverage For Changed Behavior ⚠️ Warning Three changed behaviors lack tests that would fail without the corresponding fixes. (1) LoraSignalIndicator.Rssi changed from three RSSI bands to four bands using RSSI_FAIR_THRESHOLD and `RSSI_BAD… Add a Compose/UI regression test for Rssi that checks the rendered color for values in all four bands and at the strict boundaries (-115, -120, and -126), including the weakest band. Add a SitePlannerLaunchTest case that builds a …
✅ Passed checks (6 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely summarizes the main changes: correcting LoRa preset SNR floors and signal colors.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Sibling Call Sites And Presence Semantics ✅ Passed No sibling call site failure is present. The diff adds nullable modemPreset parameters to annotateTraceroute and annotateNeighborInfo; every head reference is updated, including `TracerouteAlert…
Moved Code Diffed Against Its Original ✅ Passed The check is not applicable. The authoritative diff contains only in-place modifications plus one new test file; Git reports no renames, deleted declarations, or production code added in another file/…
Full details: Tests Prove The Path, Not The End State

Explanation

The added test every ModemPreset is covered by the firmware spreading-factor table in core/ui/src/commonTest/kotlin/org/meshtastic/core/ui/component/LoraSignalIndicatorTest.kt:71-78 does not exercise production code. It only compares ModemPreset.entries with the test-owned expectedFloors map. The test therefore still passes with the base implementation, which retains the old hardcoded ChannelOption floors, including LONG_TURBO = -12.5f. The other added floor and color tests do call production code and distinguish the changed behavior.

Resolution

Change or remove the coverage-only test. Make the test invoke the production mapping for every preset and assert the independent expected floor, while also verifying that each preset has a real ChannelOption mapping. Do not only compare the expected table with the enum entries.

Full details: Regression Coverage For Changed Behavior

Explanation

Three changed behaviors lack tests that would fail without the corresponding fixes. (1) LoraSignalIndicator.Rssi changed from three RSSI bands to four bands using RSSI_FAIR_THRESHOLD and RSSI_BAD_THRESHOLD, but LoraSignalIndicatorUiTest only checks that Rssi(-70) renders text. It does not verify color or any threshold, so the old implementation passes. (2) sensitivityDbmFor added the LONG_TURBO -> -136.0 dBm mapping, but SitePlannerLaunchTest passes channelSet = null and does not test modem-preset sensitivity; the existing parameter URL tests cover unrelated defaults. (3) TracerouteAlertHandler, TracerouteLogScreen, and NeighborInfoLogScreen now obtain LocalModemPreset and pass it to the annotation helpers. AnnotatedStringsTest tests the helpers directly with explicit presets, but no test exercises these call sites, so it would pass if a call site supplied null or the wrong preset.

Resolution

Add a Compose/UI regression test for Rssi that checks the rendered color for values in all four bands and at the strict boundaries (-115, -120, and -126), including the weakest band. Add a SitePlannerLaunchTest case that builds a ChannelSet with LoRaConfig.modem_preset = ModemPreset.LONG_TURBO and asserts toSitePlannerParams(...).rxSensitivityDbm == -136.0; this must fail against the base implementation. Add call-site integration tests for the traceroute alert, traceroute log detail, and neighbor-info detail that provide contrasting LocalModemPreset values such as LONG_TURBO and SHORT_FAST, trigger the detail output, and assert the captured AnnotatedString uses the corresponding colors. These tests must verify the preset is propagated, not only that annotateTraceroute and annotateNeighborInfo classify an explicitly supplied preset.

  • Fix all pre-merge checks with AI

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.

@github-actions github-actions Bot added the bugfix PR tag label Sep 10, 2026
@jamesarich
jamesarich added this pull request to the merge queue Sep 10, 2026
@jamesarich
jamesarich removed this pull request from the merge queue due to the queue being cleared Sep 10, 2026
@jamesarich
jamesarich merged commit ef33160 into main Sep 10, 2026
15 checks passed
@jamesarich
jamesarich deleted the fix/lora-snr-floors branch September 10, 2026 11:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bugfix PR tag

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant