diff --git a/emulator/stubs.py b/emulator/stubs.py index c31faf395..b9b9f5276 100644 --- a/emulator/stubs.py +++ b/emulator/stubs.py @@ -19,7 +19,7 @@ VirtualAudiocard — in-memory audiocard; no ALSA/hardware access. StubWifiManager — in-memory wifi; satisfies Mod/Modhandler's wifi_manager. -StubEthernetManager — pinned-up ethernet stub; no sysfs / systemctl / threads. +StubEthernetManager — pinned-up ethernet stub; no sysfs / pi-status / threads. StubRelay — no-op relay; satisfies the Relay interface without GPIO. """ @@ -258,18 +258,16 @@ def delete_connection(self, name: str) -> Optional[bytes]: class StubEthernetManager(EthernetManager): """Pinned-up ethernet stub for the emulator. - `carrier_up` is always True so the Wired Connection menu surface is always - reachable. `service_active` is flipped locally by start/stop so the menu - re-renders with the new state on the next poll tick — no real systemd unit - is touched. The base class' background polling thread is not started; we - override __init__ to skip it entirely so the emulator has no /sysfs or - systemctl dependencies. + `carrier_up` and `service_active` are always True, so the full status + screen is reachable. The Mac owns the real lifecycle, so there are no + start or stop verbs. __init__ skips the base thread, so the emulator + needs no sysfs or jackbridge-pi-status. """ def __init__(self) -> None: - # Deliberately skip super().__init__() — no thread, no sysfs polling. + # Skip super().__init__() — no thread, no sysfs polling. self.carrier_up = True - self.service_active = False + self.service_active = True # Signal a single initial render so the menu picks up our fake state. self._changed = True self._lock = threading.Lock() @@ -287,15 +285,11 @@ def read_jack_settings(self) -> tuple[Optional[int], Optional[int]]: def read_xrun_buckets(self) -> tuple[int, int, int]: return (0, 0, 0) - def start_service(self) -> None: - with self._lock: - self.service_active = True - self._changed = True + def read_netadapter_health(self) -> tuple[int, int, str]: + return (1, 6, "eth0") - def stop_service(self) -> None: - with self._lock: - self.service_active = False - self._changed = True + def read_link_health(self) -> tuple[bool, int]: + return (False, 0) class StubJackMute(JackMute): diff --git a/modalapi/ethernet/manager.py b/modalapi/ethernet/manager.py index 3e3a22644..a92e79e3b 100644 --- a/modalapi/ethernet/manager.py +++ b/modalapi/ethernet/manager.py @@ -15,37 +15,54 @@ # You should have received a copy of the GNU Affero General Public License # along with pi-stomp. If not, see . +"""Ethernet (wired) status — a read-only view of the pi JackBridge slave. + +The Mac drives the bridge lifecycle: this class only reports what the unit +does. There is no start or stop method here — that path caused the +duplicate-netadapter bug. + +`jackbridge-pi-status` supplies the data (pistomp-companion installs it). +The helper only reads: systemd state, the jack_lsp graph, the IP route, +xrun counts, and netadapter link restarts. Like WifiManager, a background thread does all blocking +I/O and caches the result. The UI thread reads the cache through the +read_* methods. +""" + import logging import os import subprocess import threading -import time from functools import cached_property from typing import Optional from common.util import TEARDOWN_JOIN_S from pistomp.alsa_pcm import read_hw_params -# Contract with the JackBridge service: truncate-on-start, atomic-rewrite of a -# bounded list (entries older than 15 min are dropped on each append). The UI -# just reads the whole file each poll. -SERVICE = "pi-stomp-jackbridge.service" -XRUN_FILE = "/tmp/pi-stomp-jackbridge.xruns" +# Source: pistomp-companion jackbridge/pi/bin/. It reaches the pi only +# through pi-gen-pistomp's `jackbridge` deb — add it to that deb's +# debian/rules install list, which is a separate manifest from +# companion's own install.sh. +STATUS_BIN = "/usr/local/libexec/jackbridge/jackbridge-pi-status" POLL_INTERVAL_S = 2.0 -class EthernetManager: - """Polls Ethernet carrier + JackBridge state on a background thread. +def _int(s: Optional[str]) -> int: + """Parse an int from status output. Return 0 for None, empty, or bad text.""" + if not s: + return 0 + try: + return int(s) + except ValueError: + return 0 - Mirrors the WifiManager pattern: all blocking I/O (sysfs, systemctl, - `ip`, `jack_*`, xrun file) runs on the poll thread and is cached under - a lock; the UI thread only reads cached values. `_changed` is flipped - when carrier/service-active flip so the handler's main poll loop can - notify the UI; field-only changes (IP, sample rate, xruns) are picked - up by the menu's periodic tick re-render without setting `_changed`. - Writes (start/stop service) are fire-and-forget via subprocess.Popen - so the UI thread never blocks on systemctl. +class EthernetManager: + """Poll the Ethernet carrier and the JackBridge status on a background thread. + + Read-only. `_changed` is set when the carrier, the service-active state + or the link-resyncing state changes, so the handler poll loop can refresh the UI. Field + changes (IP, sample rate, xruns, port counts) appear on the next + render without `_changed`. """ @cached_property @@ -65,6 +82,11 @@ def iface(self) -> str: def __init__(self) -> None: self.carrier_up: bool = False self.service_active: bool = False + self._netadapters: int = 0 + self._ports_wired: int = 0 + self._route: str = "" + self._link_resyncing: bool = False + self._net_restarts: int = 0 self._ipv4: Optional[str] = None self._sample_rate: Optional[int] = None self._period: Optional[int] = None @@ -89,23 +111,40 @@ def _run(self) -> None: def _refresh(self) -> None: carrier = self._probe_carrier() - active = self._probe_service_active() if carrier else False + status = self._read_pi_status() if carrier else {} + + active = status.get("service") == "active" + netadapters = _int(status.get("netadapters")) + ports_wired = _int(status.get("ports_wired")) + route = status.get("route", "") or "" + link_resyncing = status.get("link") == "resyncing" + net_restarts = _int(status.get("net_restarts")) + xruns = ( + _int(status.get("xruns_1m")), + _int(status.get("xruns_5m")), + _int(status.get("xruns_15m")), + ) ipv4 = self._probe_ipv4() if carrier else None + if active: - # One /proc read for both — jack_samplerate/jack_bufsize each fork - # and join the RT graph to learn what hw_params already states. + # One /proc read. jack_samplerate and jack_bufsize each fork and + # join the RT graph to read what hw_params already holds. params = read_hw_params() sample_rate = int(params["rate"]) if "rate" in params else None period = int(params["period_size"]) if "period_size" in params else None - xruns = self._probe_xrun_buckets() else: sample_rate = period = None - xruns = (0, 0, 0) + with self._lock: - if carrier != self.carrier_up or active != self.service_active: + if carrier != self.carrier_up or active != self.service_active or link_resyncing != self._link_resyncing: self._changed = True self.carrier_up = carrier self.service_active = active + self._netadapters = netadapters + self._ports_wired = ports_wired + self._route = route + self._link_resyncing = link_resyncing + self._net_restarts = net_restarts self._ipv4 = ipv4 self._sample_rate = sample_rate self._period = period @@ -125,12 +164,29 @@ def _probe_carrier(self) -> bool: return False @staticmethod - def _probe_service_active() -> bool: + def _read_pi_status() -> dict[str, str]: + """Run jackbridge-pi-status and parse its key=value lines. + + The helper only reads and always exits 0. Any error gives an empty + dict, which the callers treat as zeros. + """ try: - return subprocess.call(["systemctl", "is-active", "--quiet", SERVICE]) == 0 + out = subprocess.check_output( + [STATUS_BIN], + text=True, + timeout=3, + stderr=subprocess.DEVNULL, + ) except Exception as e: - logging.warning("systemctl is-active failed for %s: %s", SERVICE, e) - return False + logging.debug("jackbridge-pi-status failed: %s", e) + return {} + + status: dict[str, str] = {} + for line in out.splitlines(): + key, sep, val = line.partition("=") + if sep: + status[key] = val + return status def _probe_ipv4(self) -> Optional[str]: try: @@ -146,38 +202,7 @@ def _probe_ipv4(self) -> Optional[str]: return parts[i + 1] return None - @staticmethod - def _probe_xrun_buckets() -> tuple[int, int, int]: - # File format (produced by jackbridge-xrun-watcher): up to 15 lines, - # oldest first, " ". Each bucket covers - # [ts, ts+60); include it if its END (ts+60) is within the window so a - # freshly-rolled bucket counts for the 1-min query. - try: - with open(XRUN_FILE) as f: - lines = f.read().splitlines() - except OSError: - return (0, 0, 0) - now = time.time() - b1 = b5 = b15 = 0 - for line in lines: - parts = line.split() - if len(parts) != 2: - continue - try: - ts = float(parts[0]) - count = int(parts[1]) - except ValueError: - continue - dt = now - (ts + 60) - if dt < 60: - b1 += count - if dt < 300: - b5 += count - if dt < 900: - b15 += count - return b1, b5, b15 - - # ----- UI-thread reads (return cached values, no I/O) ----- + # ----- UI-thread reads (cached values; no I/O) ----- def read_ipv4(self) -> Optional[str]: with self._lock: @@ -191,21 +216,16 @@ def read_xrun_buckets(self) -> tuple[int, int, int]: with self._lock: return self._xruns - # ----- service control (non-blocking; bg poll picks up the state flip) ----- - - def start_service(self) -> None: - self._spawn_systemctl("start") + def read_link_health(self) -> tuple[bool, int]: + """(resyncing, restarts). Cached; read from the UI thread.""" + with self._lock: + return self._link_resyncing, self._net_restarts - def stop_service(self) -> None: - self._spawn_systemctl("stop") + def read_netadapter_health(self) -> tuple[int, int, str]: + """(netadapters, ports_wired, route). Cached; read from the UI thread. - @staticmethod - def _spawn_systemctl(verb: str) -> None: - try: - subprocess.Popen( - ["sudo", "systemctl", verb, SERVICE], - stdout=subprocess.DEVNULL, - stderr=subprocess.DEVNULL, - ) - except Exception as e: - logging.warning("systemctl %s %s failed to spawn: %s", verb, SERVICE, e) + `netadapters > 1` means the duplicate-slave bug. An empty route, or + a route on a wireless interface, means the Wi-Fi-escape case. + """ + with self._lock: + return self._netadapters, self._ports_wired, self._route diff --git a/tests/snapshots/v3/test_ethernet_menu/test_ethernet_menu_disabled/0.png b/tests/snapshots/v3/test_ethernet_menu/test_ethernet_menu_disabled/0.png index 2a22e5b94..3184ac126 100644 Binary files a/tests/snapshots/v3/test_ethernet_menu/test_ethernet_menu_disabled/0.png and b/tests/snapshots/v3/test_ethernet_menu/test_ethernet_menu_disabled/0.png differ diff --git a/tests/snapshots/v3/test_ethernet_menu/test_ethernet_menu_duplicate_adapters_warning/0.png b/tests/snapshots/v3/test_ethernet_menu/test_ethernet_menu_duplicate_adapters_warning/0.png new file mode 100644 index 000000000..10be9c089 Binary files /dev/null and b/tests/snapshots/v3/test_ethernet_menu/test_ethernet_menu_duplicate_adapters_warning/0.png differ diff --git a/tests/snapshots/v3/test_ethernet_menu/test_ethernet_menu_enabled_with_stats/0.png b/tests/snapshots/v3/test_ethernet_menu/test_ethernet_menu_enabled_with_stats/0.png index 4d50bc3cf..a317a3381 100644 Binary files a/tests/snapshots/v3/test_ethernet_menu/test_ethernet_menu_enabled_with_stats/0.png and b/tests/snapshots/v3/test_ethernet_menu/test_ethernet_menu_enabled_with_stats/0.png differ diff --git a/tests/snapshots/v3/test_ethernet_menu/test_ethernet_menu_link_resyncing/0.png b/tests/snapshots/v3/test_ethernet_menu/test_ethernet_menu_link_resyncing/0.png new file mode 100644 index 000000000..486fa49bb Binary files /dev/null and b/tests/snapshots/v3/test_ethernet_menu/test_ethernet_menu_link_resyncing/0.png differ diff --git a/tests/snapshots/v3/test_ethernet_menu/test_ethernet_menu_muted/0.png b/tests/snapshots/v3/test_ethernet_menu/test_ethernet_menu_muted/0.png index 9763426a5..f11d000c4 100644 Binary files a/tests/snapshots/v3/test_ethernet_menu/test_ethernet_menu_muted/0.png and b/tests/snapshots/v3/test_ethernet_menu/test_ethernet_menu_muted/0.png differ diff --git a/tests/test_ethernet_manager.py b/tests/test_ethernet_manager.py index 49ae2ceea..a5f1c2933 100644 --- a/tests/test_ethernet_manager.py +++ b/tests/test_ethernet_manager.py @@ -1,13 +1,17 @@ -"""Unit tests for EthernetManager — carrier/sysfs/systemctl/ip orchestration. +"""Unit tests for EthernetManager — carrier/sysfs + jackbridge-pi-status parsing. -The background polling thread is suppressed so each test drives state -explicitly via _refresh() or the static _probe_* helpers. All blocking I/O -runs on the poll thread; the public read_* accessors just return cached -values, so they're exercised via _refresh. +The polling thread is suppressed, so each test drives state through +_refresh() or the _probe_* helpers. All blocking I/O is mocked, so this +runs off-device. + +The start/stop verbs are gone (the Mac owns the lifecycle), so their +tests went with them. What remains: carrier detection via sysfs, IPv4 +parsing via `ip`, jackbridge-pi-status parsing, the "service=active +unlocks the hw_params read" gating, and the read_* accessor contract. """ from typing import Generator -from unittest.mock import patch, mock_open +from unittest.mock import mock_open, patch import pytest @@ -50,35 +54,74 @@ def test_probe_carrier_false_when_iface_missing(em: EthernetManager): assert em._probe_carrier() is False -# ---------- _probe_service_active ---------- +# ---------- _read_pi_status ---------- -def test_probe_service_active_true_on_exit_0(): - with patch("subprocess.call", return_value=0): - assert EthernetManager._probe_service_active() is True +def test_read_pi_status_parses_key_value_lines(): + out = ( + "service=active\n" + "netadapters=1\n" + "ports_wired=6\n" + "route=eth0\n" + "iface=eth0\n" + "xruns_1m=0\n" + "xruns_5m=2\n" + "xruns_15m=5\n" + "link=resyncing\n" + "net_restarts=4\n" + "a line with no equals sign\n" + ) + with patch("subprocess.check_output", return_value=out): + s = EthernetManager._read_pi_status() + assert s["service"] == "active" + assert s["netadapters"] == "1" + assert s["ports_wired"] == "6" + assert s["route"] == "eth0" + assert s["xruns_15m"] == "5" + assert s["link"] == "resyncing" + assert len(s) == 10 # the no-equals line is skipped -def test_probe_service_active_false_on_nonzero_exit(): - with patch("subprocess.call", return_value=3): - assert EthernetManager._probe_service_active() is False +def test_read_pi_status_empty_on_missing_helper(): + with patch("subprocess.check_output", side_effect=OSError("not installed")): + assert EthernetManager._read_pi_status() == {} -def test_probe_service_active_false_on_subprocess_error(): - with patch("subprocess.call", side_effect=OSError("boom")): - assert EthernetManager._probe_service_active() is False +# ---------- _refresh + drain_changed ---------- -# ---------- _refresh + drain_changed ---------- +def _pi_status( + service="active", + netadapters="1", + ports_wired="6", + route="eth0", + x1="0", + x5="0", + x15="0", + link="up", + net_restarts="0", +): + return ( + f"service={service}\n" + f"netadapters={netadapters}\n" + f"ports_wired={ports_wired}\n" + f"route={route}\n" + f"iface=eth0\n" + f"xruns_1m={x1}\n" + f"xruns_5m={x5}\n" + f"xruns_15m={x15}\n" + f"link={link}\n" + f"net_restarts={net_restarts}\n" + ) def test_refresh_flips_changed_on_state_transition(em: EthernetManager): assert em.drain_changed() is False # baseline with ( patch.object(EthernetManager, "_probe_carrier", return_value=True), - patch.object(EthernetManager, "_probe_service_active", return_value=True), patch.object(EthernetManager, "_probe_ipv4", return_value="10.0.0.5/24"), + patch("subprocess.check_output", return_value=_pi_status(service="active")), patch(_HW, return_value={"rate": "48000", "period_size": "64"}), - patch.object(EthernetManager, "_probe_xrun_buckets", return_value=(0, 0, 0)), ): em._refresh() assert em.carrier_up is True @@ -89,48 +132,85 @@ def test_refresh_flips_changed_on_state_transition(em: EthernetManager): def test_refresh_no_change_keeps_flag_clear(em: EthernetManager): em.carrier_up = True - em.service_active = False + em.service_active = True with ( patch.object(EthernetManager, "_probe_carrier", return_value=True), - patch.object(EthernetManager, "_probe_service_active", return_value=False), - patch.object(EthernetManager, "_probe_ipv4", return_value=None), + patch.object(EthernetManager, "_probe_ipv4", return_value="10.0.0.5/24"), + patch("subprocess.check_output", return_value=_pi_status(service="active")), + patch(_HW, return_value={"rate": "48000", "period_size": "64"}), ): em._refresh() assert em.drain_changed() is False -def test_refresh_skips_systemctl_and_jack_when_carrier_down(em: EthernetManager): - """Optimization: if no cable, don't bother shelling out to systemctl/ip/jack.""" +def test_refresh_skips_status_and_jack_when_carrier_down(em: EthernetManager): + """Optimisation: with no cable, skip jackbridge-pi-status, ip, and jack.""" with ( patch.object(EthernetManager, "_probe_carrier", return_value=False), - patch.object(EthernetManager, "_probe_service_active") as mock_active, - patch.object(EthernetManager, "_probe_ipv4") as mock_ipv4, - patch(_HW) as mock_jack, - patch.object(EthernetManager, "_probe_xrun_buckets") as mock_xrun, + patch("subprocess.check_output") as m_co, + patch(_HW) as m_hw, ): em._refresh() - mock_active.assert_not_called() - mock_ipv4.assert_not_called() - mock_jack.assert_not_called() - mock_xrun.assert_not_called() - assert em.service_active is False - assert em.read_ipv4() is None + m_co.assert_not_called() + m_hw.assert_not_called() assert em.read_jack_settings() == (None, None) def test_refresh_caches_values_for_ui_thread(em): - """The public read_* accessors return whatever the last _refresh stored — no I/O.""" + """The read_* accessors return what the last _refresh stored, with no I/O.""" with ( patch.object(EthernetManager, "_probe_carrier", return_value=True), - patch.object(EthernetManager, "_probe_service_active", return_value=True), - patch.object(EthernetManager, "_probe_ipv4", return_value="169.254.1.2/16"), + patch.object(EthernetManager, "_probe_ipv4", return_value="10.0.0.5/24"), + patch( + "subprocess.check_output", + return_value=_pi_status( + service="active", + netadapters="2", + ports_wired="4", + route="eth0", + x1="1", + x5="2", + x15="3", + ), + ), patch(_HW, return_value={"rate": "48000", "period_size": "64"}), - patch.object(EthernetManager, "_probe_xrun_buckets", return_value=(1, 2, 3)), ): em._refresh() - assert em.read_ipv4() == "169.254.1.2/16" + assert em.read_ipv4() == "10.0.0.5/24" assert em.read_jack_settings() == (48000, 64) assert em.read_xrun_buckets() == (1, 2, 3) + assert em.read_netadapter_health() == (2, 4, "eth0") + assert em.read_link_health() == (False, 0) + + +def test_refresh_flips_changed_when_link_starts_resyncing(em: EthernetManager): + """netadapter restarts leave the graph, the ports and the xrun rate + healthy, so a link flip is its own state change or the screen keeps + the readout it had.""" + em.carrier_up = True + em.service_active = True + with ( + patch.object(EthernetManager, "_probe_carrier", return_value=True), + patch.object(EthernetManager, "_probe_ipv4", return_value="10.0.0.5/24"), + patch("subprocess.check_output", return_value=_pi_status(link="resyncing", net_restarts="4")), + patch(_HW, return_value={"rate": "48000", "period_size": "64"}), + ): + em._refresh() + assert em.read_link_health() == (True, 4) + assert em.drain_changed() is True + + +def test_refresh_link_unknown_reads_as_not_resyncing(em: EthernetManager): + """`link=unknown` means the watcher has written nothing yet. That is not + evidence of a fault.""" + with ( + patch.object(EthernetManager, "_probe_carrier", return_value=True), + patch.object(EthernetManager, "_probe_ipv4", return_value="10.0.0.5/24"), + patch("subprocess.check_output", return_value=_pi_status(link="unknown")), + patch(_HW, return_value={"rate": "48000", "period_size": "64"}), + ): + em._refresh() + assert em.read_link_health() == (False, 0) # ---------- _probe_ipv4 ---------- @@ -160,21 +240,20 @@ def test_probe_ipv4_returns_none_on_command_error(em: EthernetManager): "subformat: STD\n" "channels: 2\n" "rate: 48000 (48000/1)\n" - "period_size: 64\n" - "buffer_size: 128\n" + "period_size: 128\n" + "buffer_size: 512\n" ) def test_read_hw_params_takes_first_token_of_each_value(): with patch("builtins.open", mock_open(read_data=_HW_PARAMS)): params = read_hw_params() - assert params["rate"] == "48000" # not "48000 (48000/1)" - assert params["period_size"] == "64" + assert params["rate"] == "48000" + assert params["period_size"] == "128" assert params["channels"] == "2" def test_read_hw_params_empty_when_pcm_closed(): - # The file exists but reads "closed" when nothing holds the device. with patch("builtins.open", mock_open(read_data="closed\n")): assert read_hw_params() == {} @@ -182,44 +261,3 @@ def test_read_hw_params_empty_when_pcm_closed(): def test_read_hw_params_empty_off_device(): with patch("builtins.open", side_effect=FileNotFoundError()): assert read_hw_params() == {} - - -# ---------- _probe_xrun_buckets ---------- - - -def test_probe_xrun_buckets_zero_when_file_missing(): - with patch("builtins.open", side_effect=OSError): - assert EthernetManager._probe_xrun_buckets() == (0, 0, 0) - - -def test_probe_xrun_buckets_bins_by_age(): - # File format: " " per line, bucket end at ts+60. - # now=1000; buckets centered so their END (ts+60) gives dt = now-(ts+60): - # ts=910, count=2 -> dt=30 -> 1m,5m,15m - # ts=740, count=3 -> dt=200 -> 5m,15m - # ts=340, count=5 -> dt=600 -> 15m - # ts=-260, count=7 -> dt=1200 -> none - # garbage and a malformed 1-field line are skipped. - data = "910 2\n740 3\n340 5\n-260 7\ngarbage\n970\n" - with patch("builtins.open", mock_open(read_data=data)), patch("time.time", return_value=1000.0): - b1, b5, b15 = EthernetManager._probe_xrun_buckets() - assert (b1, b5, b15) == (2, 5, 10) - - -# ---------- start_service / stop_service ---------- - - -def test_start_service_spawns_systemctl_non_blocking(em: EthernetManager): - with patch("subprocess.Popen") as m: - em.start_service() - m.assert_called_once() - args, _ = m.call_args - assert args[0] == ["sudo", "systemctl", "start", "pi-stomp-jackbridge.service"] - - -def test_stop_service_spawns_systemctl_non_blocking(em: EthernetManager): - with patch("subprocess.Popen") as m: - em.stop_service() - m.assert_called_once() - args, _ = m.call_args - assert args[0] == ["sudo", "systemctl", "stop", "pi-stomp-jackbridge.service"] diff --git a/tests/v3/test_ethernet_menu.py b/tests/v3/test_ethernet_menu.py index 8dec301fa..b24b96900 100644 --- a/tests/v3/test_ethernet_menu.py +++ b/tests/v3/test_ethernet_menu.py @@ -3,6 +3,9 @@ Replaces the live EthernetManager (sysfs polling thread) and JackMute (subprocess) on the handler with controllable fakes, then exercises the menu's render and action paths. + +The menu is *read-only* since the "Mac owns the lifecycle" change: there +is no Enable/Disable button anymore, only status rows + Mute MOD + Back. """ from typing import Optional @@ -15,18 +18,25 @@ class FakeEthernetManager: - """Mirrors the EthernetManager surface used by EthernetMenu, with no I/O.""" - - def __init__(self, carrier_up=True, service_active=False, - ipv4="169.254.125.193/16", jack=(48000, 128), - xruns=(0, 0, 0)): + """Mirrors the read-only EthernetManager surface used by EthernetMenu.""" + + def __init__( + self, + carrier_up=True, + service_active=False, + ipv4="169.254.125.193/16", + jack=(48000, 128), + xruns=(0, 0, 0), + health=(1, 6, "eth0"), + link=(False, 0), + ): self.carrier_up = carrier_up self.service_active = service_active self._ipv4 = ipv4 self._jack = jack self._xruns = xruns - self.start_calls = 0 - self.stop_calls = 0 + self._health = health + self._link = link def read_ipv4(self) -> Optional[str]: return self._ipv4 @@ -37,13 +47,11 @@ def read_jack_settings(self): def read_xrun_buckets(self): return self._xruns - def start_service(self) -> None: - self.start_calls += 1 - self.service_active = True + def read_netadapter_health(self): + return self._health - def stop_service(self) -> None: - self.stop_calls += 1 - self.service_active = False + def read_link_health(self): + return self._link def shutdown(self) -> None: pass @@ -51,7 +59,7 @@ def shutdown(self) -> None: @pytest.fixture def ethernet_env(v3_system): - """Replace the live ethernet_manager and jack_mute with fakes; yield (lcd, fake_em, fake_mute).""" + """Replace ethernet_manager and jack_mute with fakes; yield (lcd, fake_em, fake_mute).""" handler = v3_system.handler handler.ethernet_manager.shutdown() fake_em = FakeEthernetManager() @@ -73,7 +81,7 @@ def _open(lcd) -> EthernetMenu: def test_ethernet_menu_disabled(ethernet_env, snapshot): - """Service inactive — only IP shown, toggle says Enable, MOD not muted.""" + """Service inactive — only IP shown, MOD not muted.""" lcd, em, _ = ethernet_env em.service_active = False _open(lcd) @@ -81,7 +89,7 @@ def test_ethernet_menu_disabled(ethernet_env, snapshot): def test_ethernet_menu_enabled_with_stats(ethernet_env, snapshot): - """Service active — sample rate, period, xrun buckets visible.""" + """Service active — sample rate, period, xrun buckets, link ports visible.""" lcd, em, _ = ethernet_env em.service_active = True em._xruns = (1, 3, 7) @@ -89,6 +97,25 @@ def test_ethernet_menu_enabled_with_stats(ethernet_env, snapshot): snapshot() +def test_ethernet_menu_duplicate_adapters_warning(ethernet_env, snapshot): + """netadapters > 1 surfaces the duplicate-slave warning row.""" + lcd, em, _ = ethernet_env + em.service_active = True + em._health = (2, 6, "eth0") # two netadapters contend for one stream + _open(lcd) + snapshot() + + +def test_ethernet_menu_link_resyncing(ethernet_env, snapshot): + """netadapter is restarting its link — the port count is replaced by the + one row that reports it, plus the remedy.""" + lcd, em, _ = ethernet_env + em.service_active = True + em._link = (True, 4) + _open(lcd) + snapshot() + + def test_ethernet_menu_muted(ethernet_env, snapshot): """Service active + MOD muted → button reads "Unmute MOD".""" lcd, em, mute = ethernet_env @@ -99,7 +126,7 @@ def test_ethernet_menu_muted(ethernet_env, snapshot): def test_ethernet_menu_cable_disconnected(ethernet_env, snapshot): - """No carrier → dialog reports the cable is disconnected, no toggle row.""" + """No carrier → dialog reports the cable is disconnected.""" lcd, em, _ = ethernet_env em.carrier_up = False _open(lcd) @@ -111,22 +138,6 @@ def test_ethernet_menu_cable_disconnected(ethernet_env, snapshot): # --------------------------------------------------------------------------- -def test_enable_calls_start_service(ethernet_env): - lcd, em, _ = ethernet_env - em.service_active = False - menu = _open(lcd) - menu._on_toggle_service() - assert em.start_calls == 1 - - -def test_disable_calls_stop_service(ethernet_env): - lcd, em, _ = ethernet_env - em.service_active = True - menu = _open(lcd) - menu._on_toggle_service() - assert em.stop_calls == 1 - - def test_toggle_mute_when_unmuted_calls_mute(ethernet_env): lcd, em, mute = ethernet_env em.service_active = True @@ -145,14 +156,20 @@ def test_toggle_mute_when_muted_calls_unmute(ethernet_env): assert mute.is_muted() is False +def test_menu_has_no_enable_disable_button(ethernet_env): + """The Enable/Disable verb went with the Mac-owns-lifecycle change. + Nothing on this screen may let a musician toggle the service.""" + lcd, em, _ = ethernet_env + em.service_active = False + menu = _open(lcd) + assert menu._panel is not None + labels = [w.text for w in menu._panel.sel_list] + assert "Enable" not in labels + assert "Disable" not in labels + + # --------------------------------------------------------------------------- # In-place update regression tests -# -# The dialog used to be torn down and rebuilt on every 2-second tick and on -# every button press, which both (a) destroyed the widget under the user's -# finger and (b) forced a full-screen redraw. The current implementation -# mutates a small set of widgets in place via set_text(), which takes the -# per-widget dirty-rect path. These tests guard that contract. # --------------------------------------------------------------------------- @@ -164,8 +181,6 @@ def test_tick_does_not_rebuild_panel(ethernet_env): first_panel = menu._panel menu.tick() assert menu._panel is first_panel, "tick() must not pop+rebuild the dialog" - # The xrun widgets must still be the same instances — that's how we know - # set_text mutated in place rather than _render recreating them. assert len(menu._xrun_widgets) == 3 widget_ids = [id(w) for w in menu._xrun_widgets] menu.tick() @@ -194,20 +209,6 @@ def test_tick_noop_when_service_inactive(ethernet_env): assert menu._panel is first_panel # still untouched -def test_toggle_service_updates_button_label(ethernet_env): - lcd, em, _ = ethernet_env - em.service_active = False - menu = _open(lcd) - toggle = menu._toggle_btn - assert toggle is not None - assert toggle.text == "Enable" - menu._on_toggle_service() - assert menu._panel is toggle.parent # no rebuild - assert toggle.text == "Disable" # mutated in place - menu._on_toggle_service() - assert toggle.text == "Enable" - - def test_toggle_mute_updates_button_label(ethernet_env): lcd, em, mute = ethernet_env em.service_active = True @@ -258,15 +259,3 @@ def test_back_pops_panel(ethernet_env): menu._on_back() assert menu._panel is None assert lcd.pstack.current is not panel - - -def test_enable_then_state_flip_shows_disable(ethernet_env): - """After Enable fires, the toggle button optimistically reads 'Disable'.""" - lcd, em, _ = ethernet_env - em.service_active = False - menu = _open(lcd) - menu._on_toggle_service() # StubFake flips service_active to True synchronously - # Find the toggle widget by walking the panel's selectable list. - assert menu._panel is not None - labels = [w.text for w in menu._panel.sel_list] - assert "Disable" in labels diff --git a/ui/ethernet_menu.py b/ui/ethernet_menu.py index fbf96385c..1b2c488b0 100644 --- a/ui/ethernet_menu.py +++ b/ui/ethernet_menu.py @@ -51,30 +51,31 @@ class _EthernetHost(Protocol): class EthernetMenu: - """The Wired Connection sub-screen: status readout + enable/disable toggle. + """The Wired Connection sub-screen. Status readout only. - A single Dialog is pushed onto the panel stack; re-renders are done by - popping and rebuilding (mirroring WifiMenu.notify_status_change). State - comes from EthernetManager, which polls carrier + service-active on a - background thread; this class touches the panel stack only from the UI - thread (via handler poll-loop callbacks). + The Enable/Disable toggle is gone (see modalapi/ethernet/manager.py). + pi-stomp-jackbridge starts when the Mac asks for it and stays under + systemd supervision. A musician has nothing to toggle here. The repair + verbs live in the Mac menu. + + One Dialog sits on the panel stack. A re-render pops and rebuilds it + (like WifiMenu.notify_status_change). State comes from EthernetManager, + which polls on a background thread. This class touches the panel stack + only from the UI thread. """ def __init__(self, lcd: "Lcd") -> None: self.lcd: "Lcd" = lcd self._panel: Optional[Dialog] = None - # Remembered across pop/rebuild so periodic re-renders don't yank focus - # back to the toggle button after the user has moved selection. - # One of: 'back', 'toggle', 'mute', or None. + # Kept across a pop/rebuild so a re-render holds the user's selection. + # One of: 'back', 'mute', or None. self._last_selected_role: Optional[str] = None self._role_widgets: dict[str, object] = {} - # Refs to widgets that mutate in place — reset on every _render(). - # tick() updates the xrun rows via set_text(); the action handlers - # update their own button label the same way. None of these paths - # rebuild the dialog, so the buttons don't vanish under the user's - # finger and the SPI blit stays a precise clip. + # Widgets that mutate in place. Reset on every _render(). tick() + # updates the xrun rows with set_text(); the mute handler updates its + # own label the same way. Neither path rebuilds the dialog, so a + # button never moves under the user's finger. self._xrun_widgets: list[TextWidget] = [] - self._toggle_btn: Optional[TextWidget] = None self._mute_btn: Optional[TextWidget] = None def _capture_selected_role(self) -> None: @@ -117,10 +118,10 @@ def open(self, event: object = None, widget: object = None) -> None: self._render() def notify_change(self) -> None: - """Carrier or service-active flipped — re-render if we're on top. + """Carrier or service-active changed. Re-render if this panel is on top. - If the cable was pulled, pop the sub-screen and surface the - disconnected dialog so the user isn't left looking at a stale IP.""" + If the cable was pulled, pop the sub-screen and show the + disconnected dialog so no stale IP stays on screen.""" if self._panel is None or self._pstack.current is not self._panel: return if not self._manager.carrier_up: @@ -132,11 +133,11 @@ def notify_change(self) -> None: self._render() def tick(self) -> None: - """Update the xrun counters while we're on top, without rebuilding - the dialog. The rest of the rows and the buttons are static between - state flips; mutating them via set_text() takes the per-widget - dirty-rect path so the buttons stay put (no reblit under the user's - finger, no full-screen redraw on the SPI bus).""" + """Update the xrun counters in place while this panel is on top. + + The other rows and the buttons are static between state changes. + set_text() takes the per-widget dirty-rect path, so the buttons stay + put and the SPI bus sees no full redraw.""" if self._panel is None or self._pstack.current is not self._panel: return if not self._manager.carrier_up: @@ -159,13 +160,14 @@ def _render(self) -> None: self._panel = None self._pstack.pop_panel(old) - # Old widgets were just destroyed — drop our refs so tick() / actions - # don't try to mutate zombies if a render races a poll. + # The old widgets are gone. Drop the refs so a render that races a + # poll does not touch a dead widget. self._xrun_widgets = [] - self._toggle_btn = None self._mute_btn = None active = self._manager.service_active + n_adapters, n_wired, route = self._manager.read_netadapter_health() + resyncing, restarts = self._manager.read_link_health() d = Dialog(width=DIALOG_W, height=DIALOG_H, title="Ethernet Audio Interface", auto_destroy=True) font = _make_font(_FONTS_DIR / "DejaVuSans.ttf", 14) @@ -179,9 +181,19 @@ def _render(self) -> None: rows.append(("xruns 1m:", str(b1))) rows.append(("xruns 5m:", str(b5))) rows.append(("xruns 15m:", str(b15))) + if resyncing: + # The ports stay wired through a netadapter restart, so the + # port count (unfortunately) continues to read healthy + rows.append(("Link:", f"⚠ resyncing (x{restarts})")) + rows.append(("", "Restart JackBridge on Host")) + else: + rows.append(("Link ports:", f"{n_wired}/6 wired")) + if n_adapters > 1: + rows.append(("Adapters:", f"⚠ {n_adapters} (duplicate)")) + if not route or route.startswith("wl"): + rows.append(("Route:", f"⚠ {route or 'none'}")) muted = self._mute.is_muted() - toggle_label = "Disable" if active else "Enable" mute_label = "Unmute MOD" if muted else "Mute MOD" line_h = 18 @@ -215,29 +227,8 @@ def _render(self) -> None: ) d.add_sel_widget(back_btn) - # Toggle sits to the right of back; constructed with the wider of the - # two labels so the box is the same size in both states. Without - # this, swapping "Enable"→"Disable" via set_text() would clip the - # trailing "e" inside the original (narrower) box. - assert back_btn.box - toggle_x = back_btn.box.x0 + back_btn.box.width + 6 - toggle_btn = TextWidget( - box=Box.xywh(toggle_x, btn_y, 0, 0), - text="Disable", - parent=d, - outline=1, - sel_width=3, - outline_radius=5, - action=self._on_toggle_service, - align=WidgetAlign.NONE, - name="ethernet_toggle_btn", - ) - toggle_btn.set_text(toggle_label) - d.add_sel_widget(toggle_btn) - self._toggle_btn = toggle_btn - - # Mute button: same trick — size to fit "Unmute MOD" so a set_text - # back to "Mute MOD" doesn't leave dead space at the right edge. + # Size the mute button for "Unmute MOD" so a set_text back to + # "Mute MOD" leaves no dead space at the right edge. mute_btn = TextWidget( box=Box.xywh(0, btn_y, 0, 0), text="Unmute MOD", @@ -257,13 +248,13 @@ def _render(self) -> None: d.add_sel_widget(mute_btn) self._mute_btn = mute_btn - # Stash refs by role so re-renders can preserve selection (panel pop - # blows away widget identity, so we track which role was selected). - self._role_widgets = {"back": back_btn, "toggle": toggle_btn, "mute": mute_btn} + # Track which role holds the selection. A panel pop destroys widget + # identity, so a re-render restores selection by role, not by ref. + self._role_widgets = {"back": back_btn, "mute": mute_btn} - # Restore selection from before the rebuild when possible, so periodic - # ticks and unrelated actions (Mute) don't drag focus back to Toggle. - restore_target = self._role_widgets.get(self._last_selected_role or "toggle", toggle_btn) + # Keep the selection across a rebuild so ticks and the Mute action + # do not move the focus. + restore_target = self._role_widgets.get(self._last_selected_role or "mute", mute_btn) d.sel_widget(restore_target) self._panel = d @@ -275,20 +266,8 @@ def _show_disconnected_dialog(self) -> None: # ----- actions ----- - def _on_toggle_service(self, _event: object = None, _widget: object = None) -> None: - # Optimistic update: show the *new* state immediately, before the - # background poll observes systemctl's effect. The bg poll's - # notify_change() will re-render the dialog (full rebuild is needed - # there anyway, because the row *set* changes when service_active - # flips) and reconcile any drift. - if self._manager.service_active: - self._manager.stop_service() - new_label = "Enable" - else: - self._manager.start_service() - new_label = "Disable" - if self._toggle_btn is not None: - self._toggle_btn.set_text(new_label) + # No _on_toggle_service: the Enable/Disable button is gone. The Mac menu + # owns start and stop; this screen only renders. def _on_toggle_mute(self, _event: object = None, _widget: object = None) -> None: if self._mute.is_muted():