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
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
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.
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").