From 240fc4f0dee66bdc81a8ede80cc0002d76e14eaf Mon Sep 17 00:00:00 2001 From: frozenwizard <172203+frozenwizard@users.noreply.github.com> Date: Fri, 10 Jul 2026 09:11:48 -0500 Subject: [PATCH 1/2] Track lightshow status for the UI via LightshowManager MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The manager now records what the model is doing, not just whether a task exists: - States: idle, running, completed, stopped, errored, plus finished for one-shot actions like "All LEDs on". Each show runs under a supervisor coroutine that records its outcome — MicroPython's Task has no done-callbacks or exception() to inspect after the fact. - Shows are started by their config name, so the start/stop responses and the index page report the real show name instead of "wrapper". - Manual LED routes (/led//on|off, /all/on|off) go through the manager: they cancel any running show first, so a show can no longer fight the user over the LEDs, and the UI shows the action as the last activity. - New /lightshow/status JSON route; index renders a status one-liner. - Show errors are recorded by the supervisor instead of being swallowed as an unread task result by safe_execution, which now only guards request handlers. lightshow_route/RouteDecorator.py retired; the manager lives in src/server/lightshow_manager.py. Co-Authored-By: Claude Fable 5 --- src/server/RouteDecorator.py | 72 ------------------- src/server/Wrappers.py | 20 +++--- src/server/lightshow_manager.py | 123 ++++++++++++++++++++++++++++++++ src/server/webserver.py | 48 +++++++------ src/templates/index.html | 8 +-- tests/test_virtual_hardware.py | 71 +++++++++++++++--- 6 files changed, 222 insertions(+), 120 deletions(-) delete mode 100644 src/server/RouteDecorator.py create mode 100644 src/server/lightshow_manager.py diff --git a/src/server/RouteDecorator.py b/src/server/RouteDecorator.py deleted file mode 100644 index 8448d3c..0000000 --- a/src/server/RouteDecorator.py +++ /dev/null @@ -1,72 +0,0 @@ -import asyncio - - -class LightshowManager: - """ - Owns the single running lightshow task for a gunpla. start() and stop() are - serialized with a lock so overlapping requests can't orphan a running show - or leave two shows fighting over the same LEDs. - """ - - def __init__(self, gunpla): - self.gunpla = gunpla - self._task = None - self._lock = asyncio.Lock() - - def is_running(self) -> bool: - """ - :return: True if a lightshow is currently running, False otherwise - """ - return self._task is not None and not self._task.done() - - async def start(self, func) -> None: - """ - Cancels any running show (turning the LEDs off), then starts func as the - tracked lightshow task. - """ - async with self._lock: - if await self._cancel_current(): - self.gunpla.all_off() - self._task = asyncio.create_task(func()) - - async def stop(self) -> bool: - """ - Cancels any running lightshow and waits for it to clean up. - :return: True if a running show was cancelled, False if nothing was running - """ - async with self._lock: - return await self._cancel_current() - - async def _cancel_current(self) -> bool: - task = self._task - self._task = None - if task is None or task.done(): - return False - task.cancel() - try: - await task # Wait for cleanup - except asyncio.CancelledError: - pass - except Exception as e: - # A show that died on its own shouldn't fail the cancelling request - print(f"Lightshow ended with error: {e}") - return True - - -def lightshow_route(manager: LightshowManager): - """ - A decorator factory that handles task management and - standardized HTTP responses. - """ - def decorator(func): - async def wrapper(request, *args, **kwargs): - # Replaces any running lightshow with this one. - await manager.start(func) - - # Return common HTTP response that the show started. - return { - "status": "started", - "show": func.__name__, - }, 202 - return wrapper - return decorator diff --git a/src/server/Wrappers.py b/src/server/Wrappers.py index 2c268d3..40c8dde 100644 --- a/src/server/Wrappers.py +++ b/src/server/Wrappers.py @@ -1,6 +1,3 @@ -from src.server.RouteDecorator import lightshow_route - - def safe_execution(func): """ Wraps an async route handler with a try/except block. @@ -23,18 +20,19 @@ async def wrapper(*args, **kwargs): return wrapper -def create_show_handler(func, show_manager): +def create_show_handler(name, func, show_manager): """ - Helper that when given a function, wraps it as a lighthow_route and safe_execution. + Builds a route handler that starts func as the named lightshow through the + show manager. Errors inside the running show itself are recorded by the + manager's supervisor, not here — safe_execution only guards the request. + :param name: the lightshow name from the model config, shown in the UI + :param func: the async lightshow method on the gunpla :param show_manager: the LightshowManager that owns the running show - if needed we can add back in the request obj to show_handler and func(request) - :param func: :return: """ - # note order matters for these - @lightshow_route(show_manager) @safe_execution - async def show_handler(): - return await func() + async def show_handler(request): + await show_manager.start(name, func) + return {"status": "started", "show": name}, 202 return show_handler diff --git a/src/server/lightshow_manager.py b/src/server/lightshow_manager.py new file mode 100644 index 0000000..076d4d3 --- /dev/null +++ b/src/server/lightshow_manager.py @@ -0,0 +1,123 @@ +import asyncio + +# Status states reported to the UI (MicroPython has no enum module) +IDLE = "idle" # nothing has run yet +RUNNING = "running" # a lightshow is currently running +COMPLETED = "completed" # the last lightshow ran to its natural end +STOPPED = "stopped" # the last lightshow was cancelled by the user or replaced +ERRORED = "errored" # the last lightshow raised an exception +FINISHED = "finished" # the last activity was a one-shot action like "All LEDs on" + + +class LightshowManager: + """ + Owns the single running lightshow task for a gunpla and the status shown in + the UI. start/stop/run_action are serialized with a lock so overlapping + requests can't orphan a running show or leave two shows fighting over the + same LEDs. Each show runs under a supervisor coroutine that records how it + ended (completed, stopped, or errored) — outcomes MicroPython's Task object + can't report on its own. + """ + + def __init__(self, gunpla): + self.gunpla = gunpla + self._task = None + self._lock = asyncio.Lock() + self._state = IDLE + self._show = None + self._error = None + + def status(self) -> dict: + """ + :return: the current status as a dict, suitable for JSON responses + """ + return {"state": self._state, "show": self._show, "error": self._error} + + def describe(self) -> str: + """ + :return: a human readable one-liner of the current status for the UI + """ + if self._state == IDLE: + return "No lightshow has run yet." + if self._state == RUNNING: + return f"Lightshow '{self._show}' is running." + if self._state == COMPLETED: + return f"Lightshow '{self._show}' completed." + if self._state == STOPPED: + return f"Lightshow '{self._show}' was stopped." + if self._state == ERRORED: + return f"Lightshow '{self._show}' failed: {self._error}" + return f"{self._show} — finished." + + def is_running(self) -> bool: + """ + :return: True if a lightshow is currently running, False otherwise + """ + return self._task is not None and not self._task.done() + + async def start(self, name: str, func) -> None: + """ + Cancels any running show (turning the LEDs off), then starts func as + the tracked lightshow task under a supervisor that records its outcome. + """ + async with self._lock: + await self._cancel_current() + self._set_status(RUNNING, name) + self._task = asyncio.create_task(self._supervise(name, func)) + + async def stop(self): + """ + Cancels any running lightshow, waits for it to clean up and turns the + LEDs off. + :return: the name of the cancelled show, or None if nothing was running + """ + async with self._lock: + return await self._cancel_current() + + async def run_action(self, description: str, action) -> None: + """ + Runs a one-shot LED action (e.g. "All LEDs on"), cancelling any running + show first so the show can't fight the user over the LEDs. + :param description: what the action is, shown in the UI status + :param action: a plain callable that manipulates the LEDs + """ + async with self._lock: + await self._cancel_current() + action() + self._set_status(FINISHED, description) + + def _set_status(self, state: str, show: str, error: str = None) -> None: + self._state = state + self._show = show + self._error = error + + async def _supervise(self, name: str, func) -> None: + try: + await func() + except asyncio.CancelledError: + self._set_status(STOPPED, name) + raise + except Exception as e: + print(f"Lightshow '{name}' failed: {e}") + self._set_status(ERRORED, name, str(e)) + else: + self._set_status(COMPLETED, name) + + async def _cancel_current(self): + """ + Cancels the tracked show if one is still running, waits for its cleanup + and turns all LEDs off. Callers must hold the lock. + :return: the name of the cancelled show, or None if nothing was running + """ + task = self._task + self._task = None + if task is None or task.done(): + return None + name = self._show + task.cancel() + try: + await task # Wait for cleanup; the supervisor records the outcome + except asyncio.CancelledError: + pass + self.gunpla.all_off() + return name diff --git a/src/server/webserver.py b/src/server/webserver.py index 6fd92d5..5b8ad91 100644 --- a/src/server/webserver.py +++ b/src/server/webserver.py @@ -4,9 +4,9 @@ from src.gunpla.generic_gundam import GenericGundam from src.hardware.Hardware import Hardware from src.pi.led_effect import LEDEffects +from src.server.lightshow_manager import LightshowManager from src.server.microdot.Microdot import Microdot, Request from src.server.microdot.utemplate import Template -from src.server.RouteDecorator import LightshowManager from src.server.Wrappers import create_show_handler, safe_execution @@ -40,12 +40,11 @@ async def index(self, request: Request): """ led_list = [{"name": led.name()} for led in self.gundam.get_all_leds()] show_list = self.gundam.config['lightshow'] - running_show = self._is_lightshow_running() return await Template('index.html').render_async( name_of_title="Gundam LED Control", all_leds=led_list, lightshows=show_list, - running_show=running_show + status_message=self.show_manager.describe() ), 200, {'Content-Type': 'text/html'} @safe_execution @@ -56,22 +55,22 @@ async def canary(self, request: Request): asyncio.create_task(LEDEffects.blink(self.hardware.board_led())) return "chirp", 202 - def all_on(self, request: Request): + async def all_on(self, request: Request): """ - Turns on all LEDs. + Turns on all LEDs, halting any running lightshow first. :param request: Ignored. :return: HTTP 202 and message """ - self.gundam.all_on() + await self.show_manager.run_action("All LEDs on", self.gundam.all_on) return "All leds are on", 202 - def all_off(self, request: Request): + async def all_off(self, request: Request): """ - Turns off all LEDs. + Turns off all LEDs, halting any running lightshow first. :param request: Ignored. :return: HTTP 202 and message """ - self.gundam.all_off() + await self.show_manager.run_action("All LEDs off", self.gundam.all_off) return "All leds are off", 202 async def _connect_to_wifi(self): @@ -102,12 +101,6 @@ async def run(self): port=self.settings.get('port', 80), debug=self.settings.get('debug', True)) - def _is_lightshow_running(self): - """ - :return: True if lightshow is running, False otherwise - """ - return self.show_manager.is_running() - def _add_routes(self): """ Given a server adds all endpoints for Leds and lightshows @@ -119,12 +112,12 @@ def _add_routes(self): @self.app.route("/led//on") @safe_execution async def led_on_handler(request, led_name): - return self.gundam.led_on(led_name) + await self.show_manager.run_action(f"LED '{led_name}' on", lambda: self.gundam.led_on(led_name)) @self.app.route("/led//off") @safe_execution async def led_off_handler(request, led_name): - return self.gundam.led_off(led_name) + await self.show_manager.run_action(f"LED '{led_name}' off", lambda: self.gundam.led_off(led_name)) self.app.route("/all/on")(self.all_on) self.app.route("/all/off")(self.all_off) @@ -134,21 +127,32 @@ async def led_off_handler(request, led_name): path = f"/lightshow/{lightshow['path']}" method_func = getattr(self.gundam, lightshow['method']) - self.app.route(path)(create_show_handler(method_func, self.show_manager)) + self.app.route(path)(create_show_handler(lightshow['name'], method_func, self.show_manager)) @self.app.route("/lightshow/stop") @safe_execution async def stop_lightshow(request): """ Stops any currently running lightshow task on the gundam instance. + Stop always leaves all the LEDs off, running show or not. """ - stopped = await self.show_manager.stop() - self.gundam.all_off() + stopped_show = await self.show_manager.stop() + if stopped_show: + return {"status": "stopped", "message": f"Lightshow '{stopped_show}' terminated"}, 200 - if stopped: - return {"status": "stopped", "message": "Lightshow terminated"}, 200 + await self.show_manager.run_action("All LEDs off", self.gundam.all_off) return {"status": "idle", "message": "No active lightshow to stop"}, 200 + @self.app.route("/lightshow/status") + @safe_execution + async def lightshow_status(request): + """ + Reports the current lightshow status as JSON. + """ + status = self.show_manager.status() + status["message"] = self.show_manager.describe() + return status, 200 + # 404 Handler @self.app.errorhandler(404) async def not_found(request): diff --git a/src/templates/index.html b/src/templates/index.html index 9ff515f..1b93aa8 100644 --- a/src/templates/index.html +++ b/src/templates/index.html @@ -1,4 +1,4 @@ -{% args name_of_title, all_leds, lightshows, running_show %} +{% args name_of_title, all_leds, lightshows, status_message %} @@ -22,11 +22,7 @@

