fix(model): rssi explicit presence for protobufs 2.7.26.138

[jamesarich]

Supersedes #6496, which cannot go green on its own.

Upstream protobufs commit 3fec03bb made MeshPacket.rx_rssi an optional int32, so Wire now generates Int? and :core:model stops compiling against the pinned snapshot. Every check on #6496 fails on that one error, because :core:model sits at the bottom of the module graph.

The reason upstream added explicit presence is the interesting part: 0 dBm is a legitimate reading. An SX126x can report exactly 0 dBm and an SX127x's formula can go positive, so has_rx_rssi is what distinguishes a real 0 from "the radio told us nothing" — and a replayed packet rebuilt from history is supposed to leave the field absent rather than emit 0.

So rather than coerce the absent case back to 0 and throw away what the upstream change exists to provide, this threads nullability through the domain layer.

🛠️ Changes

• DataPacket, Message, Reaction and MeshBeaconOffer carry rssi: Int?.
• packet.rssi and reactions.rssi become nullable columns at schema 51, auto-migrated.
• MetricFormatter.rssi renders an absent reading as —; the Rssi composable renders nothing at all, so no row ever claims a fabricated 0 dBm.
• Locally-sent reactions store a null rssi instead of 0 — they were never received over the air, so they have no reading.
• Node.rssi keeps its Int.MAX_VALUE sentinel; absence was already representable there and SignalInfo already checks it. What changed is that an absent reading no longer clobbers a node's last real one.

🐛 Two zero-guards that were hiding real data

Making absence explicit exposed two places that were already losing genuine readings:

• DiscoveryScanEngine did if (rx_rssi != 0) node.rssi = rx_rssi, discarding a reported 0 dBm outright.
• The signal-metrics chart filtered those packets out of the series entirely.

Both are now presence checks, so a real 0 dBm is recorded and plotted.

🧹 Migration notes

Room generated the 50→51 auto-migration; I verified the emitted SQL rather than assuming:

• Both tables are recreated with every column, row and index preserved, and uuid is copied — which matters because packet_fts is @Fts5(contentEntity = Packet::class) and keyed by rowid.
• FTS sync triggers are dropped in onPreMigrate and rebuilt in onPostMigrate, so the search index survives the table rebuild.
• The 50→51 schema diff is exactly those two columns and nothing else.

Known limitation: rows written before schema 51 stored 0 for both absent and 0 dBm, so existing history stays ambiguous — unrecoverable by construction. The reaction dialog therefore keeps its legacy == 0 check alongside the new null check, and says so in a comment.

🌟 SNR audit

rx_snr is unchanged upstream — still a bare float rx_snr = 8 at both the pinned commit and HEAD. No compile impact.

It does carry the identical latent bug, but the app cannot fix it: a proto3 bare float has no presence, so 0f on the wire is genuinely ambiguous. The inconsistency is already visible across three call sites:

• DiscoveryScanEngine and ExportDataUseCase both guard on rx_snr != 0f, discarding real 0 dB readings (the export silently omits those rows from the CSV).
• SignalMetrics instead guards on !isNaN(), so it does treat 0f as valid.

The real fix is an upstream optional float rx_snr mirroring what 3fec03bb did for rssi. Left alone here deliberately — papering over it in the app would just add a fourth convention.

Also worth flagging: 3fec03bb additionally added a NodeInfoLite.rssi storage field, and 94cdec45 removed it hours later the same day. The pinned commit 26db1b5 already includes the removal, so we're fine — but treat this snapshot range as unsettled.

Testing Performed

Full baseline green: spotlessApply spotlessCheck detekt assembleDebug test allTests kmpSmokeCompile.

New coverage:

• MeshDataMapperTest — absent rssi stays null; a reported 0 stays 0.
• MetricFormatterTest — rssi(null) renders — (alongside the existing rssi(0) → 0 dBm).
• MeshBeaconOfferTest — encode/decode round-trips an absent rssi without collapsing it to zero.
• CommonPacketDaoTest — null and 0 persist distinctly through the nullable column.
• DiscoveryScanEngineTest — a reported 0 dBm is recorded rather than discarded.

Not verified on device: the migration was validated by reading the generated SQL and by the schema diff, not by an on-device upgrade from 50 → 51.

🤖 Generated with Claude Code

Summary by CodeRabbit

• Bug Fixes
  • RSSI readings now distinguish between an unavailable value and a valid 0 dBm measurement (including “unknown” display).
  • Missing RSSI values no longer overwrite previously recorded readings.
  • Signal charts, discovery outputs, maps, reports, and reaction dialogs consistently handle unavailable RSSI.
  • Discovery ranking now ignores nodes without RSSI when computing signal-strength scores.
  • CSV exports leave unavailable RSSI fields blank.
