diff --git a/src/defib/power/routeros.py b/src/defib/power/routeros.py index a6a813f..31c40a3 100644 --- a/src/defib/power/routeros.py +++ b/src/defib/power/routeros.py @@ -365,7 +365,21 @@ async def power_off(self, port: str) -> None: async def power_on(self, port: str) -> None: restore_mode = self._saved_poe_out.pop(port, "forced-on") - logger.info("PoE ON: %s on %s (restoring %s)", port, self._host, restore_mode) + # power_off saves whatever the port's poe-out was BEFORE we turned + # it off. If the port was already off (e.g. recovering a camera + # that's been parked), the saved "previous" state is "off" — and + # restoring it would defeat the purpose of power_on entirely. + # Promote to forced-on so the port actually comes up. + if restore_mode == "off": + logger.info( + "PoE ON: %s on %s (saved state was 'off' — promoting to " + "'forced-on')", port, self._host, + ) + restore_mode = "forced-on" + else: + logger.info( + "PoE ON: %s on %s (restoring %s)", port, self._host, restore_mode, + ) await self._set_poe(port, restore_mode) async def power_cycle(self, port: str, off_duration: float = 5.0) -> None: diff --git a/tests/test_power.py b/tests/test_power.py index f497960..e4968b1 100644 --- a/tests/test_power.py +++ b/tests/test_power.py @@ -187,3 +187,77 @@ def test_custom_values(self, monkeypatch: pytest.MonkeyPatch) -> None: assert ctrl._username == "root" assert ctrl._password == "secret" assert ctrl._api_port == 9999 + + +# --------------------------------------------------------------------------- +# RouterOS power_off / power_on save+restore semantics +# +# RouterOSController records the previous poe-out mode on power_off so +# power_on can put the port back to its prior state ("forced-on" / "auto-on" / +# "auto-off") rather than blindly hard-coding one mode. The tricky case is +# when the port was already "off" on entry — see test_power_on_promotes_off +# below; that was a real bug that left ports stuck off. +# --------------------------------------------------------------------------- + + +class _PoeStateRouterOS(RouterOSController): + """RouterOSController with the two network-touching primitives stubbed + out so the save/restore state machine can be tested without a switch.""" + + def __init__(self, initial_poe_out: str) -> None: + super().__init__(host="test", username="u", password="p") + self._poe_state: dict[str, str] = {"ether3": initial_poe_out} + self.set_calls: list[tuple[str, str]] = [] + + async def _get_poe_out(self, interface_name: str) -> str: # type: ignore[override] + return self._poe_state.get(interface_name, "auto-on") + + async def _set_poe(self, interface_name: str, poe_out: str) -> None: # type: ignore[override] + self.set_calls.append((interface_name, poe_out)) + self._poe_state[interface_name] = poe_out + + +class TestRouterOSPowerOnOff: + async def test_power_off_then_on_restores_forced_on(self) -> None: + ctrl = _PoeStateRouterOS("forced-on") + await ctrl.power_off("ether3") + await ctrl.power_on("ether3") + assert ctrl._poe_state["ether3"] == "forced-on" + # Saved state cleared on restore + assert "ether3" not in ctrl._saved_poe_out + + async def test_power_off_then_on_restores_auto_on(self) -> None: + # PoE mode can legitimately be "auto-on" (negotiate with device); + # power_on should preserve that, not blindly set "forced-on". + ctrl = _PoeStateRouterOS("auto-on") + await ctrl.power_off("ether3") + await ctrl.power_on("ether3") + assert ctrl._poe_state["ether3"] == "auto-on" + + async def test_power_on_promotes_off(self) -> None: + # Regression: with no fix, this left the port at "off" — because + # power_off saved "off" as the previous state, and power_on then + # "restored" to "off". power_on must always result in a powered + # port, so a saved "off" gets promoted to "forced-on". + ctrl = _PoeStateRouterOS("off") + await ctrl.power_off("ether3") + await ctrl.power_on("ether3") + assert ctrl._poe_state["ether3"] == "forced-on" + + async def test_power_on_without_prior_off_uses_forced_on(self) -> None: + # No saved state means power_on has nothing to restore — default + # to forced-on. + ctrl = _PoeStateRouterOS("auto-off") + await ctrl.power_on("ether3") + assert ctrl._poe_state["ether3"] == "forced-on" + + async def test_double_power_off_does_not_clobber_saved_state(self) -> None: + # Calling power_off twice in a row must not overwrite the saved + # state with "off" — otherwise power_on would have nothing useful + # to restore on the chip that was once "forced-on". + ctrl = _PoeStateRouterOS("forced-on") + await ctrl.power_off("ether3") + await ctrl.power_off("ether3") + assert ctrl._saved_poe_out["ether3"] == "forced-on" + await ctrl.power_on("ether3") + assert ctrl._poe_state["ether3"] == "forced-on"