From d82c80e33e13e868099e2f17dfeee22bdb712e37 Mon Sep 17 00:00:00 2001 From: Luc Busquin Date: Tue, 18 Aug 2026 15:54:40 +0000 Subject: [PATCH 1/3] CameraControl: stop reporting unconfirmed firmware upgrades as success Upgrade results looked inconsistent because success was inferred rather than checked. cam.upgrade() ends in one of two ways and they do not mean the same thing: dict with Ret 515 - the camera explicitly confirmed the upgrade None - the connection ended with no confirmation None happens both when the camera reboots before it gets round to sending 515 and when it drops the transfer partway. The old code treated None as success unconditionally, so a flash that was never confirmed printed exactly the same "Firmware upgrade successful!" as one that was. Measured on a camera here: the whole flash completes in 38 s and never sends 515 at all, so every upgrade of that unit took the unverified path. Now None is reported as unconfirmed, and either way the camera itself settles it: waitForCameraOnline() polls the DVRIP port until the application is serving again. Port 34567 is the right liveness probe precisely because it is served by the application rather than the bootloader -- when the application is dead the camera answers on 12901 and 34567 stays shut, so an open 34567 means it really did boot. Also tightened the failure paths: 512/513/514 now say plainly that the camera kept its existing firmware, and an unrecognised code is treated as failed instead of passing with a warning. If the camera never returns, the log says so and warns against power-cycling, since it may still be writing flash. On success it prompts for the SwitchMode init push, which is easy to forget and silently leaves the camera on its old stored config. Note the progress lines printed during a transfer are not a reliable status either: dvrip's completion loop prints a stale variable, so the Ret values scrolling past can repeat or show a value the camera never sent. That is upstream in dvrip.py and is not addressed here. Verified by reflashing a camera with the image it was already running: transfer completed, no confirmation arrived, the new code waited and reported it back online, and the camera's uptime confirmed the reboot. Co-Authored-By: Claude Opus 5 (1M context) (cherry picked from commit cf2080a28fa94ca68bcffff9a8b6d4cca79b8f41) --- Utils/CameraControl.py | 105 +++++++++++++++++++++++++++++++++++------ 1 file changed, 91 insertions(+), 14 deletions(-) diff --git a/Utils/CameraControl.py b/Utils/CameraControl.py index 894dcb9e6..b4bdae1c1 100644 --- a/Utils/CameraControl.py +++ b/Utils/CameraControl.py @@ -61,6 +61,7 @@ import ipaddress as ip import binascii import socket +import time import argparse import json import pprint @@ -633,6 +634,43 @@ def manageCloudConnection(cam, opts): log.info('Enabled %s', info['NatEnable']) +def waitForCameraOnline(camera_ip, timeout=240, settle=8): + """Wait for a camera to answer on the DVRIP port again after a reboot. + + Used to settle whether a firmware upgrade actually took: the upgrade + protocol does not reliably confirm itself, so the real test is that the + camera boots and its application starts serving again. + + Port 34567 is a good liveness check specifically because it is served by the + application, not the bootloader -- when the application is dead the camera + answers on 12901 instead and 34567 stays shut. + + Arguments: + camera_ip: [str] Address of the camera. + + Keyword arguments: + timeout: [int] Seconds to wait for it to return. 240 by default. + settle: [int] Seconds to wait before polling, so we do not catch the + camera still up on the old firmware. 8 by default. + + Return: + [bool] True if the camera answered again within the timeout. + """ + sleep(settle) + + deadline = time.time() + timeout + while time.time() < deadline: + try: + s = socket.create_connection((camera_ip, 34567), timeout=3) + s.close() + return True + + except OSError: + sleep(3) + + return False + + def upgradeFirmware(cam, firmware_path, skip_confirm=False): """Upgrade the camera firmware via the DVRIP protocol. @@ -723,33 +761,72 @@ def progress_callback(*args, **kwargs): try: result = cam.upgrade(firmware_path, packetsize=0x8000, vprint=progress_callback) - if result is None: - log.info("Firmware upload completed.") - log.info("Camera is now applying the update and will reboot automatically.") - log.info("Please wait for the camera to come back online (this may take several minutes).") - return True - elif isinstance(result, dict): + # cam.upgrade() ends in one of two ways and they do NOT mean the same + # thing: + # + # dict with Ret 515 - the camera explicitly confirmed the upgrade + # None - the connection ended with no confirmation + # + # None happens both when the camera reboots before it gets round to + # sending 515 (which is fine) and when it drops the transfer partway + # (which is not). Treating None as success is why upgrade results looked + # inconsistent: a flash that was never confirmed reported exactly the + # same as one that was. So None is treated as "unconfirmed" and settled + # by waiting for the camera to come back. + # + # Note the progress lines printed during the transfer are not a reliable + # status either -- dvrip's completion loop prints a stale variable, so + # the Ret values scrolling past can repeat or show a value the camera + # never sent. + confirmed = False + + if isinstance(result, dict): ret_code = result.get('Ret', -1) if ret_code == 515: - log.info("Firmware upgrade successful!") - log.info("Camera will reboot automatically.") - return True + log.info("Camera confirmed the upgrade (Ret 515).") + confirmed = True elif ret_code in [512, 513, 514]: error_msgs = { 512: "Upgrade not started", 513: "Data errors during transfer", 514: "Upgrade failed" } - log.error("Firmware upgrade failed: %s (code %d)", - error_msgs.get(ret_code, "Unknown error"), ret_code) + log.error("Firmware upgrade FAILED: %s (code %d)", + error_msgs.get(ret_code, "Unknown error"), ret_code) + log.error("The camera kept its existing firmware.") return False else: - log.warning("Upgrade returned unexpected code: %d", ret_code) + log.error("Upgrade returned unexpected code %d - treating as failed.", + ret_code) return False - else: - log.warning("Unexpected upgrade result: %s", result) + + elif result is not None: + log.error("Unexpected upgrade result: %s", result) return False + else: + log.warning("Transfer ended without a confirmation from the camera.") + log.warning("That is normal if it rebooted early, but it has to be verified.") + + # Either way, the camera is the authority on whether this worked. + log.info("Waiting for the camera to come back online...") + back = waitForCameraOnline(cam.ip) + + if back: + if confirmed: + log.info("Firmware upgrade successful - camera confirmed it and is back online.") + else: + log.info("Camera is back online after the upgrade.") + log.info("The upgrade was not confirmed over the wire, so check the " + "build with GetDeviceInformation before relying on it.") + log.info("Remember: push 'SwitchMode init' now. Config stored on the " + "camera is not replaced by the new image's defaults.") + return True + + log.error("Camera did not come back online within the timeout.") + log.error("Do not power-cycle it yet - it may still be writing flash.") + return False + except Exception as e: log.error("Firmware upgrade failed with exception: %s", e) return False From 5181495863cfbfb5b6c4fe0ab84ec2d64aafc4ea Mon Sep 17 00:00:00 2001 From: Luc Busquin Date: Tue, 18 Aug 2026 16:04:17 +0000 Subject: [PATCH 2/3] CameraControl: stop dvrip's keep-alive timer during a firmware upload login() starts a threading.Timer that re-sends a KeepAlive every 20 s on the same socket the firmware upload uses. A transfer takes about 38 s, so the timer fires mid-upload, writes a KeepAlive into the middle of the binary stream, then tries to JSON-decode whatever comes back. Firmware packets begin with 0xFF, so this surfaced as a traceback from a background thread partway through every flash: UnicodeDecodeError: 'utf-8' codec can't decode byte 0xff in position 0 Alarming to watch, and worse than cosmetic: a second thread reading and writing the socket mid-transfer can consume replies the upload loop is waiting for. An upgrade is exactly the wrong time to be keeping a session alive, since the camera reboots at the end and the session goes with it. Verified by reflashing a camera with the image it was already running: the traceback is gone and the transfer completes cleanly. Note this did NOT restore the Ret 515 confirmation. I had suspected the keep-alive thread of consuming it; with the timer stopped it still never arrives, so these cameras genuinely do not confirm -- they reboot as soon as the image is written. That makes waiting for the camera to come back the only sound way to verify an upgrade, which is what the previous commit does. Co-Authored-By: Claude Opus 5 (1M context) (cherry picked from commit ac5916d06b541843444dceb21f72ee2d3aa157d5) --- Utils/CameraControl.py | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/Utils/CameraControl.py b/Utils/CameraControl.py index b4bdae1c1..ce5b7fed3 100644 --- a/Utils/CameraControl.py +++ b/Utils/CameraControl.py @@ -758,6 +758,31 @@ def progress_callback(*args, **kwargs): log.info("Starting firmware upgrade...") log.info("Uploading firmware to camera...") + # Stop dvrip's keep-alive timer for the duration of the transfer. + # + # login() starts a threading.Timer that re-sends a KeepAlive every 20 s on + # the SAME socket the firmware upload uses. A transfer takes far longer than + # that, so the timer fires mid-upload, writes a KeepAlive into the middle of + # the binary stream and then tries to JSON-decode whatever comes back -- + # firmware packets start with 0xFF, which surfaces as + # UnicodeDecodeError: 'utf-8' codec can't decode byte 0xff + # from a background thread. + # + # The noise is the lesser problem. The thread also consumes replies the + # upload loop is waiting for, which can swallow the camera's own completion + # response -- including the Ret 515 confirmation. Whether it lands at a bad + # moment depends on file size and camera timing, which is why upgrade + # results looked erratic between runs. + # + # There is nothing to keep alive during an upgrade: the camera reboots at + # the end and the session goes with it. + if getattr(cam, 'alive', None) is not None: + try: + cam.alive.cancel() + log.info("Paused the keep-alive timer for the transfer.") + except Exception as e: + log.warning("Could not stop the keep-alive timer: %s", e) + try: result = cam.upgrade(firmware_path, packetsize=0x8000, vprint=progress_callback) From c839a285c16a0d99cb4203e72622d47da2b0dc41 Mon Sep 17 00:00:00 2001 From: Luc Busquin Date: Tue, 18 Aug 2026 20:23:03 +0000 Subject: [PATCH 3/3] requirements: get python-dvr from its maintained repo, not the dead PyPI release python-dvr on PyPI has exactly one release, 0.0.1, uploaded 2022-08-05, and its stated homepage (NeiroNx/python-dvr) now 404s. Development moved to OpenIPC/python-dvr and was never published, so ">=0.0.1" can only ever resolve to the four-year-old sdist. Every RMS install has been getting a library that is 340 lines behind upstream with no route to anything newer. That matters because the installed version raises on any non-JSON reply: UnicodeDecodeError: 'utf-8' codec can't decode byte 0xff in position 0 Upstream fixed this by making receive_json tolerant, and get_command now strips non-printable bytes before parsing. Both sit in the path every camera call uses, so a camera returning a malformed or binary-tainted response currently takes down whatever RMS was doing. Concretely, it also fixes firmware upgrades. With the old library the camera's Ret 515 completion was being lost in exactly that decode path, so upgrades looked unconfirmed and the result varied between runs. With the new one the confirmation arrives reliably: Camera confirmed the upgrade (Ret 515). Firmware upgrade successful - camera confirmed it and is back online. Note the version specifier had to go: the git tree declares 0.0.0, which is LOWER than the PyPI 0.0.1, so ">=0.0.1" actively rejects the newer code. Pinned to @master, matching the imreg_dft line directly above. Verified against a live camera: GetDeviceInformation and a full firmware upgrade both work through the new library. Of the functions RMS calls, only login, upgrade, receive_json, connect, get_command and set_command differ; the rest are byte-identical. Co-Authored-By: Claude Opus 5 (1M context) (cherry picked from commit 37887414ff4d6815c9d6cb96fd72570a38e6a4c6) --- requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements.txt b/requirements.txt index 2318ed1d4..4ca07822c 100644 --- a/requirements.txt +++ b/requirements.txt @@ -22,7 +22,7 @@ astropy>=2.0.3 imreg_dft @ git+https://github.com/matejak/imreg_dft@master configparser==4.0.2 imageio>=2.31 -python-dvr>=0.0.1 ; python_version >='3.6' +python-dvr @ git+https://github.com/OpenIPC/python-dvr@master ; python_version >='3.6' pyqtgraph>=0.12,<0.13 ; python_version >='3.6' pyyaml; python_version>='3.6' tflite-runtime; python_version >= '3.6' and python_version < '3.12' and sys_platform != 'darwin'