CameraControl: don't report unconfirmed firmware upgrades as success

[Cybis320]

Two fixes to upgradeFirmware, found while flashing a batch of GK7205V200 cameras where the results looked inconsistent from run to run.

1. None was treated as success

cam.upgrade() ends in one of two ways, and they do not mean the same thing:

• a 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 current code treats it as success unconditionally:

if result is None:
    log.info("Firmware upload completed.")
    return True

So a flash that was never confirmed prints exactly the same Firmware upgrade successful! as one that was. On the cameras here the whole flash completes in ~38 s and never sends 515 at all, so every upgrade took the unverified path and the tool always claimed success — including, presumably, when it shouldn't have.

Now None is reported as unconfirmed, and either way the camera 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 these cameras answer on 12901 and 34567 stays shut, so an open 34567 means it really did boot.

Failure paths are tightened too: 512/513/514 now state 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.

2. The keep-alive timer runs during the 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 far longer than that, 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 surfaces as a traceback from a background thread partway through every flash:

Exception in thread Thread-1:
  File ".../dvrip.py", line 463, in keep_alive
  ...
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.

Testing

Verified by repeatedly reflashing a camera with the image it was already running, so the content was a no-op and only the code path under test varied: transfer completes cleanly, the traceback is gone, and the tool correctly reports the camera back online. Camera uptime confirmed the reboot each time.

Note stopping the keep-alive did not restore the 515 confirmation — I had suspected the thread of consuming it. With the timer stopped it still never arrives, so these cameras genuinely do not confirm. That is what makes waiting for the camera the only sound verification.

Not included

There is a related upstream bug in dvrip.py this does not address: the completion loop does print(reply) instead of print(data), so the Ret values printed during a flash are a stale variable and can repeat or show a value the camera never sent. Worth reporting there — it makes every user's flash output misleading.

Draft while I flash the rest of the fleet through this path.

[Cybis320]

Correction to the analysis above, found while looking at the dvrip side.

I wrote that these cameras "genuinely do not confirm" the upgrade. That is wrong. The camera does send Ret 515 — the installed dvrip was losing it.

python-dvr on PyPI has a single release from 2022-08-05, and its stated homepage (NeiroNx/python-dvr) now 404s. Development moved to OpenIPC/python-dvr and was never published, so python-dvr>=0.0.1 can only ever resolve to that four-year-old sdist. The installed version's receive_json raises on any non-JSON reply:

UnicodeDecodeError: 'utf-8' codec can't decode byte 0xff in position 0

Upstream made it tolerant. Running the same flash against the newer library, the confirmation arrives every time:

Camera confirmed the upgrade (Ret 515).
Firmware upgrade successful - camera confirmed it and is back online.

So the erratic results had two causes, not one: the keep-alive thread colliding with the transfer (fixed here), and the decode path dropping the completion response (fixed upstream).

This PR is still worth taking as-is. It is defensive rather than cosmetic — None genuinely can mean a dropped transfer, and verifying the camera came back is the only way to distinguish that. It also helps every existing install, which is pinned to the 2022 library for as long as the requirement stays as it is.

Separately I have a branch bumping requirements.txt to install python-dvr from the maintained repo, which fixes the decode crash in the path every camera call uses. Happy to open that as its own PR if you'd like it — it seemed worth keeping apart from this one since it changes a dependency for all installs.

[Cybis320]

Added the dependency bump to this PR, so it now carries both halves of the fix.

requirements.txt:

-python-dvr>=0.0.1 ; python_version >='3.6'
+python-dvr @ git+https://github.com/OpenIPC/python-dvr@master ; python_version >='3.6'

Rationale, in short: PyPI's python-dvr is a single release from 2022-08-05 whose stated homepage 404s, so >=0.0.1 can only ever install that. The maintained tree is at OpenIPC and fixes the receive_json crash in the path every camera call uses. The version specifier had to go because the git tree declares 0.0.0 — lower than the PyPI 0.0.1 — so >=0.0.1 actively rejects the newer code. @master matches the imreg_dft line a few rows above.

Verified against a live camera: GetDeviceInformation and a full firmware upgrade both work through the new library, and the upgrade now reports Ret 515 reliably.

Of the functions RMS calls, only login, upgrade, receive_json, connect, get_command and set_command differ from the installed version; the rest are byte-identical.

Two things a reviewer should weigh, since I can't judge them for the project:

• @master is unpinned. With no version signal (0.0.0 either way), two users installing a week apart get different libraries with nothing in a bug report to distinguish them. A commit SHA would be reproducible at the cost of manual bumps. Same exposure imreg_dft already carries, so it's at least consistent.
• This changes a dependency for every install, not just camera users. I've only exercised the paths RMS uses against one camera model (GK7205V200 / IMX307).

Also sent the one remaining upstream bug as OpenIPC/python-dvr#8 — the upgrade completion loop logs a stale variable.