Control individual LEDs

Light Shows

-{% if running_show %} -

A lightshow is currently running.

-{% else %} -

No lightshow is running.

-{% endif %} +

{{ status_message }}

    {% for show in lightshows %}
  • diff --git a/tests/test_virtual_hardware.py b/tests/test_virtual_hardware.py index 417a652..5c440f1 100644 --- a/tests/test_virtual_hardware.py +++ b/tests/test_virtual_hardware.py @@ -8,7 +8,8 @@ from src.hardware.VirtualHardware import VirtualHardware from src.pi.disabled_LED import DisabledLED from src.pi.led_effect import LEDEffects -from src.server.RouteDecorator import LightshowManager, lightshow_route +from src.server.lightshow_manager import LightshowManager +from src.server.Wrappers import create_show_handler def test_get_hardware_selects_virtual_off_device(): @@ -61,29 +62,79 @@ def test_all_on_and_off_run_on_virtual_hardware(): class FakeGunpla: + def __init__(self): + self.all_off_calls = 0 + def all_off(self): - pass + self.all_off_calls += 1 -def test_lightshow_lifecycle_tracks_and_clears_task(): +def test_lightshow_lifecycle_tracks_status(): manager = LightshowManager(FakeGunpla()) async def scenario(): + assert manager.status()["state"] == "idle" started = asyncio.Event() async def show(): started.set() await asyncio.sleep(60) - handler = lightshow_route(manager)(show) - _, status = await handler(None) + handler = create_show_handler("Marathon", show, manager) + body, status = await handler(None) assert status == 202 + assert body["show"] == "Marathon" await started.wait() assert manager.is_running() + assert manager.status() == {"state": "running", "show": "Marathon", "error": None} + + assert await manager.stop() == "Marathon" + assert not manager.is_running() + assert manager.status()["state"] == "stopped" + assert await manager.stop() is None + + asyncio.run(scenario()) + + +def test_show_outcomes_are_recorded(): + manager = LightshowManager(FakeGunpla()) + + async def scenario(): + async def quick(): + pass + + await manager.start("Quick", quick) + while manager.is_running(): + await asyncio.sleep(0) + assert manager.status() == {"state": "completed", "show": "Quick", "error": None} - assert await manager.stop() is True + async def bad(): + raise RuntimeError("boom") + + await manager.start("Bad", bad) + while manager.is_running(): + await asyncio.sleep(0) + assert manager.status() == {"state": "errored", "show": "Bad", "error": "boom"} + + asyncio.run(scenario()) + + +def test_manual_action_cancels_show_and_records_finished(): + gunpla = FakeGunpla() + manager = LightshowManager(gunpla) + actions = [] + + async def scenario(): + async def show(): + await asyncio.sleep(60) + + await manager.start("Marathon", show) + await asyncio.sleep(0) + await manager.run_action("All LEDs on", lambda: actions.append("all_on")) + assert actions == ["all_on"] assert not manager.is_running() - assert await manager.stop() is False + assert manager.status() == {"state": "finished", "show": "All LEDs on", "error": None} + assert gunpla.all_off_calls == 1 # the cancelled show's LEDs were cleared first asyncio.run(scenario()) @@ -102,14 +153,16 @@ async def show(): return show async def scenario(): - await manager.start(make_show("first")) + await manager.start("first", make_show("first")) await asyncio.sleep(0) # Two replacement requests land while the first show is still being # cancelled — without serialization one of these orphaned a show. - await asyncio.gather(manager.start(make_show("second")), manager.start(make_show("third"))) + await asyncio.gather(manager.start("second", make_show("second")), + manager.start("third", make_show("third"))) await asyncio.sleep(0) assert running == ["third"] assert manager.is_running() + assert manager.status()["show"] == "third" await manager.stop() assert running == [] From c8e4ab3ffc6d2bf2755b91adcbbb90d3bdb9ca83 Mon Sep 17 00:00:00 2001 From: frozenwizard <172203+frozenwizard@users.noreply.github.com> Date: Sun, 2 Aug 2026 22:40:08 -0500 Subject: [PATCH 2/2] fixes --- src/gunpla/base_gundam.py | 6 +++ src/server/lightshow_manager.py | 27 +++++++++--- src/server/webserver.py | 8 +++- tests/test_virtual_hardware.py | 78 +++++++++++++++++++++++++++++++++ 4 files changed, 112 insertions(+), 7 deletions(-) diff --git a/src/gunpla/base_gundam.py b/src/gunpla/base_gundam.py index ffad81b..391cdd4 100644 --- a/src/gunpla/base_gundam.py +++ b/src/gunpla/base_gundam.py @@ -64,6 +64,12 @@ def all_off(self) -> None: for led in self.get_all_leds(): led.off() + def has_led(self, led_name: str) -> bool: + """ + :return: True if led_name is a configured LED, False otherwise + """ + return any(entry['name'] == led_name for entry in self.config['leds']) + def get_all_leds(self, ignore_list: list[str] = None) -> list[LED]: """ Returns all LEDs configured, enabled or disabled. But not the board_led diff --git a/src/server/lightshow_manager.py b/src/server/lightshow_manager.py index 076d4d3..e327775 100644 --- a/src/server/lightshow_manager.py +++ b/src/server/lightshow_manager.py @@ -68,22 +68,31 @@ async def start(self, name: str, func) -> None: async def stop(self): """ Cancels any running lightshow, waits for it to clean up and turns the - LEDs off. + LEDs off — whether or not a show was actually running. :return: the name of the cancelled show, or None if nothing was running """ async with self._lock: - return await self._cancel_current() + stopped_show = await self._cancel_current() + if stopped_show is None: + self.gunpla.all_off() + return stopped_show async def run_action(self, description: str, action) -> None: """ Runs a one-shot LED action (e.g. "All LEDs on"), cancelling any running - show first so the show can't fight the user over the LEDs. + show first so the show can't fight the user over the LEDs. If the + action itself fails, that's recorded as the current status instead of + leaving the (now-cancelled) show's stale status in place. :param description: what the action is, shown in the UI status :param action: a plain callable that manipulates the LEDs """ async with self._lock: await self._cancel_current() - action() + try: + action() + except Exception as e: + self._set_status(ERRORED, description, str(e)) + raise self._set_status(FINISHED, description) def _set_status(self, state: str, show: str, error: str = None) -> None: @@ -107,11 +116,18 @@ async def _cancel_current(self): """ Cancels the tracked show if one is still running, waits for its cleanup and turns all LEDs off. Callers must hold the lock. + + self._task is kept set until the cancelled task is fully done, so + is_running() keeps reporting True for the outgoing show throughout its + cleanup instead of flipping to False before the supervisor has updated + status() to match — the two would otherwise disagree for however long + the show's own cleanup takes to unwind. + :return: the name of the cancelled show, or None if nothing was running """ task = self._task - self._task = None if task is None or task.done(): + self._task = None return None name = self._show task.cancel() @@ -119,5 +135,6 @@ async def _cancel_current(self): await task # Wait for cleanup; the supervisor records the outcome except asyncio.CancelledError: pass + self._task = None self.gunpla.all_off() return name diff --git a/src/server/webserver.py b/src/server/webserver.py index 5b8ad91..419a73b 100644 --- a/src/server/webserver.py +++ b/src/server/webserver.py @@ -112,11 +112,17 @@ def _add_routes(self): @self.app.route("/led//on") @safe_execution async def led_on_handler(request, led_name): + # Validate before run_action, which cancels any running show — an + # unknown led_name shouldn't destroy a show just to then 500. + if not self.gundam.has_led(led_name): + raise Exception(f"Entry '{led_name}' not found") await self.show_manager.run_action(f"LED '{led_name}' on", lambda: self.gundam.led_on(led_name)) @self.app.route("/led//off") @safe_execution async def led_off_handler(request, led_name): + if not self.gundam.has_led(led_name): + raise Exception(f"Entry '{led_name}' not found") await self.show_manager.run_action(f"LED '{led_name}' off", lambda: self.gundam.led_off(led_name)) self.app.route("/all/on")(self.all_on) @@ -139,8 +145,6 @@ async def stop_lightshow(request): stopped_show = await self.show_manager.stop() if stopped_show: return {"status": "stopped", "message": f"Lightshow '{stopped_show}' terminated"}, 200 - - await self.show_manager.run_action("All LEDs off", self.gundam.all_off) return {"status": "idle", "message": "No active lightshow to stop"}, 200 @self.app.route("/lightshow/status") diff --git a/tests/test_virtual_hardware.py b/tests/test_virtual_hardware.py index 5c440f1..243451f 100644 --- a/tests/test_virtual_hardware.py +++ b/tests/test_virtual_hardware.py @@ -55,6 +55,12 @@ def test_gundam_caches_led_objects(): assert gundam._get_led_from_name("head") is gundam._get_led_from_name("head") +def test_has_led_reports_configured_leds(): + gundam = GenericGundam(VirtualHardware()) + assert gundam.has_led("head") + assert not gundam.has_led("nonexistent") + + def test_all_on_and_off_run_on_virtual_hardware(): gundam = GenericGundam(VirtualHardware()) gundam.all_on() @@ -139,6 +145,78 @@ async def show(): asyncio.run(scenario()) +def test_stop_turns_leds_off_even_when_nothing_was_running(): + gunpla = FakeGunpla() + manager = LightshowManager(gunpla) + + async def scenario(): + assert await manager.stop() is None + assert gunpla.all_off_calls == 1 + + asyncio.run(scenario()) + + +def test_run_action_records_error_status_and_reraises_instead_of_leaving_it_stale(): + gunpla = FakeGunpla() + manager = LightshowManager(gunpla) + + async def scenario(): + async def show(): + await asyncio.sleep(60) + + await manager.start("Marathon", show) + await asyncio.sleep(0) + + def bad_action(): + raise ValueError("nope") + + try: + await manager.run_action("Bad action", bad_action) + assert False, "expected ValueError to propagate" + except ValueError: + pass + + # The show was still cancelled (run_action always clears the way first), + # but status now reflects the failed action instead of being left at + # whatever the cancelled show's supervisor last recorded. + assert not manager.is_running() + assert manager.status() == {"state": "errored", "show": "Bad action", "error": "nope"} + assert gunpla.all_off_calls == 1 + + asyncio.run(scenario()) + + +def test_is_running_stays_consistent_with_status_during_cancellation(): + manager = LightshowManager(FakeGunpla()) + observed = [] + + async def scenario(): + async def slow_cleanup_show(): + try: + await asyncio.sleep(60) + finally: + # cleanup needs an extra tick before the task is actually done + await asyncio.sleep(0) + + await manager.start("Slow", slow_cleanup_show) + await asyncio.sleep(0) + assert manager.is_running() + + async def watcher(): + for _ in range(5): + observed.append((manager.is_running(), manager.status()["state"])) + await asyncio.sleep(0) + + await asyncio.gather(manager.stop(), watcher()) + + for is_running, state in observed: + assert not (is_running is False and state == "running"), ( + f"is_running() reported False while status still said running: {observed}" + ) + + asyncio.run(scenario()) + + def test_overlapping_start_requests_do_not_orphan_a_show(): manager = LightshowManager(FakeGunpla()) running = []