fix(nodedb): seed and persist the USERPREFS_FIXED_GPS position - #11640
fix(nodedb): seed and persist the USERPREFS_FIXED_GPS position#11640Simplycissmus wants to merge 1 commit into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (4)
Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review. 📝 WalkthroughWalkthroughThe change adds a native fixed-GPS test environment, seeds compiled coordinates into NodeDB when no stored position exists, persists the state, and tests reboot, override, removal, reseeding, and vanilla-build behavior. ChangesFixed GPS user preferences
Estimated code review effort: 3 (Moderate) | ~25 minutes Merge Risk: 🟡 Moderate · up to This change makes compiled fixed GPS coordinates seed and persist correctly, but interruption during separate writes, encrypted-storage reload, or tracker startup cleanup can still leave location state missing, stale, or replaced. Because that state may be emitted by the device, the PR needs these lifecycle cases addressed or explicitly accepted before merge. Sequence Diagram(s)sequenceDiagram
participant FixedGPSTests
participant NodeDB
participant NodeDBStorage
FixedGPSTests->>NodeDB: initialize baked or vanilla build
NodeDB->>NodeDBStorage: read persisted configuration and position
NodeDB->>NodeDBStorage: save seeded or application-updated position
FixedGPSTests->>NodeDB: simulate reboot and assert coordinates
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Description checkExplanation The description is complete and directly related to the changes. It explains the defects, implementation, test coverage, hardware validation, and unavailable regression devices. The required attestations are included with the tested device identified. Full details: Docstring CoverageExplanation Docstring coverage is 23.81% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 21 functions across 2 files. (2 skipped: 2 unsupported.)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
The
USERPREFS_FIXED_GPSblock inNodeDB.cppwas broken four ways. It did not compile: the satellite-map insert usesinfo->numwithinfoundeclared in that scope. It callednodeDB->setLocalPosition()from inside the NodeDB constructor, where the globalnodeDBpointer is still null — a store through nullthis, which panics with a store-access fault on ESP32-C6 and segfaults the native build. It ran after the boot save decision, so even a successful seed was never written to flash. And it was gated onmyNodeInfo.reboot_count == 1, an ESP32-only NVS counter that is >1 after any non-erase reflash, so the path effectively never ran on real devices.This change moves the feature to where the other
USERPREFS_defaults live:installDefaultConfig()turnsconfig.position.fixed_positionon when the defines are present, and the constructor's existing "restore persisted position" block seeds the compiled-in coordinates viaupdatePosition()whenfixed_positionis on but no self position is stored, ORingSEGMENT_CONFIG | SEGMENT_NODEDATABASEintosaveWhat(thenodePositionssatellite map is not covered by the nodeDatabase CRC compare). A position set or removed from the app wins on later boots.Tests: new native suite
test_fixed_gps_userprefs, run in two flavours. The plaincoverageenv (no defines) covers 3 cases: no position appears from nowhere, an app-set fixed position is restored after reboot, remove stays removed. The newcoverage-fixed-gpsenv (defines set) covers 5 cases: first boot seeds and persists, reboot restores, app override survives reboot, a lost self position is re-seeded, remove stays removed. On unfixed code the seeded suite segfaults in the firstnew NodeDB(). CI runs it as its own step, mirroring the existing coverage steps.Tested on hardware: Seeed XIAO ESP32-C6 DIY node. After erase+flash, the first boot seeded and persisted the position; the next boot logged "Restored fixed position", and the CLI shows
fixedPosition truewith the compiled-in coordinates. No Heltec/RAK hardware available.Tested on Other: Seeed XIAO ESP32-C6 DIY (LLCC68).
Summary by CodeRabbit
New Features
Tests