From 18c36a0e931178dc49db495c96438dcaf6c12fc7 Mon Sep 17 00:00:00 2001 From: joseg20 Date: Mon, 3 Aug 2026 17:36:53 -0400 Subject: [PATCH 1/9] feat(reolink): detect a motorised lens instead of assuming static cameras have none cam_type describes how a camera is mounted, not what optics it carries. A "static" camera is one that does not pan or tilt, which says nothing about zoom: Reolink bullets such as the RLC-811A or the P430 sit fixed on their mast and still ship a motorised varifocal lens. Ask the camera instead of inferring. GetZoomFocus reports a zoom position only on models that can drive the lens, so the answer comes from the device and holds for any model. The probe is cached, since a lens cannot grow a motor at runtime and zoom commands are frequent. A camera that cannot be reached is treated as fixed-lens rather than raising, so a network blip degrades the feature instead of failing the call. Fixed-lens cameras keep their current behaviour: they report no zoom position, so the commands stay unsent exactly as before. PTZ cameras are unaffected. --- .../camera/adapters/reolink.py | 29 ++++++- pyro_camera_api/tests/test_reolink_lens.py | 75 +++++++++++++++++++ 2 files changed, 101 insertions(+), 3 deletions(-) create mode 100644 pyro_camera_api/tests/test_reolink_lens.py diff --git a/pyro_camera_api/pyro_camera_api/camera/adapters/reolink.py b/pyro_camera_api/pyro_camera_api/camera/adapters/reolink.py index edb2ff59..8838169d 100644 --- a/pyro_camera_api/pyro_camera_api/camera/adapters/reolink.py +++ b/pyro_camera_api/pyro_camera_api/camera/adapters/reolink.py @@ -96,6 +96,29 @@ def __init__( len(self.cam_poses), len(self.cam_azimuths), ) + self._has_motorised_lens: Optional[bool] = None + + def has_motorised_lens(self) -> bool: + """Whether this camera's lens can be driven. + + ``cam_type`` describes how the camera is mounted, not what optics it + carries. A "static" camera is one that does not pan or tilt, which says + nothing about zoom: Reolink bullets such as the RLC-811A or the P430 sit + fixed on their mast and still ship a motorised varifocal lens. + + Rather than infer it, ask the camera. ``GetZoomFocus`` reports a zoom + position only on models that can move the lens, so the answer comes from + the device itself and holds for any model. The result is cached: it + cannot change while the camera is running, and this saves a request on + every zoom or focus command. + """ + if self._has_motorised_lens is None: + try: + self._has_motorised_lens = (self.get_focus_level() or {}).get("zoom") is not None + except Exception as exc: + logger.warning("[%s] could not probe lens capability: %s", self.ip_address, exc) + return False + return self._has_motorised_lens def _build_url(self, command: str) -> str: """Constructs a URL for API commands to the camera.""" @@ -257,7 +280,7 @@ def set_auto_focus(self, disable: bool): return self._handle_response(response, "Set AutoFocus settings successfully.") def start_zoom_focus(self, position: int): - if self.cam_type != "static": + if self.has_motorised_lens(): url = self._build_url("StartZoomFocus") data: Any = [ { @@ -274,7 +297,7 @@ def set_manual_focus(self, position: int): """ Set manual focus to a specific position. """ - if self.cam_type != "static": + if self.has_motorised_lens(): self.focus_position = position url = self._build_url("StartZoomFocus") data: Any = [ @@ -346,7 +369,7 @@ def capture_and_score(pos: int) -> float: image.save(f"{folder}/focus_{pos}.jpg") return score_local - if self.cam_type == "static": + if not self.has_motorised_lens(): return 720 # set_manual_focus records every probed position in focus_position, so diff --git a/pyro_camera_api/tests/test_reolink_lens.py b/pyro_camera_api/tests/test_reolink_lens.py new file mode 100644 index 00000000..1ed162e6 --- /dev/null +++ b/pyro_camera_api/tests/test_reolink_lens.py @@ -0,0 +1,75 @@ +# Copyright (C) 2022-2026, Pyronear. + +# This program is licensed under the Apache License 2.0. +# See LICENSE or go to for full license details. + + +from unittest.mock import patch + +from pyro_camera_api.camera.adapters.reolink import ReolinkCamera + + +def _camera(cam_type="static"): + return ReolinkCamera( + camera_id="cam", + ip_address="192.168.1.10", + username="user", + password="pwd", # ruff: ignore[hardcoded-password-func-arg] + cam_type=cam_type, + ) + + +def test_static_camera_with_a_varifocal_lens_can_zoom(): + """A bullet camera does not pan, which says nothing about its optics.""" + cam = _camera(cam_type="static") + with patch.object(ReolinkCamera, "get_focus_level", return_value={"focus": 336, "zoom": 0}): + assert cam.has_motorised_lens() is True + + +def test_fixed_lens_camera_reports_no_zoom(): + cam = _camera(cam_type="static") + with patch.object(ReolinkCamera, "get_focus_level", return_value={"focus": 336, "zoom": None}): + assert cam.has_motorised_lens() is False + + +def test_unreachable_camera_is_treated_as_fixed_lens(): + """Probing must not raise: a camera that cannot be asked keeps its commands + from being sent rather than taking the whole call down.""" + cam = _camera() + with patch.object(ReolinkCamera, "get_focus_level", side_effect=OSError("unreachable")): + assert cam.has_motorised_lens() is False + + +def test_capability_is_probed_once(): + """Zoom and focus commands are frequent; the lens cannot grow a motor.""" + cam = _camera() + with patch.object(ReolinkCamera, "get_focus_level", return_value={"focus": 1, "zoom": 4}) as probe: + cam.has_motorised_lens() + cam.has_motorised_lens() + assert probe.call_count == 1 + + +def test_zoom_command_is_sent_to_a_static_varifocal_camera(): + """The regression this change is about: the command used to be dropped for + every static camera, silently returning None with no request made.""" + cam = _camera(cam_type="static") + with ( + patch.object(ReolinkCamera, "get_focus_level", return_value={"focus": 1, "zoom": 0}), + patch("pyro_camera_api.camera.adapters.reolink.requests.post") as post, + patch.object(ReolinkCamera, "_handle_response", return_value="ok"), + ): + assert cam.start_zoom_focus(32) == "ok" + assert post.call_count == 1 + payload = post.call_args.kwargs["json"][0] + assert payload["param"]["ZoomFocus"]["pos"] == 32 + assert payload["param"]["ZoomFocus"]["op"] == "ZoomPos" + + +def test_zoom_command_is_not_sent_to_a_fixed_lens_camera(): + cam = _camera(cam_type="static") + with ( + patch.object(ReolinkCamera, "get_focus_level", return_value={"focus": 1, "zoom": None}), + patch("pyro_camera_api.camera.adapters.reolink.requests.post") as post, + ): + assert cam.start_zoom_focus(32) is None + assert post.call_count == 0 From b8e0fcd6ad948ee919119e78d63921db1f84668c Mon Sep 17 00:00:00 2001 From: joseg20 Date: Mon, 3 Aug 2026 17:44:52 -0400 Subject: [PATCH 2/9] =?UTF-8?q?fix(reolink):=20address=20review=20?= =?UTF-8?q?=E2=80=94=20inconclusive=20probes=20and=20the=20autofocus=20rou?= =?UTF-8?q?te?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two problems raised in review, both real. A failed GetZoomFocus request was being cached as "no motorised lens". One transient error would have stranded a PTZ camera without zoom or manual focus for the rest of the process, long after the camera recovered. An inconclusive probe now skips the command without caching, so the next call tries again. The autofocus route rejected static cameras before ever reaching the adapter, so the capability check could not take effect through the camera service. It now asks the camera for its lens the same way, and falls back to allowing the call on adapters that cannot answer. --- pyro_camera_api/pyro_camera_api/api/routes_focus.py | 9 ++++++--- .../pyro_camera_api/camera/adapters/reolink.py | 10 +++++++++- pyro_camera_api/tests/test_reolink_lens.py | 12 +++++++++++- 3 files changed, 26 insertions(+), 5 deletions(-) diff --git a/pyro_camera_api/pyro_camera_api/api/routes_focus.py b/pyro_camera_api/pyro_camera_api/api/routes_focus.py index 488b89ea..f4542d69 100644 --- a/pyro_camera_api/pyro_camera_api/api/routes_focus.py +++ b/pyro_camera_api/pyro_camera_api/api/routes_focus.py @@ -104,7 +104,7 @@ def run_focus_optimization(camera_ip: str, save_images: bool = False): """ Run the autofocus search algorithm and return the optimal focus position. - This operation is supported only on PTZ cameras implementing FocusMixin. + Supported on any camera with a motorised lens, whether or not it pans. If the camera exposes PTZ presets the algorithm tries moving to the second preset before the optimization step when available. The optional `save_images` parameter allows storing captured frames generated @@ -124,8 +124,11 @@ def run_focus_optimization(camera_ip: str, save_images: bool = False): if not isinstance(cam, FocusMixin): raise HTTPException(status_code=400, detail="Camera does not support autofocus") - if getattr(cam, "cam_type", "static") == "static": - raise HTTPException(status_code=400, detail="Autofocus is not supported for static cameras") + # Mounting type says nothing about the optics: a bullet camera that does not + # pan can still carry a motorised lens. Ask the camera when it can tell us. + has_motorised_lens = getattr(cam, "has_motorised_lens", None) + if has_motorised_lens is not None and not has_motorised_lens(): + raise HTTPException(status_code=400, detail="Camera does not have a motorised lens") if stream_is_active(camera_ip): raise HTTPException(status_code=409, detail="Stream active, focus optimization refused") diff --git a/pyro_camera_api/pyro_camera_api/camera/adapters/reolink.py b/pyro_camera_api/pyro_camera_api/camera/adapters/reolink.py index 8838169d..5ee10b6b 100644 --- a/pyro_camera_api/pyro_camera_api/camera/adapters/reolink.py +++ b/pyro_camera_api/pyro_camera_api/camera/adapters/reolink.py @@ -114,10 +114,18 @@ def has_motorised_lens(self) -> bool: """ if self._has_motorised_lens is None: try: - self._has_motorised_lens = (self.get_focus_level() or {}).get("zoom") is not None + lens = self.get_focus_level() except Exception as exc: logger.warning("[%s] could not probe lens capability: %s", self.ip_address, exc) return False + if lens is None: + # The camera did not answer, which says nothing about its optics. + # Caching this would strand a PTZ camera as fixed-lens for the + # rest of the process over one failed request, so try again next + # time and skip the command for now. + logger.warning("[%s] lens capability probe was inconclusive", self.ip_address) + return False + self._has_motorised_lens = lens.get("zoom") is not None return self._has_motorised_lens def _build_url(self, command: str) -> str: diff --git a/pyro_camera_api/tests/test_reolink_lens.py b/pyro_camera_api/tests/test_reolink_lens.py index 1ed162e6..e132808f 100644 --- a/pyro_camera_api/tests/test_reolink_lens.py +++ b/pyro_camera_api/tests/test_reolink_lens.py @@ -14,7 +14,7 @@ def _camera(cam_type="static"): camera_id="cam", ip_address="192.168.1.10", username="user", - password="pwd", # ruff: ignore[hardcoded-password-func-arg] + password="pwd", # noqa: S106 cam_type=cam_type, ) @@ -40,6 +40,16 @@ def test_unreachable_camera_is_treated_as_fixed_lens(): assert cam.has_motorised_lens() is False +def test_an_inconclusive_probe_is_not_cached(): + """A failed request says nothing about the optics. Caching it would strand a + PTZ camera as fixed-lens for the rest of the process over one bad answer.""" + cam = _camera(cam_type="ptz") + with patch.object(ReolinkCamera, "get_focus_level", return_value=None): + assert cam.has_motorised_lens() is False + with patch.object(ReolinkCamera, "get_focus_level", return_value={"focus": 1, "zoom": 0}): + assert cam.has_motorised_lens() is True + + def test_capability_is_probed_once(): """Zoom and focus commands are frequent; the lens cannot grow a motor.""" cam = _camera() From 2dd6a49963f66e7413487200037b43856b8a0584 Mon Sep 17 00:00:00 2001 From: joseg20 Date: Wed, 5 Aug 2026 13:18:39 -0400 Subject: [PATCH 3/9] =?UTF-8?q?fix(reolink):=20address=20review=20?= =?UTF-8?q?=E2=80=94=20settle=20rejections,=20scope=20out=20the=20focus=20?= =?UTF-8?q?routes?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three points from review. The probe conflated a camera that could not be reached with one that answered by rejecting the command: both returned None from get_focus_level() and neither was cached. A fixed-lens model that rejects GetZoomFocus outright would have paid an HTTP request and a warning on every zoom and focus call for the life of the process. The probe now reads the reply itself: a well-formed non-zero code is the camera settling the question and is cached, while transport failures and HTTP errors stay uncached and are retried. The capability guard moves from the autofocus route to the zoom route. Lifting it on /focus/focus_finder would let the search run on a static varifocal camera whose focus_position is unset, and that path opens with start_zoom_focus(0) — wiping zoom levels set by hand in the field, which is exactly the state this change exists to respect. routes_focus.py is left untouched, both for that reason and because #391 rewrites it. /control/zoom keeps the guard: hasattr only proves the adapter has the method, so without it the route answered 200 for a command the adapter dropped, which is the silent failure this PR set out to remove. --- .../pyro_camera_api/api/routes_control.py | 8 +++ .../pyro_camera_api/api/routes_focus.py | 9 +-- .../camera/adapters/reolink.py | 52 ++++++++------ pyro_camera_api/tests/test_reolink_lens.py | 67 +++++++++++++------ 4 files changed, 91 insertions(+), 45 deletions(-) diff --git a/pyro_camera_api/pyro_camera_api/api/routes_control.py b/pyro_camera_api/pyro_camera_api/api/routes_control.py index f6feb295..92322681 100644 --- a/pyro_camera_api/pyro_camera_api/api/routes_control.py +++ b/pyro_camera_api/pyro_camera_api/api/routes_control.py @@ -1084,6 +1084,14 @@ def zoom_camera(camera_ip: str, level: int): if not hasattr(cam, "start_zoom_focus"): raise HTTPException(status_code=400, detail="Camera does not support zoom control") + # hasattr only proves the adapter has the method, not that this camera can + # act on it: the Reolink adapter drops the command for a fixed lens and + # returns None, so without this the route would answer 200 for a zoom that + # never happened. + has_motorised_lens = getattr(cam, "has_motorised_lens", None) + if has_motorised_lens is not None and not has_motorised_lens(): + raise HTTPException(status_code=400, detail="Camera does not have a motorised lens") + lock = _acquire_or_409(camera_ip) try: diff --git a/pyro_camera_api/pyro_camera_api/api/routes_focus.py b/pyro_camera_api/pyro_camera_api/api/routes_focus.py index f4542d69..488b89ea 100644 --- a/pyro_camera_api/pyro_camera_api/api/routes_focus.py +++ b/pyro_camera_api/pyro_camera_api/api/routes_focus.py @@ -104,7 +104,7 @@ def run_focus_optimization(camera_ip: str, save_images: bool = False): """ Run the autofocus search algorithm and return the optimal focus position. - Supported on any camera with a motorised lens, whether or not it pans. + This operation is supported only on PTZ cameras implementing FocusMixin. If the camera exposes PTZ presets the algorithm tries moving to the second preset before the optimization step when available. The optional `save_images` parameter allows storing captured frames generated @@ -124,11 +124,8 @@ def run_focus_optimization(camera_ip: str, save_images: bool = False): if not isinstance(cam, FocusMixin): raise HTTPException(status_code=400, detail="Camera does not support autofocus") - # Mounting type says nothing about the optics: a bullet camera that does not - # pan can still carry a motorised lens. Ask the camera when it can tell us. - has_motorised_lens = getattr(cam, "has_motorised_lens", None) - if has_motorised_lens is not None and not has_motorised_lens(): - raise HTTPException(status_code=400, detail="Camera does not have a motorised lens") + if getattr(cam, "cam_type", "static") == "static": + raise HTTPException(status_code=400, detail="Autofocus is not supported for static cameras") if stream_is_active(camera_ip): raise HTTPException(status_code=409, detail="Stream active, focus optimization refused") diff --git a/pyro_camera_api/pyro_camera_api/camera/adapters/reolink.py b/pyro_camera_api/pyro_camera_api/camera/adapters/reolink.py index 5ee10b6b..85f16233 100644 --- a/pyro_camera_api/pyro_camera_api/camera/adapters/reolink.py +++ b/pyro_camera_api/pyro_camera_api/camera/adapters/reolink.py @@ -98,6 +98,32 @@ def __init__( ) self._has_motorised_lens: Optional[bool] = None + def _probe_zoom_support(self) -> Optional[bool]: + """Ask the camera whether it reports a zoom position. + + Returns None only when the camera could not be asked, so the caller can + tell a transport failure apart from a device that answered. A camera + replying with a non-zero Reolink code has answered: it does not serve + GetZoomFocus, which settles the question. + """ + try: + response = requests.post( + self._build_url("GetZoomFocus"), + json=[{"cmd": "GetZoomFocus", "action": 0, "param": {"channel": 0}}], + verify=False, # nosec: B501 + ) + except Exception as exc: + logger.warning("[%s] lens probe could not reach the camera: %s", self.ip_address, exc) + return None + if response.status_code != 200: + logger.warning("[%s] lens probe got HTTP %s", self.ip_address, response.status_code) + return None + payload = response.json() + if payload[0].get("code") != 0: + logger.info("[%s] camera does not serve GetZoomFocus: fixed lens", self.ip_address) + return False + return payload[0]["value"]["ZoomFocus"].get("zoom", {}).get("pos") is not None + def has_motorised_lens(self) -> bool: """Whether this camera's lens can be driven. @@ -106,27 +132,15 @@ def has_motorised_lens(self) -> bool: nothing about zoom: Reolink bullets such as the RLC-811A or the P430 sit fixed on their mast and still ship a motorised varifocal lens. - Rather than infer it, ask the camera. ``GetZoomFocus`` reports a zoom - position only on models that can move the lens, so the answer comes from - the device itself and holds for any model. The result is cached: it - cannot change while the camera is running, and this saves a request on - every zoom or focus command. + The answer is cached once the camera gives one, since a lens cannot grow + a motor at runtime and zoom commands are frequent. Only an unanswered + probe is retried: caching that would strand a camera over one bad + request, and re-probing a camera that already said no would cost a + request on every command for the rest of the process. """ if self._has_motorised_lens is None: - try: - lens = self.get_focus_level() - except Exception as exc: - logger.warning("[%s] could not probe lens capability: %s", self.ip_address, exc) - return False - if lens is None: - # The camera did not answer, which says nothing about its optics. - # Caching this would strand a PTZ camera as fixed-lens for the - # rest of the process over one failed request, so try again next - # time and skip the command for now. - logger.warning("[%s] lens capability probe was inconclusive", self.ip_address) - return False - self._has_motorised_lens = lens.get("zoom") is not None - return self._has_motorised_lens + self._has_motorised_lens = self._probe_zoom_support() + return self._has_motorised_lens is True def _build_url(self, command: str) -> str: """Constructs a URL for API commands to the camera.""" diff --git a/pyro_camera_api/tests/test_reolink_lens.py b/pyro_camera_api/tests/test_reolink_lens.py index e132808f..26da6ab2 100644 --- a/pyro_camera_api/tests/test_reolink_lens.py +++ b/pyro_camera_api/tests/test_reolink_lens.py @@ -4,10 +4,21 @@ # See LICENSE or go to for full license details. -from unittest.mock import patch +from unittest.mock import MagicMock, patch from pyro_camera_api.camera.adapters.reolink import ReolinkCamera +POST = "pyro_camera_api.camera.adapters.reolink.requests.post" + + +def _reply(status=200, code=0, zoom_pos=0): + """A GetZoomFocus response as the camera would send it.""" + resp = MagicMock() + resp.status_code = status + zoom = {} if zoom_pos is None else {"pos": zoom_pos} + resp.json.return_value = [{"code": code, "value": {"ZoomFocus": {"focus": {"pos": 336}, "zoom": zoom}}}] + return resp + def _camera(cam_type="static"): return ReolinkCamera( @@ -22,41 +33,59 @@ def _camera(cam_type="static"): def test_static_camera_with_a_varifocal_lens_can_zoom(): """A bullet camera does not pan, which says nothing about its optics.""" cam = _camera(cam_type="static") - with patch.object(ReolinkCamera, "get_focus_level", return_value={"focus": 336, "zoom": 0}): + with patch(POST, return_value=_reply(zoom_pos=0)): assert cam.has_motorised_lens() is True -def test_fixed_lens_camera_reports_no_zoom(): +def test_fixed_lens_camera_reports_no_zoom_position(): + cam = _camera(cam_type="static") + with patch(POST, return_value=_reply(zoom_pos=None)): + assert cam.has_motorised_lens() is False + + +def test_a_camera_that_rejects_the_command_is_a_settled_answer(): + """A non-zero Reolink code is the camera answering that it does not serve + GetZoomFocus. Re-probing it would cost a request on every command forever.""" cam = _camera(cam_type="static") - with patch.object(ReolinkCamera, "get_focus_level", return_value={"focus": 336, "zoom": None}): + with patch(POST, return_value=_reply(code=-9)) as post: + assert cam.has_motorised_lens() is False assert cam.has_motorised_lens() is False + assert post.call_count == 1 def test_unreachable_camera_is_treated_as_fixed_lens(): """Probing must not raise: a camera that cannot be asked keeps its commands from being sent rather than taking the whole call down.""" cam = _camera() - with patch.object(ReolinkCamera, "get_focus_level", side_effect=OSError("unreachable")): + with patch(POST, side_effect=OSError("unreachable")): assert cam.has_motorised_lens() is False -def test_an_inconclusive_probe_is_not_cached(): - """A failed request says nothing about the optics. Caching it would strand a - PTZ camera as fixed-lens for the rest of the process over one bad answer.""" +def test_an_unanswered_probe_is_not_cached(): + """A transport failure says nothing about the optics. Caching it would + strand a PTZ camera as fixed-lens for the rest of the process.""" cam = _camera(cam_type="ptz") - with patch.object(ReolinkCamera, "get_focus_level", return_value=None): + with patch(POST, side_effect=OSError("unreachable")): assert cam.has_motorised_lens() is False - with patch.object(ReolinkCamera, "get_focus_level", return_value={"focus": 1, "zoom": 0}): + with patch(POST, return_value=_reply(zoom_pos=0)): + assert cam.has_motorised_lens() is True + + +def test_an_http_error_is_not_cached_either(): + cam = _camera(cam_type="ptz") + with patch(POST, return_value=_reply(status=500)): + assert cam.has_motorised_lens() is False + with patch(POST, return_value=_reply(zoom_pos=4)): assert cam.has_motorised_lens() is True def test_capability_is_probed_once(): """Zoom and focus commands are frequent; the lens cannot grow a motor.""" cam = _camera() - with patch.object(ReolinkCamera, "get_focus_level", return_value={"focus": 1, "zoom": 4}) as probe: + with patch(POST, return_value=_reply(zoom_pos=4)) as post: cam.has_motorised_lens() cam.has_motorised_lens() - assert probe.call_count == 1 + assert post.call_count == 1 def test_zoom_command_is_sent_to_a_static_varifocal_camera(): @@ -64,12 +93,12 @@ def test_zoom_command_is_sent_to_a_static_varifocal_camera(): every static camera, silently returning None with no request made.""" cam = _camera(cam_type="static") with ( - patch.object(ReolinkCamera, "get_focus_level", return_value={"focus": 1, "zoom": 0}), - patch("pyro_camera_api.camera.adapters.reolink.requests.post") as post, + patch(POST, return_value=_reply(zoom_pos=0)) as post, patch.object(ReolinkCamera, "_handle_response", return_value="ok"), ): assert cam.start_zoom_focus(32) == "ok" - assert post.call_count == 1 + # one for the probe, one for the command + assert post.call_count == 2 payload = post.call_args.kwargs["json"][0] assert payload["param"]["ZoomFocus"]["pos"] == 32 assert payload["param"]["ZoomFocus"]["op"] == "ZoomPos" @@ -77,9 +106,7 @@ def test_zoom_command_is_sent_to_a_static_varifocal_camera(): def test_zoom_command_is_not_sent_to_a_fixed_lens_camera(): cam = _camera(cam_type="static") - with ( - patch.object(ReolinkCamera, "get_focus_level", return_value={"focus": 1, "zoom": None}), - patch("pyro_camera_api.camera.adapters.reolink.requests.post") as post, - ): + with patch(POST, return_value=_reply(zoom_pos=None)) as post: assert cam.start_zoom_focus(32) is None - assert post.call_count == 0 + # only the probe + assert post.call_count == 1 From 6d0c621a8e88dc98a9463b63ebea7aa9d4804874 Mon Sep 17 00:00:00 2001 From: Jose Guillen <82850113+joseg20@users.noreply.github.com> Date: Sun, 23 Aug 2026 00:25:54 -0400 Subject: [PATCH 4/9] fix(reolink): retry rejected lens probes --- .../camera/adapters/reolink.py | 20 +++++++++---------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/pyro_camera_api/pyro_camera_api/camera/adapters/reolink.py b/pyro_camera_api/pyro_camera_api/camera/adapters/reolink.py index 85f16233..170e49b9 100644 --- a/pyro_camera_api/pyro_camera_api/camera/adapters/reolink.py +++ b/pyro_camera_api/pyro_camera_api/camera/adapters/reolink.py @@ -101,10 +101,9 @@ def __init__( def _probe_zoom_support(self) -> Optional[bool]: """Ask the camera whether it reports a zoom position. - Returns None only when the camera could not be asked, so the caller can - tell a transport failure apart from a device that answered. A camera - replying with a non-zero Reolink code has answered: it does not serve - GetZoomFocus, which settles the question. + Returns None whenever the probe is inconclusive, so the caller can + distinguish transport, HTTP, and Reolink API failures from a successful + response that establishes whether a zoom position is available. """ try: response = requests.post( @@ -120,8 +119,8 @@ def _probe_zoom_support(self) -> Optional[bool]: return None payload = response.json() if payload[0].get("code") != 0: - logger.info("[%s] camera does not serve GetZoomFocus: fixed lens", self.ip_address) - return False + logger.warning("[%s] lens probe was rejected by the camera", self.ip_address) + return None return payload[0]["value"]["ZoomFocus"].get("zoom", {}).get("pos") is not None def has_motorised_lens(self) -> bool: @@ -132,11 +131,10 @@ def has_motorised_lens(self) -> bool: nothing about zoom: Reolink bullets such as the RLC-811A or the P430 sit fixed on their mast and still ship a motorised varifocal lens. - The answer is cached once the camera gives one, since a lens cannot grow - a motor at runtime and zoom commands are frequent. Only an unanswered - probe is retried: caching that would strand a camera over one bad - request, and re-probing a camera that already said no would cost a - request on every command for the rest of the process. + A definitive answer from a successful response is cached, since a lens + cannot grow a motor at runtime and zoom commands are frequent. Failed + probes are retried so one transient error cannot strand a capable camera + until the service restarts. """ if self._has_motorised_lens is None: self._has_motorised_lens = self._probe_zoom_support() From b1091f3feeb1b7ceec39553a90956588cbbe4570 Mon Sep 17 00:00:00 2001 From: Jose Guillen <82850113+joseg20@users.noreply.github.com> Date: Sun, 23 Aug 2026 00:26:06 -0400 Subject: [PATCH 5/9] test(reolink): cover retry after rejected lens probe --- pyro_camera_api/tests/test_reolink_lens.py | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/pyro_camera_api/tests/test_reolink_lens.py b/pyro_camera_api/tests/test_reolink_lens.py index 26da6ab2..a89522ae 100644 --- a/pyro_camera_api/tests/test_reolink_lens.py +++ b/pyro_camera_api/tests/test_reolink_lens.py @@ -43,14 +43,13 @@ def test_fixed_lens_camera_reports_no_zoom_position(): assert cam.has_motorised_lens() is False -def test_a_camera_that_rejects_the_command_is_a_settled_answer(): - """A non-zero Reolink code is the camera answering that it does not serve - GetZoomFocus. Re-probing it would cost a request on every command forever.""" +def test_a_rejected_probe_is_not_cached(): + """A non-zero Reolink code is not evidence about the camera's optics.""" cam = _camera(cam_type="static") - with patch(POST, return_value=_reply(code=-9)) as post: + with patch(POST, side_effect=[_reply(code=-9), _reply(zoom_pos=0)]) as post: assert cam.has_motorised_lens() is False - assert cam.has_motorised_lens() is False - assert post.call_count == 1 + assert cam.has_motorised_lens() is True + assert post.call_count == 2 def test_unreachable_camera_is_treated_as_fixed_lens(): From f3729aab106d4687edafd96641ea93e778d99fbd Mon Sep 17 00:00:00 2001 From: Mateo Date: Thu, 8 Oct 2026 17:59:55 +0200 Subject: [PATCH 6/9] =?UTF-8?q?fix(reolink):=20address=20review=20?= =?UTF-8?q?=E2=80=94=20restore=20pre-stream=20zoom,=20harden=20the=20lens?= =?UTF-8?q?=20probe?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - stop_stream restores the zoom the camera had when the stream started (0 if unknown) instead of always resetting to 0, so static varifocal cameras keep their hand-set detection framing. - The lens probe never raises: parsing a non-JSON body or a reply without ZoomFocus is now inconclusive instead of propagating a 500. - A failing probe warns once per camera, then logs at debug. - Only static cameras are probed; PTZ cameras keep sending zoom and focus commands unconditionally, as before. - The probe uses the shared session (legacy TLS adapter) like every other Reolink call. - Drop the unreachable focus_finder guard change. --- .../pyro_camera_api/api/routes_stream.py | 27 ++++++++++-- .../camera/adapters/reolink.py | 41 +++++++++++------ pyro_camera_api/tests/test_reolink_lens.py | 44 +++++++++++++++++-- pyro_camera_api/tests/test_stream_zoom.py | 32 ++++++++++++++ 4 files changed, 123 insertions(+), 21 deletions(-) create mode 100644 pyro_camera_api/tests/test_stream_zoom.py diff --git a/pyro_camera_api/pyro_camera_api/api/routes_stream.py b/pyro_camera_api/pyro_camera_api/api/routes_stream.py index c94ffc2f..0326e463 100644 --- a/pyro_camera_api/pyro_camera_api/api/routes_stream.py +++ b/pyro_camera_api/pyro_camera_api/api/routes_stream.py @@ -32,6 +32,22 @@ router = APIRouter() logger = logging.getLogger(__name__) +# Zoom each camera had when its stream started, restored when the stream stops. +# Static varifocal cameras carry a zoom set by hand as their detection framing, +# so resetting them to 0 would lose it. +_pre_stream_zoom: dict[str, int] = {} + + +def _read_zoom(camera_ip: str) -> int: + """Current zoom position of a camera, or 0 when it cannot tell.""" + try: + zoom = (CAMERA_REGISTRY[camera_ip].get_focus_level() or {}).get("zoom") + except Exception as exc: + logger.warning("[%s] Failed to read zoom before stream, %s", camera_ip, exc) + return 0 + return zoom if isinstance(zoom, int) else 0 + + # Serializes stream startup: without it, two concurrent start_stream calls # can both pass the idempotent check and spawn two pipelines for the same # camera. The untracked duplicate keeps streaming invisibly, so stream-aware @@ -85,6 +101,7 @@ def start_stream(camera_ip: str, request: Request): # startup failed), otherwise autofocus would be blocked forever. try: stopped_cam = stop_any_running_stream(app) + _pre_stream_zoom[camera_ip] = _read_zoom(camera_ip) cfg_stream = STREAMS[camera_ip] input_url: str = cfg_stream["input_url"] output_url: str = cfg_stream["output_url"] @@ -155,8 +172,9 @@ def stop_stream(request: Request): """ Stop the currently running live stream, regardless of which camera is streaming. - After stopping streaming, a best-effort zoom reset is applied to position 0 - for cameras that support zoom control. Failures during zoom reset are ignored. + After stopping streaming, a best-effort zoom reset restores the zoom the + camera had when the stream started (0 if unknown), for cameras that support + zoom control. Failures during zoom reset are ignored. Returns: JSON confirmation and camera IP whose stream was stopped, @@ -168,9 +186,10 @@ def stop_stream(request: Request): if stopped_cam: cam = CAMERA_REGISTRY.get(stopped_cam) if cam: + zoom = _pre_stream_zoom.pop(stopped_cam, 0) try: - cam.start_zoom_focus(position=0) - logger.info("[%s] Zoom reset to position 0 after stream stop", stopped_cam) + cam.start_zoom_focus(position=zoom) + logger.info("[%s] Zoom reset to position %s after stream stop", stopped_cam, zoom) except Exception as exc: logger.warning("[%s] Failed to reset zoom, %s", stopped_cam, exc) return {"message": f"Stream for {stopped_cam} stopped. Zoom reset if supported.", "camera_ip": stopped_cam} diff --git a/pyro_camera_api/pyro_camera_api/camera/adapters/reolink.py b/pyro_camera_api/pyro_camera_api/camera/adapters/reolink.py index 170e49b9..155c74c9 100644 --- a/pyro_camera_api/pyro_camera_api/camera/adapters/reolink.py +++ b/pyro_camera_api/pyro_camera_api/camera/adapters/reolink.py @@ -97,31 +97,40 @@ def __init__( len(self.cam_azimuths), ) self._has_motorised_lens: Optional[bool] = None + self._lens_probe_warned = False def _probe_zoom_support(self) -> Optional[bool]: """Ask the camera whether it reports a zoom position. Returns None whenever the probe is inconclusive, so the caller can - distinguish transport, HTTP, and Reolink API failures from a successful - response that establishes whether a zoom position is available. + distinguish transport, HTTP, Reolink API and parsing failures from a + successful response that establishes whether a zoom position is + available. Never raises. """ try: - response = requests.post( + response = _session.post( self._build_url("GetZoomFocus"), json=[{"cmd": "GetZoomFocus", "action": 0, "param": {"channel": 0}}], verify=False, # nosec: B501 ) + if response.status_code != 200: + self._warn_probe_failure("got HTTP %s", response.status_code) + return None + reply = response.json()[0] + if reply.get("code") != 0: + self._warn_probe_failure("was rejected by the camera: %s", reply.get("error")) + return None + return reply["value"]["ZoomFocus"].get("zoom", {}).get("pos") is not None except Exception as exc: - logger.warning("[%s] lens probe could not reach the camera: %s", self.ip_address, exc) + self._warn_probe_failure("failed: %s", exc) return None - if response.status_code != 200: - logger.warning("[%s] lens probe got HTTP %s", self.ip_address, response.status_code) - return None - payload = response.json() - if payload[0].get("code") != 0: - logger.warning("[%s] lens probe was rejected by the camera", self.ip_address) - return None - return payload[0]["value"]["ZoomFocus"].get("zoom", {}).get("pos") is not None + + def _warn_probe_failure(self, msg: str, *args: Any) -> None: + # Failed probes are retried on every zoom/focus command, so only the + # first failure per camera is worth a warning. + log = logger.debug if self._lens_probe_warned else logger.warning + self._lens_probe_warned = True + log("[%s] lens probe " + msg, self.ip_address, *args) def has_motorised_lens(self) -> bool: """Whether this camera's lens can be driven. @@ -135,7 +144,13 @@ def has_motorised_lens(self) -> bool: cannot grow a motor at runtime and zoom commands are frequent. Failed probes are retried so one transient error cannot strand a capable camera until the service restarts. + + Only static cameras are probed: PTZ cameras always drive their lens, and + probing them would let a transient error drop a zoom command that used + to be sent unconditionally. """ + if self.cam_type != "static": + return True if self._has_motorised_lens is None: self._has_motorised_lens = self._probe_zoom_support() return self._has_motorised_lens is True @@ -389,7 +404,7 @@ def capture_and_score(pos: int) -> float: image.save(f"{folder}/focus_{pos}.jpg") return score_local - if not self.has_motorised_lens(): + if self.cam_type == "static": return 720 # set_manual_focus records every probed position in focus_position, so diff --git a/pyro_camera_api/tests/test_reolink_lens.py b/pyro_camera_api/tests/test_reolink_lens.py index a89522ae..70d3c31a 100644 --- a/pyro_camera_api/tests/test_reolink_lens.py +++ b/pyro_camera_api/tests/test_reolink_lens.py @@ -8,7 +8,7 @@ from pyro_camera_api.camera.adapters.reolink import ReolinkCamera -POST = "pyro_camera_api.camera.adapters.reolink.requests.post" +POST = "pyro_camera_api.camera.adapters.reolink._session.post" def _reply(status=200, code=0, zoom_pos=0): @@ -62,8 +62,8 @@ def test_unreachable_camera_is_treated_as_fixed_lens(): def test_an_unanswered_probe_is_not_cached(): """A transport failure says nothing about the optics. Caching it would - strand a PTZ camera as fixed-lens for the rest of the process.""" - cam = _camera(cam_type="ptz") + strand a varifocal camera as fixed-lens for the rest of the process.""" + cam = _camera(cam_type="static") with patch(POST, side_effect=OSError("unreachable")): assert cam.has_motorised_lens() is False with patch(POST, return_value=_reply(zoom_pos=0)): @@ -71,7 +71,7 @@ def test_an_unanswered_probe_is_not_cached(): def test_an_http_error_is_not_cached_either(): - cam = _camera(cam_type="ptz") + cam = _camera(cam_type="static") with patch(POST, return_value=_reply(status=500)): assert cam.has_motorised_lens() is False with patch(POST, return_value=_reply(zoom_pos=4)): @@ -109,3 +109,39 @@ def test_zoom_command_is_not_sent_to_a_fixed_lens_camera(): assert cam.start_zoom_focus(32) is None # only the probe assert post.call_count == 1 + + +def test_ptz_camera_is_never_probed(): + """PTZ cameras keep sending zoom commands unconditionally, as before.""" + cam = _camera(cam_type="ptz") + with patch(POST, side_effect=OSError("unreachable")) as post: + assert cam.has_motorised_lens() is True + assert post.call_count == 0 + + +def test_a_malformed_reply_does_not_raise(): + """A 200 with a body we did not expect must stay inconclusive, not raise + out of start_zoom_focus and turn the route's 400 into a 500.""" + not_json = MagicMock(status_code=200) + not_json.json.side_effect = ValueError("Expecting value") + no_zoom_focus = MagicMock(status_code=200) + no_zoom_focus.json.return_value = [{"code": 0, "value": {}}] + cam = _camera() + for reply in (not_json, no_zoom_focus): + with patch(POST, return_value=reply): + assert cam.has_motorised_lens() is False + with patch(POST, return_value=_reply(zoom_pos=0)): + assert cam.has_motorised_lens() is True + + +def test_a_failing_probe_warns_once(): + """Failed probes are retried on every command, the log must not fill up.""" + cam = _camera() + with ( + patch(POST, return_value=_reply(code=1)), + patch("pyro_camera_api.camera.adapters.reolink.logger") as log, + ): + for _ in range(3): + cam.has_motorised_lens() + assert log.warning.call_count == 1 + assert log.debug.call_count == 2 diff --git a/pyro_camera_api/tests/test_stream_zoom.py b/pyro_camera_api/tests/test_stream_zoom.py new file mode 100644 index 00000000..4ca9a8fd --- /dev/null +++ b/pyro_camera_api/tests/test_stream_zoom.py @@ -0,0 +1,32 @@ +# Copyright (C) 2022-2026, Pyronear. + +# This program is licensed under the Apache License 2.0. +# See LICENSE or go to for full license details. + + +from unittest.mock import MagicMock, patch + +from pyro_camera_api.api import routes_stream + +STOP = "pyro_camera_api.api.routes_stream.stop_any_running_stream" +REGISTRY = "pyro_camera_api.api.routes_stream.CAMERA_REGISTRY" + + +def test_stop_stream_restores_the_pre_stream_zoom(): + """A static varifocal camera at a hand-set zoom must get it back, not 0.""" + cam = MagicMock() + cam.get_focus_level.return_value = {"focus": 336, "zoom": 7} + with patch.dict(REGISTRY, {"cam": cam}, clear=True), patch(STOP, return_value="cam"): + routes_stream._pre_stream_zoom["cam"] = routes_stream._read_zoom("cam") + routes_stream.stop_stream(MagicMock()) + cam.start_zoom_focus.assert_called_once_with(position=7) + + +def test_stop_stream_falls_back_to_zero(): + """Unknown pre-stream zoom keeps the historical reset to 0.""" + cam = MagicMock() + cam.get_focus_level.side_effect = OSError("unreachable") + with patch.dict(REGISTRY, {"cam": cam}, clear=True), patch(STOP, return_value="cam"): + routes_stream._pre_stream_zoom["cam"] = routes_stream._read_zoom("cam") + routes_stream.stop_stream(MagicMock()) + cam.start_zoom_focus.assert_called_once_with(position=0) From 20d8784a756079ae43e17a3a7e5ed9185dccbc76 Mon Sep 17 00:00:00 2001 From: Mateo Date: Thu, 8 Oct 2026 18:17:52 +0200 Subject: [PATCH 7/9] fix(reolink): bound zoom and focus requests with a timeout --- .../pyro_camera_api/camera/adapters/reolink.py | 9 +++++++-- pyro_camera_api/tests/test_reolink_lens.py | 11 +++++++++++ 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/pyro_camera_api/pyro_camera_api/camera/adapters/reolink.py b/pyro_camera_api/pyro_camera_api/camera/adapters/reolink.py index 155c74c9..2a35fe4d 100644 --- a/pyro_camera_api/pyro_camera_api/camera/adapters/reolink.py +++ b/pyro_camera_api/pyro_camera_api/camera/adapters/reolink.py @@ -52,6 +52,10 @@ def init_poolmanager(self, *args: Any, **kwargs: Any) -> None: _session.headers["Connection"] = "close" _session.mount("https://", _LegacyTLSAdapter()) +# Zoom/focus reads and the zoom restore run under the stream lock in +# routes_stream, so a camera that stops answering must not hang them forever. +_ZOOM_FOCUS_TIMEOUT = 5 + class ReolinkCamera(BaseCamera, PTZMixin, FocusMixin): """ @@ -112,6 +116,7 @@ def _probe_zoom_support(self) -> Optional[bool]: self._build_url("GetZoomFocus"), json=[{"cmd": "GetZoomFocus", "action": 0, "param": {"channel": 0}}], verify=False, # nosec: B501 + timeout=_ZOOM_FOCUS_TIMEOUT, ) if response.status_code != 200: self._warn_probe_failure("got HTTP %s", response.status_code) @@ -324,7 +329,7 @@ def start_zoom_focus(self, position: int): "param": {"ZoomFocus": {"channel": 0, "pos": position, "op": "ZoomPos"}}, } ] - response = _session.post(url, json=data, verify=False) # nosec: B501 + response = _session.post(url, json=data, verify=False, timeout=_ZOOM_FOCUS_TIMEOUT) # nosec: B501 return self._handle_response(response, "Started ZoomFocus successfully.") return None @@ -350,7 +355,7 @@ def get_focus_level(self): """Retrieve the current manual focus and zoom positions.""" url = self._build_url("GetZoomFocus") data: Any = [{"cmd": "GetZoomFocus", "action": 0, "param": {"channel": 0}}] - response = _session.post(url, json=data, verify=False) # nosec: B501 + response = _session.post(url, json=data, verify=False, timeout=_ZOOM_FOCUS_TIMEOUT) # nosec: B501 result = self._handle_response(response, "Got zoom/focus values") if result and result[0]["code"] == 0: zoom_focus = result[0]["value"]["ZoomFocus"] diff --git a/pyro_camera_api/tests/test_reolink_lens.py b/pyro_camera_api/tests/test_reolink_lens.py index 70d3c31a..16e654b9 100644 --- a/pyro_camera_api/tests/test_reolink_lens.py +++ b/pyro_camera_api/tests/test_reolink_lens.py @@ -6,6 +6,8 @@ from unittest.mock import MagicMock, patch +import requests + from pyro_camera_api.camera.adapters.reolink import ReolinkCamera POST = "pyro_camera_api.camera.adapters.reolink._session.post" @@ -145,3 +147,12 @@ def test_a_failing_probe_warns_once(): cam.has_motorised_lens() assert log.warning.call_count == 1 assert log.debug.call_count == 2 + + +def test_a_hung_camera_times_out_instead_of_blocking(): + """The probe runs under the stream lock: it must be bounded, and a + timeout must stay inconclusive rather than raise.""" + cam = _camera() + with patch(POST, side_effect=requests.Timeout("read timed out")) as post: + assert cam.has_motorised_lens() is False + assert post.call_args.kwargs["timeout"] > 0 From 7824a1156c090c3d7eef20ea3e899c571de91180 Mon Sep 17 00:00:00 2001 From: Mateo Date: Thu, 8 Oct 2026 18:18:10 +0200 Subject: [PATCH 8/9] fix(stream): serialize stop_stream with stream startup --- .../pyro_camera_api/api/routes_stream.py | 11 +++++++---- pyro_camera_api/tests/test_stream_zoom.py | 14 ++++++++++++++ 2 files changed, 21 insertions(+), 4 deletions(-) diff --git a/pyro_camera_api/pyro_camera_api/api/routes_stream.py b/pyro_camera_api/pyro_camera_api/api/routes_stream.py index 0326e463..1494ec02 100644 --- a/pyro_camera_api/pyro_camera_api/api/routes_stream.py +++ b/pyro_camera_api/pyro_camera_api/api/routes_stream.py @@ -182,8 +182,12 @@ def stop_stream(request: Request): """ update_command_time() app = request.app - stopped_cam = stop_any_running_stream(app) - if stopped_cam: + # Same lock as startup: a concurrent start_stream for this camera must not + # have its freshly saved zoom popped by this stop. + with _START_STREAM_LOCK: + stopped_cam = stop_any_running_stream(app) + if not stopped_cam: + return {"message": "No active stream was running"} cam = CAMERA_REGISTRY.get(stopped_cam) if cam: zoom = _pre_stream_zoom.pop(stopped_cam, 0) @@ -192,8 +196,7 @@ def stop_stream(request: Request): logger.info("[%s] Zoom reset to position %s after stream stop", stopped_cam, zoom) except Exception as exc: logger.warning("[%s] Failed to reset zoom, %s", stopped_cam, exc) - return {"message": f"Stream for {stopped_cam} stopped. Zoom reset if supported.", "camera_ip": stopped_cam} - return {"message": "No active stream was running"} + return {"message": f"Stream for {stopped_cam} stopped. Zoom reset if supported.", "camera_ip": stopped_cam} @router.get("/status") diff --git a/pyro_camera_api/tests/test_stream_zoom.py b/pyro_camera_api/tests/test_stream_zoom.py index 4ca9a8fd..7eea42aa 100644 --- a/pyro_camera_api/tests/test_stream_zoom.py +++ b/pyro_camera_api/tests/test_stream_zoom.py @@ -30,3 +30,17 @@ def test_stop_stream_falls_back_to_zero(): routes_stream._pre_stream_zoom["cam"] = routes_stream._read_zoom("cam") routes_stream.stop_stream(MagicMock()) cam.start_zoom_focus.assert_called_once_with(position=0) + + +def test_stop_stream_holds_the_startup_lock(): + """Otherwise a concurrent start_stream could save a zoom this stop pops.""" + held = [] + + def stop(_app): + held.append(routes_stream._START_STREAM_LOCK.locked()) + return "cam" + + with patch.dict(REGISTRY, {"cam": MagicMock()}, clear=True), patch(STOP, side_effect=stop): + routes_stream.stop_stream(MagicMock()) + assert held == [True] + assert not routes_stream._START_STREAM_LOCK.locked() From a746776466adec4b5a7bbfa9c71ed7e9b558c218 Mon Sep 17 00:00:00 2001 From: Mateo Date: Thu, 8 Oct 2026 18:48:41 +0200 Subject: [PATCH 9/9] style: drop unused noqa in reolink lens tests --- pyro_camera_api/tests/test_reolink_lens.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyro_camera_api/tests/test_reolink_lens.py b/pyro_camera_api/tests/test_reolink_lens.py index 16e654b9..eb673eab 100644 --- a/pyro_camera_api/tests/test_reolink_lens.py +++ b/pyro_camera_api/tests/test_reolink_lens.py @@ -27,7 +27,7 @@ def _camera(cam_type="static"): camera_id="cam", ip_address="192.168.1.10", username="user", - password="pwd", # noqa: S106 + password="pwd", cam_type=cam_type, )