diff --git a/NIGHTLY_CHANGELOG.md b/NIGHTLY_CHANGELOG.md index 2916255..290cdb1 100644 --- a/NIGHTLY_CHANGELOG.md +++ b/NIGHTLY_CHANGELOG.md @@ -4,3 +4,4 @@ - Feat: the GCode browser now supports multi-select and bulk delete — click any file's checkbox to enter select mode, use "Select All" to grab everything currently visible (respecting your active search/filter), then delete the selection with one confirmation instead of one-by-one (Issue #94, thanks @Blaim) - Fix: **printing via OrcaSlicer "Upload and print" failed (or fed the wrong spool) when a slot below the used filament was empty** — e.g. printing with Filament 4 while slot 3 was empty. The generated AMS slot mapping inserted a placeholder pointing at the physically empty tray, which the printer rejects. Placeholders now point at a loaded tray instead. Printing with all slots full already worked and is unchanged. - Fix: manually assigning a filament profile to an ACE slot could crash the MQTT callback (`'list' object has no attribute 'get'`) when the printer rejected the assignment, e.g. for custom-RFID/third-party filament. The bridge now handles the printer's failure response cleanly instead of crashing, and the log now correlates a rejected assignment with the request that triggered it (slot/type/color) so a rejection reason can be diagnosed from bridge logs alone (Issue #100, thanks @Blaim) +- Fix: **the camera stream could hang forever after a printer reboot** — the printer rotates its stream token on reboot, but the running ffmpeg processes kept reading from the stale, now-silent connection and never noticed. The bridge now detects the URL change and automatically restarts the camera pipeline, ffmpeg itself now times out on a stalled read as a second line of defense, and `/api/camera/stream` now returns a proper 503 instead of hanging if no frame arrives within 5s (Issue #99, thanks @fmontagna for the excellent root-cause analysis and reproduction) diff --git a/kobrax_moonraker_bridge.py b/kobrax_moonraker_bridge.py index 158ab6c..cdc56d2 100644 --- a/kobrax_moonraker_bridge.py +++ b/kobrax_moonraker_bridge.py @@ -631,7 +631,16 @@ class CameraCache: self._fail_count_mjpeg: int = 0 def set_url(self, url: str): + # A changed URL means the printer rotated its stream token (typically + # after a reboot). Running ffmpeg processes still hold the stale URL + # and will never pick it up on their own - they only re-read self._url + # at the top of their outer loop, which they never reach while blocked + # in a stdout read on the old, now-silent connection. Tear them down; + # the next ensure_running() respawns them against the new URL. + changed = bool(url and self._url and url != self._url) self._url = url + if changed: + self.reset() def reset(self): """Reset backoff counters and forcefully tear down any running @@ -679,7 +688,12 @@ class CameraCache: self._task_mjpeg = asyncio.create_task(self._run_mjpeg_loop()) def _input_args(self, url: str) -> list[str]: - args = ["-fflags", "nobuffer", "-flags", "low_delay"] + args = ["-fflags", "nobuffer", "-flags", "low_delay", + # Bail out if the source goes silent. A printer reboot or + # network loss leaves the TCP connection ESTABLISHED with no + # data and no FIN, so a passive stdout read blocks forever + # without this (Issue #99). Value is microseconds. + "-timeout", "10000000"] if url.lower().startswith("rtsp://"): args += ["-probesize", "32", "-analyzeduration", "0", "-rtsp_transport", "tcp"] else: @@ -4368,6 +4382,16 @@ class KobraXBridge: q: asyncio.Queue[bytes] = asyncio.Queue(maxsize=8) self.camera_cache.mjpeg_subscribers.add(q) + # Wait for the first frame BEFORE resp.prepare() - once prepare() sends + # the response headers the status is committed to 200, so a stalled + # source (Issue #99) must be caught here to actually return a 503 + # instead of hanging the client forever with no frame ever arriving. + try: + first_frame = await asyncio.wait_for(q.get(), timeout=5.0) + except asyncio.TimeoutError: + self.camera_cache.mjpeg_subscribers.discard(q) + return web.Response(status=503, text="No frame in cache yet") + boundary = "kobraxframe" resp = web.StreamResponse(headers={ "Content-Type": f"multipart/x-mixed-replace;boundary={boundary}", @@ -4376,8 +4400,8 @@ class KobraXBridge: }) await resp.prepare(request) try: + frame = first_frame while True: - frame = await q.get() header = ( f"--{boundary}\r\n" f"Content-Type: image/jpeg\r\n" @@ -4389,6 +4413,7 @@ class KobraXBridge: break except Exception: break + frame = await q.get() except Exception as e: log.warning(f"Camera stream interrupted: {e}") finally: diff --git a/tests/test_camera_cache_url_rotation.py b/tests/test_camera_cache_url_rotation.py new file mode 100644 index 0000000..8cb4caf --- /dev/null +++ b/tests/test_camera_cache_url_rotation.py @@ -0,0 +1,51 @@ +"""Camera stream hangs forever after printer reboot (Issue #99). + +The printer rotates its stream token on reboot, changing the camera URL. +CameraCache.set_url() used to be a bare assignment - the running ffmpeg loops +never noticed since they only re-read self._url at the top of their outer +loop, which they never reach while permanently blocked reading stdout from +the now-silent, stale-token connection. set_url() must detect the change and +tear the loops down so the next ensure_running() respawns them against the +new URL. +""" +from kobrax_moonraker_bridge import CameraCache + + +def test_set_url_first_time_does_not_reset(): + """No prior URL - nothing stale to tear down.""" + c = CameraCache() + calls = [] + c.reset = lambda: calls.append(True) + c.set_url("http://printer/live/tokenA") + assert c._url == "http://printer/live/tokenA" + assert not calls + + +def test_set_url_same_value_does_not_reset(): + c = CameraCache() + calls = [] + c.set_url("http://printer/live/tokenA") + c.reset = lambda: calls.append(True) + c.set_url("http://printer/live/tokenA") + assert not calls + + +def test_set_url_changed_triggers_reset(): + """The actual bug scenario: token rotates after a printer reboot.""" + c = CameraCache() + calls = [] + c.set_url("http://printer/live/tokenA") + c.reset = lambda: calls.append(True) + c.set_url("http://printer/live/tokenB") + assert c._url == "http://printer/live/tokenB" + assert calls == [True] + + +def test_set_url_empty_to_value_does_not_reset(): + """Startup case: no URL known yet, first status push sets it - nothing + running to tear down.""" + c = CameraCache() + calls = [] + c.reset = lambda: calls.append(True) + c.set_url("http://printer/live/tokenA") + assert not calls