Noise floor#1566
Conversation
Conflicts resolved: - AccessoryManager+ToRadio.swift: Keep both sendLocalStatsRequest and exchangeUserInfo - NodeDetail.swift: Keep both RequestLocalStatsButton and ExchangeUserInfoButton - project.pbxproj: Add ExchangeUserInfoButton.swift to project
Add noise floor
|
Left some comments in the firmware PR, but in general I would say it would be good to have an explanation with e.g. TipKit about it. It needs to be taken with a grain of salt, and it can vary quite a bit (as you can see in 2 minutes there is already almost 10dB difference). Adding filters does not necessarily help, e.g. when the noise/interference is in-band and may skew the result, as they have an insertion loss which will result in a lower noise floor shown. |
|
Great feature that will really assist with infrastructure site selection. No more inferring noise floor via traceroute to other nodes. |
|
@copilot resolve the merge conflicts in this pull request |
@copilot implement this tipkit suggestion |
Co-authored-by: garthvh <1795163+garthvh@users.noreply.github.com>
|
|
Merged
|
garthvh
left a comment
There was a problem hiding this comment.
Thanks for the PR — noise floor and local stats are genuinely useful diagnostics for advanced users, and the overall structure of the feature (chart + table + CSV export + request button with cooldown) is solid and matches the pattern of the existing telemetry log views. A few blocking issues need to be resolved before this can merge.
Critical — .gitmodules points to a forked protobufs submodule
The PR changes .gitmodules to point at https://github.com/RCGV1/protobufs-fork.git on branch noise-floor. This cannot be merged — the submodule must always point to https://github.com/meshtastic/protobufs.git.
The good news: noiseFloor (field 15 in LocalStats) is already present in the official protobufs on main — telemetryMessage.localStats.noiseFloor is already available as Int32. Please revert the .gitmodules change and point the submodule back to the official repo.
Critical — Wrong persistence layer for TelemetryEntity
The PR modifies Meshtastic/Meshtastic.xcdatamodeld/MeshtasticDataModelV 55.xcdatamodel and TelemetryEntity+CoreDataClass.swift. Core Data has been replaced by SwiftData in this project. On main, TelemetryEntity is a SwiftData @Model at Meshtastic/Model/TelemetryEntity.swift.
The noiseFloor property must be added to the SwiftData model:
// Meshtastic/Model/TelemetryEntity.swift
var noiseFloor: Int32?A VersionedSchema migration entry is also required in MeshtasticSchema.swift. See docs/developer/swiftdata.md for the migration pattern used in this project.
Critical — NSPredicate in LocalStatsLog and NodeDetail
NSPredicate is a Core Data API. Two places use it:
// LocalStatsLog.swift
node.telemetries?.filtered(using: NSPredicate(format: "metricsType == 4"))
// NodeDetail.swift
node.telemetries?.filtered(using: NSPredicate(format: "metricsType == 4")).countWith SwiftData, use Swift's native filter:
node.telemetries.filter { $0.metricsType == 4 }Critical — @Environment(.managedObjectContext) in LocalStatsLog
@Environment(\.managedObjectContext) var contextThis is the Core Data environment key. The SwiftData equivalent is:
@Environment(\.modelContext) var contextWarning — Proto type mismatch: noiseFloor is Int32, not Float
The protobuf defines noiseFloor as Int32 (whole dBm value). The PR stores and displays it as Float. Please keep it as Int32 throughout — in the SwiftData model, the CSV export, and the chart/table display. The noiseFloor != 0 nil check should also become hasValue optional pattern once stored as Int32?.
Warning — Typo in user-facing alert string
"Responses can some time."
Should be:
"Responses can take some time."
Warning — "Icky" as a localizable string
"Icky" is used as a RuleMark chart label and appears in Localizable.xcstrings with the comment "Icky" is slang for very bad. Slang does not translate well and will confuse translators. Please use a proper technical label such as "Poor Signal" or "Threshold (-85 dBm)".
Warning — Checklist not completed
All checklist items are unchecked. Please complete the self-review checklist before requesting review.
What to keep as-is
sendLocalStatsRequest()inAccessoryManager+ToRadio— clean, follows existing patterns perfectlyRateLimitedButtonwith 30s cooldown — correct approach- CSV export in
WriteCsvFile.swift— well structured - Overall
LocalStatsLogview structure (chart + table + export buttons) — matchesDeviceMetricsLogand other telemetry log views nicely noiseFloorcolour coding logic — sensible thresholds
What changed?
Added noise floor readings to the app from local stats. Also added a Local Stats log page that is in Node details. Also added a button to request local stats from a non local node (30s cooldown)
Why did it change?
Local stats and Noise Floor are extremely useful for advanced users to visualize how their node is behaving.
Depends on:
meshtastic/firmware#9347
How is this tested?
Tested with my custom firmware extensively
Screenshots/Videos (when applicable)
Checklist