Repository navigation
Conversation
A NAK whose sequence number did not match the expected one was dropped by the header check, so a device that reset its protocol state (soft reboot) could never trigger a resync. Events also reset the receive deadline, so a device emitting stdout notifications every 50 ms kept the pending command waiting forever. Accept ACK/NAK packets with any sequence number and let events pass without extending the timeout.
Stopping a running script soft-reboots the device, which restarts its protocol sequence at 0, resets the negotiated capabilities and drops the dynamic channels. Commands sent before the host noticed were ACKed by the context being torn down (and their effect lost) or NAKed afterwards. On the SOFT_REBOOT event mark a resync as pending, reset the sequence for in-flight packets and schedule a channel list refresh; the next command resyncs first. Without events there is no notification, so stop() marks the resync itself. This matches the IDE's client.
kwagyeman
changed the title
| if packet['flags'] & Flags.EVENT: | ||
| self.event_callback(packet['channel'], 0xFFFF if not packet['length'] | ||
| else struct.unpack('<H', packet['payload'])[0]) | ||
| start_time = time.time() |
There was a problem hiding this comment.
I did some testing and I can see the issue, if you configure logging you can see the events, so analysis got that right. However, this change seems to be all that's needed to make the script work/recover. To me it seems that all other changes and the firmware PR are not needed. Can you reproduce any other issue with just this fix without anything else?
There was a problem hiding this comment.
No, that line alone still loses the script 1 in 4 times, it just stops the hang.
From Claude:
Measured on an N6 with stock firmware, 20 stop()+exec() cycles on a running script, only the timeout line applied: 15 pass after the 1s timeout + resync, 5 lose the script. The camera keeps ACKing RESET/WRITE/EXEC for ~10 ms after it sends SOFT_REBOOT, the write lands in the stdin buffer the reboot wipes, every command is acknowledged and the host thinks it worked. With the NAK accepted at any sequence number the resync happens on the NAK instead of after the timeout. 1.0.7: 12/12 hang. This branch: 19/20 at 0.6s/cycle, 1 timeout on a slow reboot.
16544e1 to
26d45f5
Compare
|
Please see #29 |
Fixes the "starting the on-camera script fails every other time" report (forum 11906): a host doing
stop()thenexec()hung forever on alternate runs.Stopping a running script soft-reboots the camera. Its protocol sequence restarts at 0, the negotiated capabilities go back to defaults and dynamic channels are dropped. Commands sent before the host noticed were ACKed by the context being torn down (and their effect lost) or NAKed with a sequence error afterwards. The package then hung because of two transport bugs:
_check_seqdropped a NAK whose sequence number did not match the expected one at the header stage, so the sequence error was never seen and no resync was triggered.Unknown Event: channel=2, event=0xFFFFin the report).Changes:
SOFT_REBOOTevent mark a resync as pending, reset the sequence for in-flight packets and schedule a channel list refresh; the next command resyncs first. Without events there is no notification, sostop()marks the resync itself. This is what the IDE's client does.Tested on OPENMV_N6 with the reporter's host and camera scripts: 10/10 stop+exec cycles against the released firmware (was 50%), 8/8 against the firmware with the matching protocol fix (openmv PR "protocol: Keep the session across a soft reboot").