dfu: accept a new session after an interrupted transfer - #49
Open
fkallay1 wants to merge 1 commit into
Open
Conversation
Aborting a serial DFU and simply trying again did nothing: the device wrote no image, yet the host reported success. adafruit-nrfutil prints 'Device programmed.' while bank_0 is still BANK_INVALID_APP. Two defects combined, and both had to be fixed. 1. hci_transport.c loses the new session's first packets. m_packet_expected_seq_number is initialised only in hci_transport_open(), which runs once per boot, so an interrupted transfer leaves it mid-stream. The next host session starts numbering again at INITIAL_ACK_NUMBER_EXPECTED and therefore mismatches - and the mismatch path acknowledges the packet before discarding it, so the peer believes it was delivered. For DFU that is silently destructive: the START packet is thrown away, the sequence numbers drift into alignment a few packets later, and the whole image is then streamed into a state machine that never started. Verified with counters read over SWD: during a retry, 'START seen' stayed at 1 while 899 data packets were processed, and exactly 3 packets were acknowledged-then-discarded (zero during the original transfer). A jump back to the initial sequence number can only mean a new session, because the peer advances only on our acknowledgement and otherwise sends either the expected number or a retransmission of the previous one. So treat it as a restart and resync. 2. dfu_start_pkt_handle() refused a START while parked mid-stream. Returning NRF_ERROR_INVALID_STATE was worse than useless: dfu_transport_serial.c APP_ERROR_CHECKs it, which rebooted the chip in the middle of the new host's handshake, and dfu_transport_ble.c can only report it, never recover from it. A START is by definition a fresh session, so start over. Fixed in the shared bank handling because both transports need it. DFU_STATE_PREPARING is still refused, since an erase may be in flight on the OTA path. Measured on a ProMicro nRF52840 with the verdict read from the device (bank_0 in the settings page over SWD), never from the DFU client's exit status - the client's report is exactly what made this bug invisible: unmodified retry without reset: 0/3 part 2 alone retry without reset: 0/3 both parts retry without reset: 12/13 Regressions pass: a valid application still boots, a normal serial flash via 1200-touch completes, an abort plus reset still lands on USB DFU, and a full BLE OTA completes. One retry in 13 still failed transiently, uncorrelated with how far the aborted transfer had got (5/5 at the deepest setting), so this is a large improvement rather than a proof of perfection. Re-verified on the final branches, each flashed on its own as well as merged. On a ProMicro nRF52840 with bank_0 read over SWD: this branch alone 2/3, merged with the stall timeout 3/3, merged with the recovery change 2/3, all four merged 3/3 - 10/12 in that run and 13/15 including the earlier one, consistent with the one-in-thirteen transient noted above. Also on a XIAO nRF52840 Sense (S140 7.3.0, no debugger), where the verdict is simply whether the application boots: with this fix, a retry issued immediately after an abort wrote the image and the application came up. Without it, on that same board, the client again reported 'Device programmed.' with a full 928 progress marks while nothing had been written and the application did not boot. The silent failure reproduces on a second board and a second SoftDevice.
This was referenced Jul 28, 2026
Owner
|
Thanks for your PRs, they won't make it into the next release but I will consider them for the next release. |
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.
Aborting a serial DFU and simply trying again did nothing: the device wrote no image, yet
the host reported success. adafruit-nrfutil prints 'Device programmed.' while bank_0 is
still BANK_INVALID_APP. Two defects combined, and both had to be fixed.
hci_transport.c loses the new session's first packets.
m_packet_expected_seq_number is initialised only in hci_transport_open(), which runs
once per boot, so an interrupted transfer leaves it mid-stream. The next host session
starts numbering again at INITIAL_ACK_NUMBER_EXPECTED and therefore mismatches - and the
mismatch path acknowledges the packet before discarding it, so the peer believes it was
delivered. For DFU that is silently destructive: the START packet is thrown away, the
sequence numbers drift into alignment a few packets later, and the whole image is then
streamed into a state machine that never started.
Verified with counters read over SWD: during a retry, 'START seen' stayed at 1 while 899
data packets were processed, and exactly 3 packets were acknowledged-then-discarded (zero
during the original transfer).
A jump back to the initial sequence number can only mean a new session, because the peer
advances only on our acknowledgement and otherwise sends either the expected number or a
retransmission of the previous one. So treat it as a restart and resync.
dfu_start_pkt_handle() refused a START while parked mid-stream.
Returning NRF_ERROR_INVALID_STATE was worse than useless: dfu_transport_serial.c
APP_ERROR_CHECKs it, which rebooted the chip in the middle of the new host's handshake,
and dfu_transport_ble.c can only report it, never recover from it. A START is by
definition a fresh session, so start over. Fixed in the shared bank handling because both
transports need it. DFU_STATE_PREPARING is still refused, since an erase may be in flight
on the OTA path.
Measured on a ProMicro nRF52840 with the verdict read from the device (bank_0 in the
settings page over SWD), never from the DFU client's exit status - the client's report is
exactly what made this bug invisible:
unmodified retry without reset: 0/3
part 2 alone retry without reset: 0/3
both parts retry without reset: 12/13
Regressions pass: a valid application still boots, a normal serial flash via 1200-touch
completes, an abort plus reset still lands on USB DFU, and a full BLE OTA completes.
One retry in 13 still failed transiently, uncorrelated with how far the aborted transfer had
got (5/5 at the deepest setting), so this is a large improvement rather than a proof of
perfection.
Re-verified on the final branches, each flashed on its own as well as merged. On a ProMicro
nRF52840 with bank_0 read over SWD: this branch alone 2/3, merged with the stall timeout 3/3, merged
with the recovery change 2/3, all four merged 3/3 - 10/12 in that run and 13/15 including the
earlier one, consistent with the one-in-thirteen transient noted above.
Also on a XIAO nRF52840 Sense (S140 7.3.0, no debugger), where the verdict is simply whether the
application boots: with this fix, a retry issued immediately after an abort wrote the image and the
application came up. Without it, on that same board, the client again reported 'Device programmed.'
with a full 928 progress marks while nothing had been written and the application did not boot. The
silent failure reproduces on a second board and a second SoftDevice.
One of four independent DFU fixes, listed in the order they matter:
Any subset can be taken, in any order. Verified: all four merge cleanly in five different orders and
in all twelve ordered pairs, and each was flashed and tested on hardware on its own as well as in
combination - 26 of 26 checks on a ProMicro nRF52840 with verdicts read over SWD, plus a XIAO
nRF52840 Sense on a different SoftDevice with no debugger.