Select the bmspy-ups device from the configured devices

bmspy-ups selected its device from the devices that had data at startup, and exited with status 0 if the selection failed. One failed first read ended the monitor or made it select the wrong device.

The server has a new DEVICES command that lists the configured names, and client.list_devices calls it. bmspy-ups selects from that list and then waits for the first data of its device. A wrong or missing --device now exits with status 2. The service file does not restart on that status, because a restart cannot correct it.

An older server ends on the unknown DEVICES command, so update the server first.

bmspy-ups.service

@@ -5,9 +5,12 @@ Requires=bmspy-server.service
 [Service]
 Type=exec
 WorkingDirectory=/usr/local/bmspy
+# Add --device NAME if bmspy-server reads more than one device.
 ExecStart=uv run --no-sync bmspy-ups
 RestartSec=5
 Restart=on-failure
+# Status 2 is a wrong option or device name. A restart cannot fix it.
+RestartPreventExitStatus=2
 
 [Install]
 WantedBy=default.target

bmspy/client.py

@@ -125,6 +125,12 @@ def socket_comms(socket_path: str, request_data: dict[str, Any], debug: int = 0)
     return response_data
 
 
+def list_devices(socket_path: str, client_name: str, debug: int = 0) -> list[str]:
+    """Return the names of all configured devices, including those without data."""
+    data = socket_comms(socket_path, {"command": "DEVICES", "client": client_name}, debug)
+    return list(data["devices"])
+
+
 def read_data(socket_path: str, client_name: str, ups: str | None = None, debug: int = 0) -> dict[str, UPS]:
     """Return {ups_name: UPS} for all devices, or just the named one."""
     request: dict[str, Any] = {"command": "GET", "client": client_name}

bmspy/server.py

@@ -433,6 +433,10 @@ def main():
                         debug,
                     )
 
