diff --git a/common/util.py b/common/util.py index 06a6fb660..2d6d710da 100755 --- a/common/util.py +++ b/common/util.py @@ -18,6 +18,9 @@ import math from typing import Any +# Teardown join budget for subprocesses +TEARDOWN_JOIN_S = 0.25 + def LILV_FOREACH(collection, func): itr = collection.begin() diff --git a/emulator/modhandler.py b/emulator/modhandler.py index e66b55334..7dbf410fc 100644 --- a/emulator/modhandler.py +++ b/emulator/modhandler.py @@ -106,6 +106,7 @@ def system_menu_reboot(self, arg): def system_menu_restart_sound(self, arg): logging.info("Emulator: restart sound is a no-op") + raise KeyboardInterrupt def system_menu_reload(self, arg): logging.info("Emulator: reload configs is a no-op") diff --git a/modalapi/ethernet/manager.py b/modalapi/ethernet/manager.py index f4e59cdc2..3e3a22644 100644 --- a/modalapi/ethernet/manager.py +++ b/modalapi/ethernet/manager.py @@ -23,6 +23,7 @@ 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 @@ -76,7 +77,7 @@ def __init__(self) -> None: def shutdown(self) -> None: self._stop.set() - self._thread.join(timeout=2.0) + self._thread.join(timeout=TEARDOWN_JOIN_S) # ----- background polling ----- diff --git a/modalapi/modhandler.py b/modalapi/modhandler.py index 36722ee91..d2e81f3e6 100644 --- a/modalapi/modhandler.py +++ b/modalapi/modhandler.py @@ -284,9 +284,7 @@ def _rest_get_with_retry(self, url: str) -> Response | None: resp = self._rest_get(url) if resp is not None and resp.status_code == 200: return resp - logging.info( - "mod-ui not ready, retrying (%d/%d) in %ss...", attempt, len(STARTUP_REST_BACKOFF_S), delay - ) + logging.info("mod-ui not ready, retrying (%d/%d) in %ss...", attempt, len(STARTUP_REST_BACKOFF_S), delay) time.sleep(delay) return self._rest_get(url) @@ -450,9 +448,7 @@ def _handle_switch(self, event: SwitchEvent) -> bool: # ControllerManager._bind_encoder_longpress. if event.kind == SwitchEventKind.LONGPRESS and controller.midi_CC is not None: key = f"{controller.midi_channel}:{controller.midi_CC}" - winner = self.effective_table.resolve( - ControlRef(cls=ControlClass.ANALOG, id=key), EventKind.LONGPRESS - ) + winner = self.effective_table.resolve(ControlRef(cls=ControlClass.ANALOG, id=key), EventKind.LONGPRESS) if winner is not None: self._fire_row(winner, event) return True @@ -467,9 +463,7 @@ def _handle_footswitch(self, fs: Footswitch, kind: SwitchEventKind, timestamp: f Handler._handle_footswitch imperative if-chain.""" if kind == SwitchEventKind.LONGPRESS: key = fs.dispatch_key - winner = self.effective_table.resolve( - ControlRef(cls=ControlClass.FOOTSWITCH, id=key), EventKind.LONGPRESS - ) + winner = self.effective_table.resolve(ControlRef(cls=ControlClass.FOOTSWITCH, id=key), EventKind.LONGPRESS) if winner is not None: self._fire_row(winner, SwitchEvent(controller=fs, kind=kind, timestamp=timestamp)) return True @@ -479,9 +473,7 @@ def _handle_footswitch(self, fs: Footswitch, kind: SwitchEventKind, timestamp: f # Short press key = fs.dispatch_key - winner = self.effective_table.resolve( - ControlRef(cls=ControlClass.FOOTSWITCH, id=key), EventKind.PRESS - ) + winner = self.effective_table.resolve(ControlRef(cls=ControlClass.FOOTSWITCH, id=key), EventKind.PRESS) if winner is not None: self._fire_row(winner, SwitchEvent(controller=fs, kind=kind, timestamp=timestamp)) return True @@ -1245,7 +1237,7 @@ def _publish_bpm(self, param: Parameter) -> bool: return self.set_mod_tap_tempo(param.value) def _publish_audio(self, param: Parameter) -> bool: - """ A local ALSA write. No remote echo, so the send always lands.""" + """A local ALSA write. No remote echo, so the send always lands.""" self.audio_parameter_commit(param.symbol, param.value) return True @@ -1521,6 +1513,7 @@ def maybe_show_welcome(self): if self.settings.get_setting(Token.WELCOME_SEEN): return from ui.welcome import WelcomePanel + self.lcd.pstack.push_panel(WelcomePanel(self)) def get_software_version(self) -> str: @@ -1545,16 +1538,14 @@ def recovery_available(self) -> bool: ) def system_menu_shutdown(self, arg): - self.lcd.splash_show(False) logging.info("System Shutdown") - os.system("sudo systemctl --no-wall poweroff") - os._exit(0) + self.lcd.draw_message_dialog("Shutting down…", title="Please wait", dismissable=False) + os.system("sudo systemctl --no-wall --no-block poweroff") def system_menu_reboot(self, arg): - self.lcd.splash_show(False) logging.info("System Reboot") - os.system("sudo systemctl reboot") - os._exit(0) + self.lcd.draw_message_dialog("Rebooting…", title="Please wait", dismissable=False) + os.system("sudo systemctl --no-wall --no-block reboot") def system_menu_recovery_mode(self, arg): self.lcd.draw_info_message("Entering recovery mode...", refresh=True) @@ -1752,8 +1743,8 @@ def restart_ui_stack(self) -> None: logging.error("restart_ui_stack: %s", e) def system_menu_restart_sound(self, arg): - self.lcd.splash_show() logging.info("Restart sound engine (jack)") + self.lcd.draw_message_dialog("Restarting sound engine…", title="Please wait", dismissable=False) os.system("sudo systemctl restart jack") def system_disable_eq(self): diff --git a/modalapi/websocket_bridge.py b/modalapi/websocket_bridge.py index 45de1c348..6a8b22a7e 100644 --- a/modalapi/websocket_bridge.py +++ b/modalapi/websocket_bridge.py @@ -33,6 +33,7 @@ import websockets import uvloop from common.parameter import Symbol +from common.util import TEARDOWN_JOIN_S # Service will restart after this MAX_RECONNECT_ATTEMPTS = 4 @@ -286,7 +287,7 @@ def stop(self): self._worker.running = False self._worker.signal_stop() if self._thread and not sys.is_finalizing(): - self._thread.join(timeout=2.0) + self._thread.join(timeout=TEARDOWN_JOIN_S) logging.info(f"WebSocket worker stopped (sent={self._worker.messages_sent})") def send_bpm(self, bpm: float) -> bool: diff --git a/modalapi/wifi/commands.py b/modalapi/wifi/commands.py index 0abe37e5e..09da43b09 100644 --- a/modalapi/wifi/commands.py +++ b/modalapi/wifi/commands.py @@ -22,6 +22,8 @@ from dataclasses import dataclass from typing import TYPE_CHECKING, Any, Callable, Generic, Optional, TypeVar +from common.util import TEARDOWN_JOIN_S + if TYPE_CHECKING: from .manager import WifiManager @@ -201,4 +203,4 @@ def pending_op_count(self) -> int: def shutdown(self) -> None: self._cmd_queue.put(_SHUTDOWN_SENTINEL) - self._worker.join(timeout=2.0) + self._worker.join(timeout=TEARDOWN_JOIN_S) diff --git a/modalapi/wifi/manager.py b/modalapi/wifi/manager.py index 46ce2b98c..9683c473e 100644 --- a/modalapi/wifi/manager.py +++ b/modalapi/wifi/manager.py @@ -20,6 +20,8 @@ import threading from typing import Callable, Optional +from common.util import TEARDOWN_JOIN_S + from . import ops from .commands import CommandQueue from .nmcli import nmcli, parse_kv_lines @@ -72,7 +74,7 @@ def shutdown(self) -> None: except Exception: pass if self.thread is not None: - self.thread.join(timeout=2.0) + self.thread.join(timeout=TEARDOWN_JOIN_S) def _is_wifi_supported(self) -> bool: if self.wireless_supported: diff --git a/pistomp/lcd320x240.py b/pistomp/lcd320x240.py index 569bbc393..1cb6afe61 100644 --- a/pistomp/lcd320x240.py +++ b/pistomp/lcd320x240.py @@ -158,12 +158,9 @@ def __init__(self, cwd, handler: "Modhandler", flip=False, display=None, spi_spe # Colors self.background = (0, 0, 0) self.foreground = (255, 255, 255) - self.color_splash_up = (70, 255, 70) - self.color_splash_down = (255, 20, 20) # TODO get fonts from config.json self.title_font = _make_font(font_path("DejaVuSans-Bold.ttf"), 26) - self.splash_font = _make_font(font_path("DejaVuSans.ttf"), 48) self.small_font = _make_font(font_path("DejaVuSans.ttf"), 20) self.tiny_font = _make_font(font_path("DejaVuSans.ttf"), 16) self.subtitle_font = _make_font(font_path("DejaVuSans.ttf"), 14) @@ -199,7 +196,6 @@ def __init__(self, cwd, handler: "Modhandler", flip=False, display=None, spi_spe self.grid_panel: Optional[GridPanel] = None self.w_footswitches = [] self.w_controls = [] - self.w_splash = None self.w_info_msg = None self.w_subtitle: Optional[Subtitle] = None self._subtitle_desc = "" # last selection description seen @@ -208,8 +204,6 @@ def __init__(self, cwd, handler: "Modhandler", flip=False, display=None, spi_spe # panels self.pstack = PanelStack(display, image_format="RGB", use_dimming=True) - self.splash_panel = Panel(box=Box.xywh(0, 0, self.display_width, self.display_height)) - self.pstack.push_panel(self.splash_panel, refresh=False) self.main_panel = Panel( box=Box.xywh(0, 0, self.display_width, self.display_height), persist_on_board_change=True ) @@ -233,9 +227,6 @@ def __init__(self, cwd, handler: "Modhandler", flip=False, display=None, spi_spe # the PanelStack, which is created earlier in this block. self.wifi_menu: WifiMenu = WifiMenu(self) - if not display.has_system_splash: - self.splash_show(True) - # # Main # @@ -330,9 +321,7 @@ def _poll_updates(self): elif isinstance(icon.object, EncoderController): enc = icon.object midi_value = ( - enc.bar_midi_value() - if enc.parameter is not None - else self.handler.encoder_fallback(enc) + enc.bar_midi_value() if enc.parameter is not None else self.handler.encoder_fallback(enc) ) elif isinstance(icon.object, BlendMode): ic = icon.object.input_controller @@ -588,8 +577,8 @@ def menu_action(event, params): self.pstack.push_panel(m) return m - def draw_message_dialog(self, text, title="Error", on_dismiss=None): - d = MessageDialog(self.pstack, text, title=title, on_dismiss=on_dismiss) + def draw_message_dialog(self, text, title="Error", on_dismiss=None, dismissable=True): + d = MessageDialog(self.pstack, text, title=title, on_dismiss=on_dismiss, dismissable=dismissable) self.pstack.push_panel(d) def draw_plugins(self): @@ -1083,18 +1072,6 @@ def draw_vu_calibration_dialog(self, symbol, value, commit_callback): # # General # - def splash_show(self, boot=True): - color = self.color_splash_up if boot else self.color_splash_down - if self.w_splash is None: - self.w_splash = TextWidget( - box=Box.xywh(12, 80, self.display_width, self.display_height), - text="pi Stomp!", - font=self.splash_font, - parent=self.splash_panel, - ) - self.w_splash.set_foreground(color) - self.splash_panel.refresh() - def cleanup(self): # Walk every input-accepting panel (dialogs, tuner, plugin panels, …) # so buried panels are destroyed too, not just the top-most one. @@ -1104,7 +1081,7 @@ def cleanup(self): self.pstack.pop_panel(self.footswitch_panel) if self.main_panel_pushed and self.main_panel in self.pstack.stack: self.pstack.pop_panel(self.main_panel) - self.splash_show(False) + self.pstack.refresh() # black screen def clear(self): pass @@ -1252,8 +1229,12 @@ def draw_analog_assignments(self, controllers): if k is None: # Non-mapped control name = "none" - control_type = ControlType.EXPRESSION if i == 0 else ControlType.KNOB # HACK cuz we don't know type of unmapped - subtitle = "Expression pedal (unassigned)" if control_type == ControlType.EXPRESSION else "Knob (unassigned)" + control_type = ( + ControlType.EXPRESSION if i == 0 else ControlType.KNOB + ) # HACK cuz we don't know type of unmapped + subtitle = ( + "Expression pedal (unassigned)" if control_type == ControlType.EXPRESSION else "Knob (unassigned)" + ) color = accent_color_for(None) text_color = color control_label_fn = None diff --git a/plugins/audio_midi/panel.py b/plugins/audio_midi/panel.py index d768a38d1..0f6fb8903 100644 --- a/plugins/audio_midi/panel.py +++ b/plugins/audio_midi/panel.py @@ -681,7 +681,8 @@ def _on_toggle_mute(self) -> None: def _on_restart(self) -> None: # Mirrors handler.system_menu_restart_sound — restarts jack (which - # cascades to mod-host/mod-ui). The splash covers the teardown. + # cascades to mod-host/mod-ui). The uncancellable dialog covers the + # teardown until this service is SIGTERMed. self._handler.system_menu_restart_sound(None) def wants_fast_tick(self) -> bool: diff --git a/tests/integration/test_system_menu.py b/tests/integration/test_system_menu.py index 67fddf3c3..569ba23bc 100644 --- a/tests/integration/test_system_menu.py +++ b/tests/integration/test_system_menu.py @@ -13,18 +13,44 @@ def test_system_menu_shutdown(modhandler_system: SystemFixture): handler = modhandler_system.handler - with patch.object(handler.lcd, "cleanup"), patch("os.system") as mock_os, patch("os._exit") as mock_exit: + with patch("os.system") as mock_os: handler.system_menu_shutdown(None) - mock_os.assert_called_once_with("sudo systemctl --no-wall poweroff") - mock_exit.assert_called_once_with(0) + mock_os.assert_called_once_with("sudo systemctl --no-wall --no-block poweroff") + assert handler.lcd.pstack.current is not None # uncancellable dialog stays up def test_system_menu_reboot(modhandler_system: SystemFixture): handler = modhandler_system.handler - with patch("os.system") as mock_os, patch("os._exit") as mock_exit: + with patch("os.system") as mock_os: handler.system_menu_reboot(None) - mock_os.assert_called_once_with("sudo systemctl reboot") - mock_exit.assert_called_once_with(0) + mock_os.assert_called_once_with("sudo systemctl --no-wall --no-block reboot") + assert handler.lcd.pstack.current is not None # uncancellable dialog stays up + + +def test_shutdown_survives_polls_before_sigterm(modhandler_system: SystemFixture): + """The loop keeps ticking between the poweroff request and systemd's SIGTERM.""" + handler = modhandler_system.handler + with patch("os.system"): + handler.system_menu_shutdown(None) + handler.lcd.update_bypass(True, True) + handler.lcd.update_wifi({"hotspot_active": False, "wifi_connected": True}) + handler.poll_lcd_updates() + assert handler.lcd.pstack.current is not None # dialog still up, not black-screened + + +def test_uncancellable_message_dialog_has_no_ok_button(modhandler_system: SystemFixture): + """dismissable=False drops the Ok button and its selectable — nothing to + press, so the dialog swallows everything and can't be dismissed.""" + handler = modhandler_system.handler + from pistomp.input.event import EncoderEvent + from tests.v3.nav_helpers import nav_encoder + + handler.lcd.draw_message_dialog("Please wait…", dismissable=False) + d = handler.lcd.pstack.current + assert d is not None + assert d.sel_ref is None # no selectable Ok button + + assert d.handle(EncoderEvent(controller=nav_encoder(handler), rotations=1)) is True # NAV swallowed def test_system_menu_reload(modhandler_system: SystemFixture): diff --git a/tests/snapshots/test_lcd320x240/test_busy_dialog_snapshot/busy_dialog.png b/tests/snapshots/test_lcd320x240/test_busy_dialog_snapshot/busy_dialog.png new file mode 100644 index 000000000..c2fd9b77f Binary files /dev/null and b/tests/snapshots/test_lcd320x240/test_busy_dialog_snapshot/busy_dialog.png differ diff --git a/tests/snapshots/test_lcd320x240/test_cleanup_blacks_the_panel/0.png b/tests/snapshots/test_lcd320x240/test_cleanup_blacks_the_panel/0.png new file mode 100644 index 000000000..1307b6450 Binary files /dev/null and b/tests/snapshots/test_lcd320x240/test_cleanup_blacks_the_panel/0.png differ diff --git a/tests/snapshots/test_lcd320x240/test_splash_snapshot/0.png b/tests/snapshots/test_lcd320x240/test_splash_snapshot/0.png deleted file mode 100644 index 52245757c..000000000 Binary files a/tests/snapshots/test_lcd320x240/test_splash_snapshot/0.png and /dev/null differ diff --git a/tests/test_lcd320x240.py b/tests/test_lcd320x240.py index 06bf68a57..15639566f 100644 --- a/tests/test_lcd320x240.py +++ b/tests/test_lcd320x240.py @@ -62,8 +62,13 @@ def _make_pedalboard(title: str, plugins: list[Plugin], connections: list[Connec def _make_footswitch(id: int, toggled: bool = False, display_label: str = "", taptempo=None) -> Footswitch: fs = Footswitch( - id=id, led_pin=None, pixel=None, midi_CC=1, midi_channel=0, - refresh_callback=lambda *a, **k: None, taptempo=taptempo, + id=id, + led_pin=None, + pixel=None, + midi_CC=1, + midi_channel=0, + refresh_callback=lambda *a, **k: None, + taptempo=taptempo, ) fs.toggled = toggled fs.display_label = display_label @@ -132,15 +137,25 @@ def setup_main_ui(instance): mock_mix = _real_param(name="Mix", symbol="mix", instance_id="reverb", value=0.4) plugins = [ _make_plugin( - "distortion", uri="mock://distortion", category="Distortion", has_footswitch=True, + "distortion", + uri="mock://distortion", + category="Distortion", + has_footswitch=True, parameters={Symbol("gain"): mock_gain}, ), _make_plugin( - "delay", uri="mock://delay", category="Delay", has_footswitch=True, + "delay", + uri="mock://delay", + category="Delay", + has_footswitch=True, parameters={Symbol("time"): mock_time}, ), _make_plugin( - "reverb", uri="mock://reverb", category="Reverb", has_footswitch=True, bypassed=True, + "reverb", + uri="mock://reverb", + category="Reverb", + has_footswitch=True, + bypassed=True, parameters={Symbol("mix"): mock_mix}, ), _make_plugin("chorus", uri="mock://chorus", category="Modulator", has_footswitch=False), @@ -167,10 +182,12 @@ def setup_main_ui(instance): instance.draw_main_panel() -def test_splash_snapshot(lcd, snapshot): - _, fake = lcd +def test_cleanup_blacks_the_panel(lcd, snapshot): + instance, fake = lcd + setup_main_ui(instance) + instance.cleanup() fake.flush() - assert len(fake.frames) > 0, "expected at least one frame from splash_show during __init__" + assert instance.pstack.stack == [] snapshot() @@ -213,6 +230,14 @@ def test_system_menu_snapshot(lcd, snapshot): snapshot() +def test_busy_dialog_snapshot(lcd, snapshot): + """Uncancellable "Please wait" dialog: message only, no Ok button.""" + instance, _ = lcd + setup_main_ui(instance) + instance.draw_message_dialog("Restarting sound engine…", title="Please wait", dismissable=False) + snapshot("busy_dialog") + + def test_system_info_dialog_snapshot(lcd, snapshot): """The System Info MessageDialog must show all 5 lines without clipping.""" instance, _ = lcd diff --git a/uilib/dialog.py b/uilib/dialog.py index ac27bb928..e35f693db 100644 --- a/uilib/dialog.py +++ b/uilib/dialog.py @@ -38,6 +38,7 @@ class DialogScheme: When None is passed to Dialog/DialogDecorator, the current Config defaults are used (system menus render pixel-identical). """ + title_fgnd: tuple[int, int, int] title_bkgnd: tuple[int, int, int] outline_color: tuple[int, int, int] @@ -130,7 +131,9 @@ def __init__(self, width, height, title, title_font=None, scheme=None, **kwargs) if title_font is None: title_font = Config().get_font("default_title") self._title_strip_h = get_text_size(title, title_font)[1] + 2 - deco = functools.partial(DialogDecorator, title=title, title_font=title_font, outline_radius=radius, scheme=scheme) + deco = functools.partial( + DialogDecorator, title=title, title_font=title_font, outline_radius=radius, scheme=scheme + ) super(Dialog, self).__init__(box=box, align=WidgetAlign.CENTRE, radius=radius, decorator=deco, **kwargs) def _adjust_box(self): @@ -155,9 +158,14 @@ def tick(self) -> None: class MessageDialog(Dialog): - def __init__(self, panelstack, message, title="Error", width=200, height=90, on_dismiss=None): - super(MessageDialog, self).__init__(width=width, height=height, title=title, auto_destroy=True) - + def __init__(self, panelstack, message, title="Error", width=200, height=90, on_dismiss=None, dismissable=True): + super(MessageDialog, self).__init__( + width=width, + height=height, + title=title, + auto_destroy=True, + dismissable=dismissable, + ) font = Config().get_font("default_title") char_w = font.get_rect("a").width if font else 0 chars_per_line = width // max(1, int(char_w)) @@ -168,8 +176,11 @@ def __init__(self, panelstack, message, title="Error", width=200, height=90, on_ num_lines = wrapped.count("\n") + 1 text_h = line_h * num_lines text_box_h = max(0, min(text_h, height - 34)) + # No Ok button — center the message in the body instead of leaving a + # top-heavy block above the empty button slot. + y_offset = (height - text_box_h) // 2 if not dismissable else 0 t = TextWidget( - box=Box.xywh(5, 0, width - 10, text_box_h), + box=Box.xywh(5, y_offset, width - 10, text_box_h), text=wrapped, parent=self, outline=0, @@ -183,19 +194,20 @@ def _dismiss(x, y): if on_dismiss: on_dismiss() - b = TextWidget( - box=Box.xywh(int((width / 2) - 20), height - 30, 0, 0), - text="Ok", - parent=self, - outline=1, - sel_width=3, - outline_radius=5, - action=_dismiss, - align=WidgetAlign.NONE, - name="ok_btn", - ) - self.add_sel_widget(b) - self.sel_widget(b) + if dismissable: + b = TextWidget( + box=Box.xywh(int((width / 2) - 20), height - 30, 0, 0), + text="Ok", + parent=self, + outline=1, + sel_width=3, + outline_radius=5, + action=_dismiss, + align=WidgetAlign.NONE, + name="ok_btn", + ) + self.add_sel_widget(b) + self.sel_widget(b) class ConfirmDialog(Dialog): diff --git a/uilib/lcd_ili9341.py b/uilib/lcd_ili9341.py index fcce1dc2e..c40aa9c48 100644 --- a/uilib/lcd_ili9341.py +++ b/uilib/lcd_ili9341.py @@ -142,10 +142,6 @@ def _block_fast(self, x0, y0, x1, y1, data=None): # Release lock once spi.unlock() - @property - def has_system_splash(self) -> bool: - return has_system_splash() - def _set_stamp(self): try: with open(INIT_STAMP, "w") as _f: diff --git a/uilib/panel.py b/uilib/panel.py index 8c4adc491..a0f4bb232 100644 --- a/uilib/panel.py +++ b/uilib/panel.py @@ -404,11 +404,6 @@ def transfer_ms(self, box: Optional[Box] = None) -> float: """Estimated ms to push a clip of this box's size. 0 = no cost.""" return 0.0 - @property - def has_system_splash(self) -> bool: - return False - - class PanelStack(ContainerWidget): # A push estimated to take longer than this is coalesced rather than pushed # inline, leaving headroom under the 10ms tick.