adam/pyMC_Repeater
1
0
Fork 0
mirror of https://github.com/pyMC-dev/pyMC_Repeater.git synced 2026-07-26 19:42:46 +02:00
Code Issues Packages Projects Releases Wiki Activity

fix(cli): honor the boolean config save contract in set commands

Browse Source 
ConfigManager.save_to_file returns a bare bool, but every mesh CLI set
command (and the password command) still tuple-unpacked it, so each one
wrote the YAML and then raised, skipping the live update and returning an
error to the remote admin. Route every save through one helper that
checks the bool, reports save failures honestly, and only live-applies
after a successful write. The test fixture mocked the stale tuple shape,
which is why the suite stayed green while the CLI was broken on the real
manager.
This commit is contained in:
agessaman
2026-07-19 07:08:44 -07:00
parent 0ed92013f9
commit eaab0e5dca
2 changed files with 76 additions and 52 deletions
Download Patch File Download Diff File Expand all files Collapse all files
repeater/handler_helpers/mesh_cli.py
+63 -49
View File
@@ -39,6 +39,22 @@ class MeshCLI:
self.repeater_config = config.get("repeater", {})
self.mesh_config = config.setdefault("mesh", {})
def _save_config_and_apply(self, sections=None) -> bool:
"""Persist the config, then live-apply the changed sections.
``ConfigManager.save_to_file`` returns a bare bool; the tuple form is
tolerated for older manager doubles. Live update only runs after a
successful save so a failed write never half-applies a change. Pass
no sections to stage a change: saved to disk, applied on restart.
"""
result = self.config_manager.save_to_file()
saved = result[0] if isinstance(result, tuple) else bool(result)
if not saved:
return False
if sections:
self.config_manager.live_update_daemon(sections)
return True
def _get_node_name(self) -> str:
"""Return the configured node name, preferring the newer key when present."""
return self.repeater_config.get("node_name") or self.repeater_config.get("name", "Unknown")
@@ -455,11 +471,9 @@ class MeshCLI:
# Save config and live update
try:
saved, err = self.config_manager.save_to_file()
if not saved:
logger.error(f"Failed to save password: {err}")
return f"Error: Failed to save config: {err}"
self.config_manager.live_update_daemon(["security"])
if not self._save_config_and_apply(["security"]):
logger.error("Failed to save password: config save failed")
return "Error: Failed to save config"
return f"password now: {new_password}"
except Exception as e:
logger.error(f"Failed to save password: {e}")
@@ -615,32 +629,32 @@ class MeshCLI:
try:
if key == "af":
self.repeater_config["airtime_factor"] = float(value)
saved, _ = self.config_manager.save_to_file()
self.config_manager.live_update_daemon(["repeater"])
if not self._save_config_and_apply(["repeater"]):
return "Error: Failed to save config"
return "OK"
elif key == "name":
self._set_node_name(value)
saved, _ = self.config_manager.save_to_file()
self.config_manager.live_update_daemon(["repeater"])
if not self._save_config_and_apply(["repeater"]):
return "Error: Failed to save config"
return "OK"
elif key == "repeat":
self.repeater_config["mode"] = "forward" if value.lower() == "on" else "monitor"
saved, _ = self.config_manager.save_to_file()
self.config_manager.live_update_daemon(["repeater"])
if not self._save_config_and_apply(["repeater"]):
return "Error: Failed to save config"
return f"OK - repeat is now {'ON' if self.repeater_config['mode'] == 'forward' else 'OFF'}"
elif key == "lat":
self.repeater_config["latitude"] = float(value)
saved, _ = self.config_manager.save_to_file()
self.config_manager.live_update_daemon(["repeater"])
if not self._save_config_and_apply(["repeater"]):
return "Error: Failed to save config"
return "OK"
elif key == "lon":
self.repeater_config["longitude"] = float(value)
saved, _ = self.config_manager.save_to_file()
self.config_manager.live_update_daemon(["repeater"])
if not self._save_config_and_apply(["repeater"]):
return "Error: Failed to save config"
return "OK"
elif key == "radio":
@@ -656,46 +670,46 @@ class MeshCLI:
self.config["radio"]["bandwidth"] = float(radio_parts[1])
self.config["radio"]["spreading_factor"] = int(radio_parts[2])
self.config["radio"]["coding_rate"] = int(radio_parts[3])
saved, _ = self.config_manager.save_to_file()
self.config_manager.live_update_daemon(["radio"])
if not self._save_config_and_apply(["radio"]):
return "Error: Failed to save config"
return "OK - restart repeater to apply"
elif key == "freq":
if "radio" not in self.config:
self.config["radio"] = {}
self.config["radio"]["frequency"] = float(value)
saved, _ = self.config_manager.save_to_file()
self.config_manager.live_update_daemon(["radio"])
if not self._save_config_and_apply(["radio"]):
return "Error: Failed to save config"
return "OK - restart repeater to apply"
elif key == "tx":
if "radio" not in self.config:
self.config["radio"] = {}
self.config["radio"]["tx_power"] = int(value)
saved, _ = self.config_manager.save_to_file()
self.config_manager.live_update_daemon(["radio"])
if not self._save_config_and_apply(["radio"]):
return "Error: Failed to save config"
return "OK"
elif key == "guest.password":
if "security" not in self.config:
self.config["security"] = {}
self.config["security"]["guest_password"] = value
saved, _ = self.config_manager.save_to_file()
self.config_manager.live_update_daemon(["security"])
if not self._save_config_and_apply(["security"]):
return "Error: Failed to save config"
return "OK"
elif key == "owner.info":
self.repeater_config["owner_info"] = value.replace("|", "\n")
saved, _ = self.config_manager.save_to_file()
self.config_manager.live_update_daemon(["repeater"])
if not self._save_config_and_apply(["repeater"]):
return "Error: Failed to save config"
return "OK"
elif key == "allow.read.only":
if "security" not in self.config:
self.config["security"] = {}
self.config["security"]["allow_read_only"] = value.lower() == "on"
saved, _ = self.config_manager.save_to_file()
self.config_manager.live_update_daemon(["security"])
if not self._save_config_and_apply(["security"]):
return "Error: Failed to save config"
return "OK"
elif key == "advert.interval":
@@ -703,8 +717,8 @@ class MeshCLI:
if mins > 0 and (mins < 60 or mins > 240):
return "Error: interval range is 60-240 minutes"
self.repeater_config["advert_interval_minutes"] = mins
saved, _ = self.config_manager.save_to_file()
self.config_manager.live_update_daemon(["repeater"])
if not self._save_config_and_apply(["repeater"]):
return "Error: Failed to save config"
return "OK"
elif key == "flood.advert.interval":
@@ -712,8 +726,8 @@ class MeshCLI:
if (hours > 0 and hours < 3) or hours > 168:
return "Error: interval range is 3-168 hours"
self.repeater_config["flood_advert_interval_hours"] = hours
saved, _ = self.config_manager.save_to_file()
self.config_manager.live_update_daemon(["repeater"])
if not self._save_config_and_apply(["repeater"]):
return "Error: Failed to save config"
return "OK"
elif key == "flood.max":
@@ -721,8 +735,8 @@ class MeshCLI:
if max_val > 64:
return "Error: max 64"
self.repeater_config["max_flood_hops"] = max_val
saved, _ = self.config_manager.save_to_file()
self.config_manager.live_update_daemon(["repeater"])
if not self._save_config_and_apply(["repeater"]):
return "Error: Failed to save config"
return "OK"
elif key == "path.hash.mode":
@@ -730,8 +744,8 @@ class MeshCLI:
if mode not in (0, 1, 2):
return "Error: path.hash.mode must be 0, 1, or 2"
self.mesh_config["path_hash_mode"] = mode
saved, _ = self.config_manager.save_to_file()
self.config_manager.live_update_daemon(["mesh"])
if not self._save_config_and_apply(["mesh"]):
return "Error: Failed to save config"
return "OK"
elif key == "loop.detect":
@@ -739,8 +753,8 @@ class MeshCLI:
if mode not in ("off", "minimal", "moderate", "strict"):
return "Error: loop.detect must be off, minimal, moderate, or strict"
self.mesh_config["loop_detect"] = mode
saved, _ = self.config_manager.save_to_file()
self.config_manager.live_update_daemon(["mesh"])
if not self._save_config_and_apply(["mesh"]):
return "Error: Failed to save config"
return "OK"
elif key == "rxdelay":
@@ -748,8 +762,8 @@ class MeshCLI:
if delay < 0:
return "Error: cannot be negative"
self.config.setdefault("delays", {})["rx_delay_base"] = delay
saved, _ = self.config_manager.save_to_file()
self.config_manager.live_update_daemon(["repeater", "delays"])
if not self._save_config_and_apply(["repeater", "delays"]):
return "Error: Failed to save config"
return "OK"
elif key == "txdelay":
@@ -757,8 +771,8 @@ class MeshCLI:
if delay < 0:
return "Error: cannot be negative"
self.config.setdefault("delays", {})["tx_delay_factor"] = delay
saved, _ = self.config_manager.save_to_file()
self.config_manager.live_update_daemon(["repeater", "delays"])
if not self._save_config_and_apply(["repeater", "delays"]):
return "Error: Failed to save config"
return "OK"
elif key == "direct.txdelay":
@@ -766,20 +780,20 @@ class MeshCLI:
if delay < 0:
return "Error: cannot be negative"
self.config.setdefault("delays", {})["direct_tx_delay_factor"] = delay
saved, _ = self.config_manager.save_to_file()
self.config_manager.live_update_daemon(["repeater", "delays"])
if not self._save_config_and_apply(["repeater", "delays"]):
return "Error: Failed to save config"
return "OK"
elif key == "multi.acks":
self.repeater_config["multi_acks"] = int(value)
saved, _ = self.config_manager.save_to_file()
self.config_manager.live_update_daemon(["repeater"])
if not self._save_config_and_apply(["repeater"]):
return "Error: Failed to save config"
return "OK"
elif key == "int.thresh":
self.repeater_config["interference_threshold"] = int(value)
saved, _ = self.config_manager.save_to_file()
self.config_manager.live_update_daemon(["repeater"])
if not self._save_config_and_apply(["repeater"]):
return "Error: Failed to save config"
return "OK"
elif key == "agc.reset.interval":
@@ -787,8 +801,8 @@ class MeshCLI:
# Round to nearest multiple of 4
rounded = (interval // 4) * 4
self.repeater_config["agc_reset_interval"] = rounded
saved, _ = self.config_manager.save_to_file()
self.config_manager.live_update_daemon(["repeater"])
if not self._save_config_and_apply(["repeater"]):
return "Error: Failed to save config"
return f"OK - interval rounded to {rounded}"
else:
tests/test_handler_helpers_mesh_cli.py
+13 -3
View File
@@ -36,9 +36,9 @@ def _base_config():
}
def _cfg_mgr(save_ok=True, err=None):
def _cfg_mgr(save_ok=True):
return SimpleNamespace(
save_to_file=MagicMock(return_value=(save_ok, err)),
save_to_file=MagicMock(return_value=save_ok),
live_update_daemon=MagicMock(),
)
@@ -97,9 +97,10 @@ def test_cmd_password_save_success_failure_and_exception():
assert cli_ok._cmd_password("password newpw") == "password now: newpw"
ok_mgr.live_update_daemon.assert_called_once_with(["security"])
bad_mgr = _cfg_mgr(save_ok=False, err="disk")
bad_mgr = _cfg_mgr(save_ok=False)
cli_bad = MeshCLI("/tmp/cfg.yaml", _base_config(), bad_mgr)
assert "Failed to save config" in cli_bad._cmd_password("password x")
bad_mgr.live_update_daemon.assert_not_called()
ex_mgr = SimpleNamespace(
save_to_file=MagicMock(side_effect=RuntimeError("boom")),
@@ -442,3 +443,12 @@ def test_discovery_auto_add_skips_local_node_and_persists_remote():
)
assert remote_result["auto_added"] is True
storage.record_advert.assert_called_once()
def test_cmd_set_save_failure_reports_error_and_skips_live_update():
mgr = _cfg_mgr(save_ok=False)
cli = MeshCLI("/tmp/cfg.yaml", _base_config(), mgr)
for command in ("af 2.0", "name x", "freq 915", "guest.password g", "flood.max 8"):
assert cli._cmd_set(command) == "Error: Failed to save config"
mgr.live_update_daemon.assert_not_called()