From df82e512e96b114ab70a3dec30b140bd7bd61930 Mon Sep 17 00:00:00 2001 From: John Osumi <931193+sumitake@users.noreply.github.com> Date: Tue, 4 Aug 2026 00:39:42 -0700 Subject: [PATCH] fix: BusyBarClient.set_busy_simple sent a body the device rejects The device's /openapi.yaml documents PUT /api/busy/snapshot's BusySnapshot body as the discriminated snapshot variant merged (via allOf) with a required busy_bar_settings, sent flat -- the shape set_busy_simple sent. Against a live device this flat body gets HTTP 400 "Failed to parse snapshot" on every call, silently breaking calendar_countdown's auto_busy=true feature (it never surfaces the failure -- set_busy_simple's bool return is not checked by the caller). Empirically the firmware instead wants the snapshot variant nested under a "snapshot" key, sibling to a top-level "snapshot_timestamp_ms" -- mirroring get_busy()'s own GET response shape -- and does NOT want busy_bar_settings on this write path at all. Also found on-device: snapshot_timestamp_ms must be a genuinely current timestamp; a stale/placeholder value still returns 200 but silently no-ops the write, so set_busy_simple now sends time.time()-derived "now" rather than a fixed value. Verified on-device: the corrected body returns 200 and a subsequent GET /api/busy/snapshot shows a real active SIMPLE session; ran calendar_countdown.main.run_once with auto_busy=true and a synthetic in-progress event against the real device (draw/clear no-op'd to avoid touching the display) and confirmed the session actually starts. Device left in NOT_STARTED afterward. Public signature of set_busy_simple is unchanged. Co-Authored-By: Claude Fable 5 --- src/busybar/client.py | 33 +++++++++++++++++++++++++++++++-- tests/test_client.py | 35 ++++++++++++++++++++++++++++++----- 2 files changed, 61 insertions(+), 7 deletions(-) diff --git a/src/busybar/client.py b/src/busybar/client.py index a2dd508..cf61ca0 100644 --- a/src/busybar/client.py +++ b/src/busybar/client.py @@ -1,4 +1,5 @@ import logging +import time from enum import Enum import requests @@ -57,7 +58,35 @@ def get_busy(self) -> dict | None: return resp.json() if resp is not None and resp.status_code == 200 else None def set_busy_simple(self, time_left_ms: int) -> bool: - body = {"type": "SIMPLE", "card_id": NULL_CARD_ID, - "time_left_ms": time_left_ms, "is_paused": False} + """PUT /api/busy/snapshot to start a SIMPLE BUSY session (used by + calendar_countdown's auto_busy=true feature). + + The device's /openapi.yaml documents BusySnapshot as the + discriminated snapshot variant merged (via allOf) with a required + top-level `busy_bar_settings`, sent flat -- that is the shape this + method sent before this fix. Empirically, against a live device, + that flat body gets HTTP 400 "Failed to parse snapshot" every + time. The shape the firmware actually accepts mirrors what + get_busy() (GET, unaffected by this bug) returns: the snapshot + variant nested under a "snapshot" key, sibling to a top-level + "snapshot_timestamp_ms" -- and, on this write path, WITHOUT + `busy_bar_settings` at all, despite the spec marking it required. + Confirmed on-device: the nested body with no `busy_bar_settings` + returns 200 and the session actually starts (visible in a + subsequent get_busy() snapshot). + + `snapshot_timestamp_ms` must be a genuinely current timestamp, not + a stale or placeholder value -- also confirmed on-device: PUTting + this same nested body with a stale `snapshot_timestamp_ms` (e.g. + one copied from a prior GET) still returns HTTP 200, but the + write silently no-ops and the busy state does not actually + change. Always send `time.time()`-derived "now", never a fixed + or cached value. + """ + body = { + "snapshot": {"type": "SIMPLE", "card_id": NULL_CARD_ID, + "time_left_ms": time_left_ms, "is_paused": False}, + "snapshot_timestamp_ms": int(time.time() * 1000), + } resp = self._request("PUT", "/api/busy/snapshot", json=body) return resp is not None and resp.status_code == 200 diff --git a/tests/test_client.py b/tests/test_client.py index 85147c2..9b245b5 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -50,17 +50,42 @@ def test_status_none_when_unreachable(mock_request): assert BusyBarClient().status() is None +@patch("busybar.client.time.time") @patch("busybar.client.requests.request") -def test_set_busy_simple_payload(mock_request): +def test_set_busy_simple_payload(mock_request, mock_time): + # Regression test for the confirmed-on-device bug: the device rejects a + # flat BusySnapshot body (HTTP 400 "Failed to parse snapshot") even + # though that's the shape /openapi.yaml's schema literally describes. + # The firmware actually requires the snapshot nested under a + # "snapshot" key alongside "snapshot_timestamp_ms" -- mirroring what + # get_busy() (GET) returns -- and does NOT want busy_bar_settings on + # this write path. mock_request.return_value = _response(200) + mock_time.return_value = 1_700_000_000.5 assert BusyBarClient().set_busy_simple(90_000) is True + method, url = mock_request.call_args.args + assert method == "PUT" and url == "http://10.0.4.20/api/busy/snapshot" body = mock_request.call_args.kwargs["json"] assert body == { - "type": "SIMPLE", - "card_id": "00000000-0000-0000-0000-000000000000", - "time_left_ms": 90_000, - "is_paused": False, + "snapshot": { + "type": "SIMPLE", + "card_id": "00000000-0000-0000-0000-000000000000", + "time_left_ms": 90_000, + "is_paused": False, + }, + "snapshot_timestamp_ms": 1_700_000_000_500, } + assert "busy_bar_settings" not in body + assert "busy_bar_settings" not in body["snapshot"] + + +@patch("busybar.client.requests.request") +def test_set_busy_simple_false_on_400(mock_request): + # The pre-fix flat body reproduced a live 400 "Failed to parse + # snapshot" on every call -- guard against regressing to that shape by + # asserting the method's own failure handling is intact. + mock_request.return_value = _response(400) + assert BusyBarClient().set_busy_simple(90_000) is False @patch("busybar.client.requests.request")