agent: a READY never extends a wait, so a lost command cannot hang the host

[widgetii]

defib agent flash sat 12 minutes on hi3536dv100 (Xiongmai NBD8010S-KL-V2)
with nothing written. The process read ~100 bytes/s and wrote nothing
after the upload. The agent sends READY only from its idle loop
(agent/main.c: every ~2 s with no command in hand, never while one is
handled). recv_response() restarted its deadline on every READY, and
write_flash, read_memory, erase_flash and scan_flash waited with
recv_packet() and if cmd == RSP_READY: continue. So when a command
was lost, the idle agent's beacons kept every wait alive and the host
never timed out.

• recv_response() skips READY but keeps one deadline. READYs queued
  before the response are still read past (the case the old reset
  was for, from Enable MMU + D-cache: fix sustained host→device WRITE #20): they take microseconds, not the budget.
• The five client waits use recv_response(), so each has its own fixed
  deadline (60 s per read packet, 30 s per erase or scan step, 120 s
  per stream sector and for the final ACK). A lost command is now a
  TransportTimeout instead of a hang.

The "Bug 3" regression test re-implemented the old reset loop instead
of calling recv_response. It now runs the real recv_response against an
agent that beacons every 20 ms forever, and must time out within its
deadline. Without this change it hangs until wait_for gives up at 3 s.
Five queued READYs before an ACK still come through.

On the board: agent upload --power-cycle, then write_flash of the
board's own 16 MiB image (143 s, True), read_memory, a full-chip crc32
equal to the image, scan_flash, reboot; OpenIPC came back up. A second
CLI-timed run (write_flash ~2 s after READY, as _agent_flash_async
does) also succeeded, so the lost command itself is intermittent. The
hang it caused is what this fixes.

scan_flash's per-sector CRCs on this SoC repeat every 16 sectors: it
reads through a window that wraps at 1 MiB, while CMD_READ and CMD_CRC32
are correct. That is a separate agent bug, not touched here.

[qodo-free-for-open-source-projects]

PR Summary by Qodo

Prevent idle READY beacons from extending agent response waits

🐞 Bug fix 🧪 Tests 🕐 10-20 Minutes

AI Description

• Keep a fixed response deadline while skipping idle READY beacons, so lost commands time out.
• Apply the response helper to memory reads and flash erase, write, and scan waits.
• Test both queued READYs before an ACK and endless READYs from an idle agent.

Diagram

graph TD
  C["Client waits"] --> R["recv_response"] --> P["recv_packet"] --> D{"READY packet?"}
  A["Agent packets"] --> P
  D -->|response| S["Deliver response"]
  D -->|READY| T{"Deadline elapsed?"} -->|expired| X["TransportTimeout"]
  T -->|time left| P

Loading

High-Level Assessment

Keeping the deadline in the existing shared response helper is the right approach. Repeating deadline checks in each client operation would duplicate behavior and risk leaving a wait vulnerable.

Files changed (3) +58 / -61

Bug fix (2) +15 / -26

client.pyRoute five agent waits through the READY-skipping response helper +8/-20

Route five agent waits through the READY-skipping response helper

• Memory reads, erase progress, flash-write progress and final ACK, and scan results now use recv_response instead of handling READY packets locally. This gives each receive call a fixed timeout even when idle beacons continue.

src/defib/agent/client.py

protocol.pyStop READY packets from resetting the response deadline +7/-6

Stop READY packets from resetting the response deadline

• recv_response continues past stale READY packets while preserving its original deadline. An idle agent that keeps beaconing can therefore no longer keep a lost-command wait alive indefinitely.

src/defib/agent/protocol.py

Tests (1) +43 / -35

test_flash_stream_regression.pyExercise real response handling with queued and endless READYs +43/-35

Exercise real response handling with queued and endless READYs

• Replaces a test that reproduced the old loop with asynchronous tests of recv_response itself. They verify that queued READYs do not hide an ACK and that recurring beacons result in TransportTimeout.

tests/test_flash_stream_regression.py

[qodo-free-for-open-source-projects]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

Great, no issues found!

Qodo reviewed your code and found no material issues that require review

Tip of the day

💡 Did you know, you can pick a display mode instead of tweaking every Display option by hand

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