+                case "DEVICES":
+                    # Configured names, including devices that have no data yet.
+                    send_response(connection, {"devices": list(ups_devices)}, client, debug)
+
                 case "GET":
                     ups_filter = request_data.get("ups")
                     targets = (

bmspy/ups.py

@@ -1,16 +1,20 @@
 import argparse
 import atexit
 import os
+import sys
 import time
 import smtplib
 import ssl
 import socket
 from collections import deque
+from collections.abc import Collection
 
 from bmspy import client
 from bmspy.classes import UPS, BMSScalarField
 
 DAEMON_UPDATE_PERIOD = 30
+STARTUP_RETRY_PERIOD = 5
+USAGE_ERROR_STATUS = 2
 
 scheduled_shutdown = False
 critical_sent = False
@@ -26,9 +30,11 @@ def _get_field_value(device: UPS, field_name: str) -> float | None:
     return None
 
 
-def _resolve_ups_device(data: dict[str, UPS], requested: str | None) -> str | None:
+def _resolve_ups_device(data: Collection[str], requested: str | None) -> str | None:
     """Return the device name to monitor, or None and print an error if the selection is invalid.
 
+    *data* holds the available device names. A dict keyed by name also works.
+
     Rules:
     - No devices → error.
     - requested is set and found → use it.
@@ -58,6 +64,25 @@ def _resolve_ups_device(data: dict[str, UPS], requested: str | None) -> str | No
     return next(iter(data))
 
 
+def _wait_for_ups_device(socket_path: str, requested: str | None) -> str | None:
+    """Select the device to monitor, then wait until the server has data for it.
+
+    The selection uses the configured device names, not the devices that have
+    data now. A device that misses its first read at startup then cannot change
+    the selection. A wrong or missing choice is final: it returns None.
+    """
+    name = _resolve_ups_device(client.list_devices(socket_path, "ups"), requested)
+    if name is None:
+        return None
+    waiting = False
+    while name not in client.read_data(socket_path, "ups", ups=name):
+        if not waiting:
+            print("UPS '{}' has no data yet, waiting...".format(name))
+            waiting = True
+        time.sleep(STARTUP_RETRY_PERIOD)
+    return name
+
+
 def handle_shutdown(action: str = "cancel", delay: int = 0, debug: int = 0) -> None:
     """Schedule or cancel a system shutdown; only schedules once per incident."""
     global scheduled_shutdown
@@ -224,10 +249,11 @@ def main() -> None:
     client.handle_registration(args.socket, "ups", debug)
     atexit.register(client.handle_registration, args.socket, "ups", debug)
 
-    initial_data = client.read_data(args.socket, "ups")
-    device_name = _resolve_ups_device(initial_data, args.device)
+    device_name = _wait_for_ups_device(args.socket, args.device)
     if device_name is None:
-        return
+        # A wrong or missing --device is a usage error, as in argparse. A retry
+        # cannot fix it, so the service file does not restart on this status.
+        sys.exit(USAGE_ERROR_STATUS)
 
     history = deque()
     while True:

tests/test_client.py

@@ -91,6 +91,12 @@ class TestReadData:
             call_args = mock_comms.call_args[0][1]
             assert call_args.get("ups") == "myups"
 
+    def test_list_devices_returns_configured_names(self):
+        from bmspy.client import list_devices
+        with patch("bmspy.client.socket_comms", return_value={"devices": ["network", "nas"]}) as mock_comms:
+            assert list_devices("/fake/socket", "test") == ["network", "nas"]
+        assert mock_comms.call_args[0][1] == {"command": "DEVICES", "client": "test"}
+
     def test_no_ups_filter_not_in_request(self):
         with patch("bmspy.client.socket_comms") as mock_comms:
             mock_comms.return_value = {}

tests/test_server.py

@@ -445,6 +445,20 @@ class TestServerMain:
         finally:
             self._stop_server(sock_path, t)
 
+    def test_devices_command_lists_configured_devices(self, tmp_path):
+        """DEVICES names every configured device, through the real client."""
+        from bmspy import client
+
+        sock_path = str(tmp_path / "server_devices.sock")
+        ready = threading.Event()
+        t = self._make_server_thread(sock_path, ready, threading.Event())
+        t.start()
+        ready.wait(timeout=2)
+        try:
+            assert client.list_devices(sock_path, "test") == ["ttyUSB0"]
+        finally:
+            self._stop_server(sock_path, t)
+
     def test_deregister_command(self, tmp_path):
         """Test DEREGISTER command via server main()."""
         sock_path = str(tmp_path / "server_dereg.sock")

tests/test_ups.py

@@ -1,6 +1,6 @@
 import pytest
 import smtplib
-from unittest.mock import patch, MagicMock
+from unittest.mock import call, patch, MagicMock
 
 import bmspy.ups as ups_mod
 from bmspy.classes import BMSScalarField, BMSMultiField, BMSInfoField, UPS
@@ -24,6 +24,13 @@ def _reset_globals():
     ups_mod.alert_sent = False
 
 
+@pytest.fixture(autouse=True)
+def _configured_devices():
+    """The main() tests use one configured device unless they patch this themselves."""
+    with patch("bmspy.ups.client.list_devices", return_value=["testups"]):
+        yield
+
+
 # ---------------------------------------------------------------------------
 # _get_field_value
 # ---------------------------------------------------------------------------
@@ -569,31 +576,57 @@ class TestUpsMainDeviceSelection:
     def _make_data(self, name, current_amps=0.0, charge_ratio=0.8):
         return {name: _make_ups(current_amps, charge_ratio)}
 
-    def test_main_no_devices_returns_early(self, capsys):
-        """main() exits cleanly when no devices are found."""
+    def test_main_waits_at_startup_until_the_device_has_data(self, capsys):
+        """A failed first read must not end the monitor: systemd would not restart it."""
         with patch("sys.argv", ["bmspy-ups"]), \
-             patch("bmspy.ups.client.read_data", return_value={}), \
-             patch("bmspy.ups.client.handle_registration"):
+             patch("bmspy.ups.client.list_devices", return_value=["ups1"]), \
+             patch("bmspy.ups.client.read_data",
+                   side_effect=[{}, {}, self._make_data("ups1"), StopIteration("done")]) as read_data, \
+             patch("bmspy.ups.client.handle_registration"), \
+             patch("bmspy.ups.time.sleep") as mock_sleep:
+            with pytest.raises(StopIteration):
                 ups_mod.main()
-        assert "no UPS" in capsys.readouterr().out
+        # Two empty answers, the answer with the device, then the first loop read.
+        assert [c.kwargs for c in read_data.call_args_list] == [{"ups": "ups1"}] * 4
+        assert mock_sleep.call_args_list == [call(ups_mod.STARTUP_RETRY_PERIOD)] * 2
+        # The reason is reported once, not on each retry.
+        assert capsys.readouterr().out.count("UPS 'ups1' has no data yet") == 1
 
-    def test_main_multiple_devices_no_flag_returns_early(self, capsys):
-        """main() exits with an error when multiple devices exist and --device is not set."""
-        two_devices = {
-            "ups1": _make_ups(0.0, 0.9),
-            "ups2": _make_ups(0.0, 0.8),
-        }
-        with patch("sys.argv", ["bmspy-ups"]), \
-             patch("bmspy.ups.client.read_data", return_value=two_devices), \
-             patch("bmspy.ups.client.handle_registration"):
+    def test_main_selects_from_configured_devices_not_from_those_with_data(self):
+        """One device with data must not be picked when the server has several."""
+        with patch("sys.argv", ["bmspy-ups", "--device", "ups2"]), \
+             patch("bmspy.ups.client.list_devices", return_value=["ups1", "ups2"]), \
+             patch("bmspy.ups.client.read_data",
+                   side_effect=[{}, self._make_data("ups2"), StopIteration("done")]) as read_data, \
+             patch("bmspy.ups.client.handle_registration"), \
+             patch("bmspy.ups.time.sleep") as mock_sleep:
+            with pytest.raises(StopIteration):
                 ups_mod.main()
-        out = capsys.readouterr().out
-        assert "ups1" in out or "ups2" in out
+        assert [c.kwargs for c in read_data.call_args_list] == [{"ups": "ups2"}] * 3
+        assert mock_sleep.call_count == 1
+
+    @pytest.mark.parametrize("argv,expected", [
+        ([], "multiple UPS devices found, specify one with --device: ups1, ups2"),
+        (["--device", "ghost"], "UPS 'ghost' not found. Available: ups1, ups2"),
+    ])
+    def test_main_wrong_or_missing_device_option_is_an_error_exit(self, capsys, argv, expected):
+        """Only ups1 has data, but both are configured: do not guess, and do not wait."""
+        with patch("sys.argv", ["bmspy-ups"] + argv), \
+             patch("bmspy.ups.client.list_devices", return_value=["ups1", "ups2"]), \
+             patch("bmspy.ups.client.read_data", return_value=self._make_data("ups1")) as read_data, \
+             patch("bmspy.ups.client.handle_registration"):
+            with pytest.raises(SystemExit) as exit_info:
+                ups_mod.main()
+        # The argparse usage status: systemd reports it and does not restart.
+        assert exit_info.value.code == ups_mod.USAGE_ERROR_STATUS == 2
+        assert expected in capsys.readouterr().out
+        read_data.assert_not_called()
 
     def test_main_ups_flag_selects_device(self):
         """--device selects the correct device from multiple available ones."""
         data_seq = [self._make_data("ups2")] * 6
-        with patch("sys.argv", ["bmspy-ups", "--device", "ups2"]):
+        with patch("sys.argv", ["bmspy-ups", "--device", "ups2"]), \
+             patch("bmspy.ups.client.list_devices", return_value=["ups1", "ups2"]):
             with pytest.raises(StopIteration):
                 with patch("bmspy.ups.client.read_data",
                            side_effect=[{"ups1": _make_ups(0.0, 0.9), "ups2": _make_ups(0.0, 0.8)}]
@@ -619,14 +652,6 @@ class TestUpsMainDeviceSelection:
                 ups_mod.main()
         assert read_data.call_args_list[1] == (("/tmp/test.sock", "ups"), {"ups": "testups"})
 
-    def test_main_ups_flag_unknown_device_returns_early(self, capsys):
-        """--device with an unknown device name exits with an error."""
-        with patch("sys.argv", ["bmspy-ups", "--device", "ghost"]), \
-             patch("bmspy.ups.client.read_data", return_value={"ups1": _make_ups(0.0, 0.9)}), \
-             patch("bmspy.ups.client.handle_registration"):
-            ups_mod.main()
-        assert "ghost" in capsys.readouterr().out
-
     def test_main_device_disappears_in_loop_sleeps(self):
         """When the named device is missing from a loop response, main() sleeps and retries."""
         initial = {"testups": _make_ups(0.0, 0.8)}