• Database
  • Existing data is migrated automatically to support nullable/unavailable RSSI values.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 1b13dbdd-ec01-4182-8b15-5556607c5338

📥 Commits

Reviewing files that changed from the base of the PR and between 6db03e6 and b0283fa.

📒 Files selected for processing (3)

• core/database/src/jvmTest/kotlin/org/meshtastic/core/database/MeshtasticDatabaseMigrationTest.kt
• feature/discovery/src/commonMain/kotlin/org/meshtastic/feature/discovery/scan/DiscoveryRankingEngine.kt
• feature/discovery/src/commonTest/kotlin/org/meshtastic/feature/discovery/DiscoveryRankingEngineTest.kt

---

📝 Walkthrough

Walkthrough

RSSI handling now preserves the distinction between an absent reading and a valid 0 dBm value across models, database schema, packet processing, discovery, UI rendering, maps, charts, and exports.

Changes

Nullable RSSI propagation

Layer / File(s)	Summary
Nullable model contracts core/model/..., core/common/...	RSSI fields and beacon decoding now use nullable values, while explicit 0 dBm readings remain valid.
Database migration and persistence core/database/...	Room schema version 51 stores nullable RSSI values and validates migration, defaults, and distinct null/zero round trips.
Packet and discovery processing core/data/..., core/service/..., feature/discovery/..., gradle/libs.versions.toml	Packet, reaction, and discovery flows preserve missing RSSI, record reported zero values, exclude absent values from ranking, and update the protobuf snapshot.
Presentation and exports core/ui/..., feature/discovery/..., feature/messaging/..., feature/node/..., androidApp/...	RSSI formatting, signal indicators, discovery views, maps, reaction dialogs, charts, and CSV output now handle nullable readings.

Estimated code review effort: 4 (Complex) | ~45 minutes

Possibly related PRs

• meshtastic/Meshtastic-Android#6097: Mesh Beacon RSSI handling and invitation UI overlap with the nullable RSSI changes.
• meshtastic/Meshtastic-Android#6191: Migration-chain validation overlaps with the schema 51 and auto-migration changes.
• meshtastic/Meshtastic-Android#6398: The same RSSI composable is modified for nullable rendering and label support.

Suggested labels: testing

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title captures the main theme: making RSSI presence explicit for the protobuf update, even though the change spans more than model code.
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.

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

core/ui/src/commonMain/kotlin/org/meshtastic/core/ui/component/LoraSignalIndicator.kt (1)

176-179: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low value

Make SnrAndRssi handle nullable RSSI.

