fix(upgrade): correct appendData params to {"length": N} (hardware-validated) - #3
Merged
Merged
Conversation
…lidated)
Reversed from the hunter CUpgradeService::appendData handler and confirmed by a
live UART-free flash on a GK7205V510: appendData expects params {"length": N}
with an N-byte binary payload (the handler checks params.length == actual binary
length), NOT {"Offset","Length"} — which returned 400 "param error". prepare
({"Type":"System"}) and execute params are ignored. State machine: prepare(0->2)
-> appendData(2/4, repeatable) -> execute(4->0), with a ~60s idle timeout.
With this, DahuaClient.upgrade_firmware() successfully flashed an OpenIPC package
over DHIP end-to-end (device rebooted into the new firmware). const comment
updated to record the byte-proven param shape.
PR Summary by QodoFix firmware appendData length parameter
AI Description
Diagram
High-Level Assessment
Files changed (2)
|
Code Review by Qodo
1.
|
![qodo-free-for-open-source-projects[bot]](https://avatars.githubusercontent.com/in/1068000?s=60&v=4){kind=small}
…rams in test Addresses Qodo review on #3: - upgrade_firmware docstring + confirm ValueError no longer claim the path is byte-unproven / never run on hardware (it is validated end-to-end on a GK7205V510); docstring now states the {"length":N}+binary contract and the prepare/appendData/execute state machine. - test_upgrade_streams_file_in_chunks now records each appendData params and the real trailing-binary length and asserts params == {"length": N} with N equal to the payload for every chunk. FakeDHIPServer exposes the binary payload (__data__) so handlers can verify it. A revert to {"Offset","Length"} now fails the test (previously it passed).
Finding 1 (Qodo) also pointed at README: the API table said 'mock-validated only' and the device-support section said upgrade_firmware is 'intentionally never run against a device' — both now contradicted the validated flow. Moved firmware upgrade into 'Verified on hardware' (flashed OpenIPC on a GK7205V510 end-to-end), kept the destructive/confirm=True warning.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The
upgrader.appendDataRPC (used byDahuaClient.upgrade_firmware) was sending params{"Offset": ..., "Length": ...}, which the device rejects with 400 "param error".Reversed the
hunterdaemonsCUpgradeService::appendDatahandler on a Goke **GK7205V510** and confirmed with a live flash: the handler reads **params.length** and rejects the call unless it equals the actual trailing binary length.prepare({"Type":"System"}) andexecute` params are ignored.State machine (unchanged):
prepare(0→2)→appendData(2/4, repeatable)→execute(4→0), ~60s idle timeout.Fix
Validation
End-to-end on hardware (not a mock):
upgrade_firmware()streamed an ~11 MB OpenIPC package over DHIP,executereturned{"result": true}, the device burned the kernel+rootfs partitions and rebooted into the new firmware.const.pycomment updated to record the byte-proven param shape.pytest tests/— 30 passed.