add support for .cfg export/import#944
Conversation
adds binary import and export to mirror functionality of android (and soon ios) app
|
|
📝 WalkthroughWalkthroughAdds protobuf ChangesBinary config import/export
Estimated code review effort: 4 (Complex) | ~45 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Comment |
- Unify parsing for yaml/DeviceProfile paths - Improve autodetection - Ensure partial-config behavior with yaml - gate configuration to local node (avoiding some pre-existing bugs) - gate fixed position updates behind explicit opt-in via position settings field - use setOwner properly - tests, tests, and more tests
|
@coderabbitai full review |
There was a problem hiding this comment.
Pull request overview
Adds binary .cfg (protobuf DeviceProfile) import/export support to the Meshtastic Python CLI, enabling configuration backup/restore parity with the Android app while keeping existing YAML behavior and adding explicit format control.
Changes:
- Added
DeviceProfile-based export/import flow with autodetection and explicit--export-formatoverride (auto|yaml|binary|protobuf). - Updated
--configureto support binary profiles and added--import-configas an alias. - Expanded unit tests to cover binary serialization, autodetection, error cases, and YAML/binary consistency.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
meshtastic/__main__.py |
Implements binary profile import/export, format autodetection/override, and profile/YAML conversion helpers. |
meshtastic/tests/test_main.py |
Adds extensive unit coverage for new binary profile behavior and round-trip expectations. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #944 +/- ##
==========================================
+ Coverage 64.68% 66.84% +2.15%
==========================================
Files 25 25
Lines 4619 4741 +122
==========================================
+ Hits 2988 3169 +181
+ Misses 1631 1572 -59
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
meshtastic/tests/test_main.py (1)
2420-2438: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert fixed-position arguments, not just call count.
This test would pass even if import re-rounds protobuf microdegrees through floats. Use a precision-sensitive coordinate and verify the exact
setFixedPosition()arguments.🧪 Proposed test strengthening
- "location:\n lat: 35.0\n lon: -93.0\n alt: 100\n" + "location:\n lat: -49.82207\n lon: 0\n alt: 0\n" "config:\n position:\n fixed_position: true\n" @@ - iface.getNode.return_value.setFixedPosition.assert_called_once() + iface.getNode.return_value.setFixedPosition.assert_called_once_with(-498220700, 0, 0)🤖 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 `@meshtastic/tests/test_main.py` around lines 2420 - 2438, The test only checks that `main()` calls `iface.getNode.return_value.setFixedPosition()` once, so it can miss precision regressions. Strengthen `test_main_configure_calls_setfixedposition_when_config_opt_in` by using a precision-sensitive `location` value and asserting the exact arguments passed to `setFixedPosition`, not just the call count, so the behavior in `main()` and the `setFixedPosition` path is verified precisely.
🤖 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 `@meshtastic/__main__.py`:
- Around line 1470-1490: Guard the myinfo result from interface.getMyNodeInfo()
before accessing position data, since it can be None and cause a crash in the
fixed_position branch. In the export flow around getMyNodeInfo(), check that
myinfo is present before calling myinfo.get("position"), and skip or safely
handle the position copy when it is missing. Use the existing profile and
interface.localNode.localConfig/moduleConfig logic as the anchor for where to
add the guard.
---
Nitpick comments:
In `@meshtastic/tests/test_main.py`:
- Around line 2420-2438: The test only checks that `main()` calls
`iface.getNode.return_value.setFixedPosition()` once, so it can miss precision
regressions. Strengthen
`test_main_configure_calls_setfixedposition_when_config_opt_in` by using a
precision-sensitive `location` value and asserting the exact arguments passed to
`setFixedPosition`, not just the call count, so the behavior in `main()` and the
`setFixedPosition` path is verified precisely.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 9488dfa5-5e88-436e-b515-a190afef5e4e
📒 Files selected for processing (2)
meshtastic/__main__.pymeshtastic/tests/test_main.py
Description
This PR adds support for exporting and importing binary device profiles (
.cfg), bringing the Python CLI to feature parity with the Android app's configuration backup functionality.Key Changes
DeviceProfileprotobuf message. This captures all the same settings as the existing yaml import/export.--import-configas a clear alias for the existing--configurecommand.--export-format(acceptsauto,yaml,binary, orprotobuf) to explicitly override format detection during both imports and exports (binary, andprotobufboth do the same thing).--export-configdefaults toautoand generates a binary protobuf if the target file extension ends in.cfg. Otherwise, it falls back to the existing YAML behavior.--configureand--import-configrely on file contents rather than extensions. It attempts to parse the file as a YAML dictionary; if it fails (e.g., throwing a decode error on binary data), it assumes the file is a binaryDeviceProfileprotobuf and parses it accordingly.Summary by CodeRabbit