Rssi already accepts Int?, but SnrAndRssi still takes rssi: Int and has no callers, so it just adds an unnecessary coercion surface for nullable RSSI sources. Give it the same nullable signature and pass it through to Rssi(rssi).

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@core/ui/src/commonMain/kotlin/org/meshtastic/core/ui/component/LoraSignalIndicator.kt`
around lines 176 - 179, Update SnrAndRssi to accept rssi: Int? instead of Int,
and pass the nullable value directly to Rssi(rssi). Preserve the existing SNR
rendering behavior and avoid coercing nullable RSSI values.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In
`@feature/discovery/src/commonMain/kotlin/org/meshtastic/feature/discovery/DiscoveryScanEngine.kt`:
- Around line 270-271: Make CollectedNodeData.rssi nullable with a null default
so absent RSSI remains distinct from a reported 0 dBm; update
DiscoveryScanEngine presence checks, including the logic around the
direct-signal handling near the existing rssi == 0 check, to use node.rssi ==
null. Preserve the nullable value through persistence and add a regression test
alongside the zero-value RSSI test for packets without RSSI.

---

Nitpick comments:
In
`@core/ui/src/commonMain/kotlin/org/meshtastic/core/ui/component/LoraSignalIndicator.kt`:
- Around line 176-179: Update SnrAndRssi to accept rssi: Int? instead of Int,
and pass the nullable value directly to Rssi(rssi). Preserve the existing SNR
rendering behavior and avoid coercing nullable RSSI values.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 8a83524d-98f2-47eb-bac1-3fc7505a5d31

📥 Commits

Reviewing files that changed from the base of the PR and between 4c3196c and 18bbbc4.

📒 Files selected for processing (22)

• core/common/src/commonMain/kotlin/org/meshtastic/core/common/util/MetricFormatter.kt
• core/common/src/commonTest/kotlin/org/meshtastic/core/common/util/MetricFormatterTest.kt
• core/data/src/commonMain/kotlin/org/meshtastic/core/data/manager/MeshMessageProcessorImpl.kt
• core/database/schemas/org.meshtastic.core.database.MeshtasticDatabase/51.json
• core/database/src/commonMain/kotlin/org/meshtastic/core/database/MeshtasticDatabase.kt
• core/database/src/commonMain/kotlin/org/meshtastic/core/database/entity/Packet.kt
• core/database/src/commonTest/kotlin/org/meshtastic/core/database/dao/CommonPacketDaoTest.kt
• core/model/src/commonMain/kotlin/org/meshtastic/core/model/DataPacket.kt
• core/model/src/commonMain/kotlin/org/meshtastic/core/model/MeshBeaconOffer.kt
• core/model/src/commonMain/kotlin/org/meshtastic/core/model/Message.kt
• core/model/src/commonMain/kotlin/org/meshtastic/core/model/Reaction.kt
• core/model/src/commonTest/kotlin/org/meshtastic/core/model/util/MeshBeaconOfferTest.kt
• core/model/src/commonTest/kotlin/org/meshtastic/core/model/util/MeshDataMapperTest.kt
• core/service/src/commonMain/kotlin/org/meshtastic/core/service/MessagingControllerImpl.kt
• core/ui/src/commonMain/kotlin/org/meshtastic/core/ui/component/LoraSignalIndicator.kt
• feature/discovery/src/commonMain/kotlin/org/meshtastic/feature/discovery/DiscoveryScanEngine.kt
• feature/discovery/src/commonMain/kotlin/org/meshtastic/feature/discovery/ui/component/MeshBeaconInvitationCard.kt
• feature/discovery/src/commonTest/kotlin/org/meshtastic/feature/discovery/DiscoveryScanEngineTest.kt
• feature/messaging/src/commonMain/kotlin/org/meshtastic/feature/messaging/component/Reaction.kt
• feature/node/src/commonMain/kotlin/org/meshtastic/feature/node/metrics/MetricsViewModel.kt
• feature/node/src/commonMain/kotlin/org/meshtastic/feature/node/metrics/SignalMetrics.kt
• gradle/libs.versions.toml

[jamesarich]

@coderabbitai review

[coderabbitai]

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[coderabbitai]

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In
`@core/database/src/androidHostTest/kotlin/org/meshtastic/core/database/dao/DiscoveryMigrationTest.kt`:
- Line 194: Add a dedicated 50→51 migration test alongside
discoveredNodeEntity_defaultValues() that builds or loads a v50 database
containing a discovery row with the legacy nullable-column defaults, including
rssi = 0, then runs the Room migration to v51 and verifies the row is preserved
with the expected v51 SQL NULL behavior. Keep the existing current-schema
default-value test separate and reuse the migration/database fixtures already
defined in DiscoveryMigrationTest.

In
`@feature/discovery/src/commonMain/kotlin/org/meshtastic/feature/discovery/scan/DiscoveryRankingEngine.kt`:
- Around line 96-97: Update the RSSI median calculation in the discovery ranking
flow to remain nullable when rssiValues is empty instead of defaulting to 0 dBm.
Adjust the comparator to rank nodes with missing RSSI after nodes with measured
values, while preserving the existing median-based ordering for non-empty RSSI
data.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 1f069a00-8537-48f2-ba83-134dd1cea9e3

📥 Commits

Reviewing files that changed from the base of the PR and between 18bbbc4 and 6db03e6.

📒 Files selected for processing (12)

• androidApp/src/fdroid/kotlin/org/meshtastic/app/map/discovery/DiscoveryOsmMap.kt
• androidApp/src/google/kotlin/org/meshtastic/app/map/discovery/DiscoveryGoogleMap.kt
• core/database/schemas/org.meshtastic.core.database.MeshtasticDatabase/51.json
• core/database/src/androidHostTest/kotlin/org/meshtastic/core/database/dao/DiscoveryMigrationTest.kt
• core/database/src/commonMain/kotlin/org/meshtastic/core/database/entity/DiscoveredNodeEntity.kt
• core/database/src/commonTest/kotlin/org/meshtastic/core/database/dao/PacketDaoAtomicTransactionTest.kt
• core/ui/src/commonMain/kotlin/org/meshtastic/core/ui/component/LoraSignalIndicator.kt
• core/ui/src/commonMain/kotlin/org/meshtastic/core/ui/util/DiscoveryMapNode.kt
• feature/discovery/src/commonMain/kotlin/org/meshtastic/feature/discovery/DiscoveryScanEngine.kt
• feature/discovery/src/commonMain/kotlin/org/meshtastic/feature/discovery/export/DiscoveryReportFormatter.kt
• feature/discovery/src/commonMain/kotlin/org/meshtastic/feature/discovery/scan/DiscoveryRankingEngine.kt
• feature/discovery/src/commonTest/kotlin/org/meshtastic/feature/discovery/DiscoveryScanEngineTest.kt