From 54eb8edca73b1497da513a03a7787fe15c2887df Mon Sep 17 00:00:00 2001 From: "Zachary J. Fields (OpenClaw)" Date: Tue, 4 Aug 2026 16:44:20 +0000 Subject: [PATCH 1/2] fix: recover interrupted sideload transfers --- notecard/dfu.go | 199 +++++++++++++++++++++++++++++++++---------- notecard/dfu_test.go | 198 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 351 insertions(+), 46 deletions(-) create mode 100644 notecard/dfu_test.go diff --git a/notecard/dfu.go b/notecard/dfu.go index 0828af8..1207a83 100644 --- a/notecard/dfu.go +++ b/notecard/dfu.go @@ -22,30 +22,139 @@ import ( // card.binary path. const dfuInlineChunkMax = 8192 -// Side-loads a file to the DFU area of the notecard, to avoid download -func dfuSideload(filename string, noBin bool, verbose bool) (err error) { +// Smallest binary chunk worth retrying before we use the inline dfu.put path. +// This keeps a lossy serial connection from repeatedly attempting a large COBS +// transfer that it has already shown it cannot complete. +const dfuBinaryChunkMin = 8192 + +// Number of times to retry card.binary setup or cleanup. A prior interrupted +// transfer may consume the first reset request as binary data before the +// Notecard can process it. +const dfuBinaryRecoveryAttempts = 3 + +var dfuBinaryRecoveryDelay = time.Second + +// dfuBinaryTransportError reports errors that may leave the Notecard waiting +// for the remainder of a binary transfer. +func dfuBinaryTransportError(err error) bool { + return note.ErrorContains(err, note.ErrCardIo) || note.ErrorContains(err, "{bad-bin}") +} - // Do a card.binary transaction to see if the Notecard is capable of - // doing binary sideloads, and if so, how large. The -nobin flag forces - // the slower inline dfu.put path that doesn't use card.binary at all. - binaryMax := 0 - var rsp notecard.Request - if !noBin { - rsp, err = card.TransactionRequest(notecard.Request{Req: "card.binary"}) - if note.ErrorContains(err, note.ErrCardIo) { - return err +// dfuBinaryCapacity clears any prior partial receive, then gets the capacity +// for the fast binary path. Retrying here also handles a USB serial device that +// is still appearing when the CLI starts. +func dfuBinaryCapacity(noBin bool) (int, error) { + if noBin { + return 0, nil + } + + var lastErr error + for attempt := 1; attempt <= dfuBinaryRecoveryAttempts; attempt++ { + rsp, err := card.TransactionRequest(notecard.Request{Req: "card.binary", Reset: true}) + if err == nil { + // Use shorter delays when sending to Notecard, for performance. + notecard.RequestSegmentMaxLen = 1024 + notecard.RequestSegmentDelayMs = 5 + return int(rsp.Max), nil + } + + // Preserve the historical inline fallback for older Notecards that do + // not support card.binary. + if !dfuBinaryTransportError(err) { + return 0, nil + } + + lastErr = err + if attempt < dfuBinaryRecoveryAttempts { + fmt.Printf("recovering binary transfer state (attempt %d/%d): %s\n", attempt, dfuBinaryRecoveryAttempts, err) + time.Sleep(dfuBinaryRecoveryDelay) } + } + + return 0, lastErr +} +// dfuResetBinaryBuffer abandons any partial COBS receive before another +// binary attempt or before switching to the inline sideload path. +func dfuResetBinaryBuffer() error { + var lastErr error + for attempt := 1; attempt <= dfuBinaryRecoveryAttempts; attempt++ { + _, err := card.TransactionRequest(notecard.Request{Req: "card.binary", Reset: true}) if err == nil { + return nil + } + if !dfuBinaryTransportError(err) { + return err + } - // Get the maximum size that the notecard can handle - binaryMax = int(rsp.Max) + lastErr = err + if attempt < dfuBinaryRecoveryAttempts { + time.Sleep(dfuBinaryRecoveryDelay) + } + } - // Use shorter delays when sending to Notecard, for performance - notecard.RequestSegmentMaxLen = 1024 - notecard.RequestSegmentDelayMs = 5 + return lastErr +} +// dfuTransferBinary stages and verifies one chunk in the Notecard's binary +// buffer. On failure it resets the buffer so that the caller can safely retry +// with a smaller chunk or use inline dfu.put. +func dfuTransferBinary(payload []byte) error { + payloadEncoded, err := notecard.CobsEncodeAppend(payload, byte('\n'), byte('\n')) + if err != nil { + return err + } + + recoverTransfer := func(transferErr error) error { + if resetErr := dfuResetBinaryBuffer(); resetErr != nil { + return fmt.Errorf("%w; card.binary reset failed: %v", transferErr, resetErr) } + return transferErr + } + + _, err = card.TransactionRequest(notecard.Request{ + Req: "card.binary.put", + Cobs: int32(len(payloadEncoded) - 1), + }) + if err != nil { + return recoverTransfer(err) + } + + if err = card.SendBytes(payloadEncoded); err != nil { + return recoverTransfer(err) + } + + rsp, err := card.TransactionRequest(notecard.Request{Req: "card.binary"}) + if err != nil { + return recoverTransfer(err) + } + if int(rsp.Length) != len(payload) { + return recoverTransfer(fmt.Errorf("notecard payload is insufficient (%d sent, %d received)", len(payload), rsp.Length)) + } + + return nil +} + +func dfuSmallerBinaryChunk(chunkLen int) int { + if chunkLen <= dfuBinaryChunkMin { + return 0 + } + + next := chunkLen / 2 + if next < dfuBinaryChunkMin { + next = dfuBinaryChunkMin + } + return next +} + +// Side-loads a file to the DFU area of the notecard, to avoid download +func dfuSideload(filename string, noBin bool, verbose bool) (err error) { + + // Determine whether the Notecard supports binary sideloads and clear any + // partial binary receive left by a previous failed invocation. + binaryMax, err := dfuBinaryCapacity(noBin) + if err != nil { + return err } // Read the file up-front so we can handle this common failure @@ -81,7 +190,7 @@ func dfuSideload(filename string, noBin bool, verbose bool) (err error) { return } - rsp, err = card.TransactionRequest(notecard.Request{Req: "card.version"}) + rsp, err := card.TransactionRequest(notecard.Request{Req: "card.version"}) if err != nil { fmt.Printf("card.version request failed\n") return @@ -140,6 +249,7 @@ func dfuSideload(filename string, noBin bool, verbose bool) (err error) { func loadBin(filetype notehub.UploadType, filename string, bin []byte, binaryMax int) (err error) { var req, rsp notecard.Request totalLen := len(bin) + binaryChunkLen := binaryMax // Clean up the name to be just the filename portion s := strings.Split(filename, "/") @@ -184,8 +294,8 @@ func loadBin(filetype notehub.UploadType, filename string, bin []byte, binaryMax // If we support binary, use the binary maximum for performance & reliability. // Note that we are guaranteed that if we support large binaries that the // notecard will tell us not to use compression. - if binaryMax > 0 { - chunkLen = binaryMax + if binaryChunkLen > 0 { + chunkLen = binaryChunkLen } else if chunkLen > dfuInlineChunkMax { chunkLen = dfuInlineChunkMax } @@ -203,6 +313,7 @@ func loadBin(filetype notehub.UploadType, filename string, bin []byte, binaryMax // Send the chunk to sideload offset := 0 lenRemaining := totalLen + binaryChunkRecoveryAttempt := 0 beganSecs := time.Now().UTC().Unix() for lenRemaining > 0 { @@ -227,36 +338,31 @@ func loadBin(filetype notehub.UploadType, filename string, bin []byte, binaryMax req.Status = fmt.Sprintf("%x", md5.Sum(*req.Payload)) // If we're doing binary, do the transaction - if binaryMax > 0 { - - // Encode COBS - var payloadEncoded []byte - payloadEncoded, err = notecard.CobsEncode(payload, byte('\n')) + if binaryChunkLen > 0 { + err = dfuTransferBinary(payload) if err != nil { - return - } + binaryChunkRecoveryAttempt++ + if binaryChunkRecoveryAttempt < dfuBinaryRecoveryAttempts { + fmt.Printf("binary transfer failed at offset %d (%s); waiting %s before retrying the same chunk (attempt %d/%d)\n", + offset, err, dfuBinaryRecoveryDelay, binaryChunkRecoveryAttempt+1, dfuBinaryRecoveryAttempts) + time.Sleep(dfuBinaryRecoveryDelay) + continue + } - // Send the COBS data to the notecard - req2 := notecard.Request{Req: "card.binary.put"} - req2.Cobs = int32(len(payloadEncoded)) - rsp, err = card.TransactionRequest(req2) - if err != nil { - return - } - payloadEncoded = append(payloadEncoded, byte('\n')) - err = card.SendBytes(payloadEncoded) - if err != nil { - return - } + binaryChunkRecoveryAttempt = 0 + if smallerChunkLen := dfuSmallerBinaryChunk(binaryChunkLen); smallerChunkLen > 0 { + fmt.Printf("binary transfer failed at offset %d (%s); retrying with %d-byte chunks\n", offset, err, smallerChunkLen) + binaryChunkLen = smallerChunkLen + chunkLen = binaryChunkLen + continue + } - // Verify that the binary made it to the notecard - var rsp2 notecard.Request - rsp2, err = card.TransactionRequest(notecard.Request{Req: "card.binary"}) - if err != nil { - return - } - if int(rsp2.Length) != len(payload) { - return fmt.Errorf("notecard payload is insufficient (%d sent, %d received)", len(payload), rsp2.Length) + fmt.Printf("binary transfer failed at offset %d (%s); continuing with inline dfu.put\n", offset, err) + binaryChunkLen = 0 + if chunkLen > dfuInlineChunkMax { + chunkLen = dfuInlineChunkMax + } + continue } // Now that it's been received successfully, remove the payload and @@ -265,6 +371,7 @@ func loadBin(filetype notehub.UploadType, filename string, bin []byte, binaryMax req.Binary = true } + binaryChunkRecoveryAttempt = 0 // Perform the request rsp, err = card.TransactionRequest(req) diff --git a/notecard/dfu_test.go b/notecard/dfu_test.go new file mode 100644 index 0000000..739d3fd --- /dev/null +++ b/notecard/dfu_test.go @@ -0,0 +1,198 @@ +// Copyright 2026 Blues Inc. All rights reserved. +// Use of this source code is governed by licenses granted by the +// copyright holder including that found in the LICENSE file. + +package main + +import ( + "encoding/json" + "fmt" + "testing" + + "github.com/blues/note-go/notecard" +) + +func mockCard(t *testing.T, transaction func(notecard.Request, bool, []byte) ([]byte, error)) { + t.Helper() + previousCard := card + card = ¬ecard.Context{ + TransactionFn: func(_ *notecard.Context, _ int, noResponse bool, requestJSON []byte, _ bool) ([]byte, error) { + if noResponse { + return transaction(notecard.Request{}, true, requestJSON) + } + + var request notecard.Request + if err := json.Unmarshal(requestJSON, &request); err != nil { + t.Fatalf("decode request %q: %v", requestJSON, err) + } + return transaction(request, false, nil) + }, + } + t.Cleanup(func() { + card = previousCard + }) +} + +func noDfuBinaryRecoveryDelay(t *testing.T) { + t.Helper() + previousDelay := dfuBinaryRecoveryDelay + dfuBinaryRecoveryDelay = 0 + t.Cleanup(func() { + dfuBinaryRecoveryDelay = previousDelay + }) +} + +func TestDfuBinaryCapacityResetsStaleReceive(t *testing.T) { + previousSegmentMaxLen := notecard.RequestSegmentMaxLen + previousSegmentDelayMs := notecard.RequestSegmentDelayMs + t.Cleanup(func() { + notecard.RequestSegmentMaxLen = previousSegmentMaxLen + notecard.RequestSegmentDelayMs = previousSegmentDelayMs + }) + + var receivedRequest notecard.Request + mockCard(t, func(request notecard.Request, noResponse bool, _ []byte) ([]byte, error) { + if noResponse { + t.Fatal("card.binary capacity request must expect a response") + } + receivedRequest = request + return []byte(`{"max":16384}`), nil + }) + + capacity, err := dfuBinaryCapacity(false) + if err != nil { + t.Fatalf("dfuBinaryCapacity returned an error: %v", err) + } + if receivedRequest.Req != "card.binary" || !receivedRequest.Reset { + t.Fatalf("capacity request = %#v, want card.binary reset:true", receivedRequest) + } + if capacity != 16384 { + t.Fatalf("capacity = %d, want 16384", capacity) + } +} + +func TestLoadBinShrinksBinaryChunksAfterFailedTransfer(t *testing.T) { + noDfuBinaryRecoveryDelay(t) + + var binaryPutSizes []int + var binaryResets int + var binarySends int + var lastBinaryPayloadLen int + var sentDFUPuts []notecard.Request + + mockCard(t, func(request notecard.Request, noResponse bool, raw []byte) ([]byte, error) { + if noResponse { + binarySends++ + if len(raw) == 0 || raw[len(raw)-1] != '\n' { + t.Fatalf("binary payload must end in a newline") + } + decoded, err := notecard.CobsDecode(raw[:len(raw)-1], byte('\n')) + if err != nil { + t.Fatalf("decode binary payload: %v", err) + } + lastBinaryPayloadLen = len(decoded) + return []byte("{}"), nil + } + + switch request.Req { + case "dfu.put": + if request.Body != nil { + return []byte(`{"length":8}`), nil + } + sentDFUPuts = append(sentDFUPuts, request) + return []byte(`{"pending":false}`), nil + case "card.binary.put": + binaryPutSizes = append(binaryPutSizes, int(request.Cobs)) + return []byte(`{}`), nil + case "card.binary": + if request.Reset { + binaryResets++ + return []byte(`{}`), nil + } + switch binarySends { + case 1, 2, 3: + return []byte(`{"err":"binary receive prematurely terminated {bad-bin}"}`), nil + default: + return []byte(fmt.Sprintf(`{"length":%d}`, lastBinaryPayloadLen)), nil + } + default: + t.Fatalf("unexpected request: %#v", request) + return nil, nil + } + }) + + firmware := make([]byte, 16390) + for i := range firmware { + firmware[i] = byte(i) + } + if err := loadBin("host", "firmware.bin", firmware, 16384); err != nil { + t.Fatalf("loadBin returned an error: %v", err) + } + + if binaryResets != 3 { + t.Fatalf("card.binary reset calls = %d, want 3", binaryResets) + } + if len(binaryPutSizes) != 6 { + t.Fatalf("binary put calls = %d, want 6 (three failed 16 KiB attempts and three 8 KiB attempts)", len(binaryPutSizes)) + } + if binaryPutSizes[0] != binaryPutSizes[1] || binaryPutSizes[1] != binaryPutSizes[2] || binaryPutSizes[2] <= binaryPutSizes[3] || binaryPutSizes[3] != binaryPutSizes[4] || binaryPutSizes[5] >= binaryPutSizes[4] { + t.Fatalf("binary COBS transfer sizes = %v, want three 16 KiB attempts followed by 8 KiB chunks", binaryPutSizes) + } + if len(sentDFUPuts) != 3 { + t.Fatalf("dfu.put calls = %d, want 3", len(sentDFUPuts)) + } + for i, request := range sentDFUPuts { + if !request.Binary || request.Payload != nil { + t.Fatalf("dfu.put %d did not use the staged binary payload: %#v", i, request) + } + } +} + +func TestLoadBinFallsBackToInlineAfterSmallestBinaryChunkFails(t *testing.T) { + noDfuBinaryRecoveryDelay(t) + + var binaryResets int + var inlineDFUPuts []notecard.Request + + mockCard(t, func(request notecard.Request, noResponse bool, _ []byte) ([]byte, error) { + if noResponse { + return []byte(`{}`), nil + } + + switch request.Req { + case "dfu.put": + if request.Body != nil { + return []byte(`{"length":2}`), nil + } + inlineDFUPuts = append(inlineDFUPuts, request) + return []byte(`{"pending":false}`), nil + case "card.binary.put": + return []byte(`{}`), nil + case "card.binary": + if request.Reset { + binaryResets++ + return []byte(`{}`), nil + } + return []byte(`{"err":"binary receive prematurely terminated {bad-bin}"}`), nil + default: + t.Fatalf("unexpected request: %#v", request) + return nil, nil + } + }) + + if err := loadBin("host", "firmware.bin", []byte("0123"), 2); err != nil { + t.Fatalf("loadBin returned an error: %v", err) + } + + if binaryResets != 3 { + t.Fatalf("card.binary reset calls = %d, want 3", binaryResets) + } + if len(inlineDFUPuts) != 2 { + t.Fatalf("inline dfu.put calls = %d, want 2", len(inlineDFUPuts)) + } + for i, request := range inlineDFUPuts { + if request.Binary || request.Payload == nil || len(*request.Payload) != 2 { + t.Fatalf("dfu.put %d did not use a two-byte inline payload: %#v", i, request) + } + } +} From 787f2da85b925d86a85298a1576b65c162999d7a Mon Sep 17 00:00:00 2001 From: "Zachary J. Fields (OpenClaw)" Date: Tue, 4 Aug 2026 18:40:39 +0000 Subject: [PATCH 2/2] refactor: align sideload retries with note-c --- notecard/dfu.go | 134 ++++++++++++----------------------------- notecard/dfu_test.go | 139 +++++++++++++++++++++---------------------- 2 files changed, 104 insertions(+), 169 deletions(-) diff --git a/notecard/dfu.go b/notecard/dfu.go index 1207a83..25a9c8e 100644 --- a/notecard/dfu.go +++ b/notecard/dfu.go @@ -22,22 +22,18 @@ import ( // card.binary path. const dfuInlineChunkMax = 8192 -// Smallest binary chunk worth retrying before we use the inline dfu.put path. -// This keeps a lossy serial connection from repeatedly attempting a large COBS -// transfer that it has already shown it cannot complete. -const dfuBinaryChunkMin = 8192 - -// Number of times to retry card.binary setup or cleanup. A prior interrupted -// transfer may consume the first reset request as binary data before the -// Notecard can process it. -const dfuBinaryRecoveryAttempts = 3 - -var dfuBinaryRecoveryDelay = time.Second - -// dfuBinaryTransportError reports errors that may leave the Notecard waiting -// for the remainder of a binary transfer. -func dfuBinaryTransportError(err error) bool { - return note.ErrorContains(err, note.ErrCardIo) || note.ErrorContains(err, "{bad-bin}") +// Number of attempts to make when the Notecard rejects a binary transfer with +// {bad-bin}. This mirrors note-c's NoteBinaryStoreTransmit behavior. +const dfuBinaryRetries = 3 + +var dfuBinaryRetryDelay = time.Second + +func dfuBadBinError(err error) bool { + return note.ErrorContains(err, "{bad-bin}") +} + +func dfuRecoverableSetupError(err error) bool { + return note.ErrorContains(err, note.ErrCardIo) || dfuBadBinError(err) } // dfuBinaryCapacity clears any prior partial receive, then gets the capacity @@ -49,7 +45,7 @@ func dfuBinaryCapacity(noBin bool) (int, error) { } var lastErr error - for attempt := 1; attempt <= dfuBinaryRecoveryAttempts; attempt++ { + for attempt := 1; attempt <= dfuBinaryRetries; attempt++ { rsp, err := card.TransactionRequest(notecard.Request{Req: "card.binary", Reset: true}) if err == nil { // Use shorter delays when sending to Notecard, for performance. @@ -60,93 +56,52 @@ func dfuBinaryCapacity(noBin bool) (int, error) { // Preserve the historical inline fallback for older Notecards that do // not support card.binary. - if !dfuBinaryTransportError(err) { + if !dfuRecoverableSetupError(err) { return 0, nil } lastErr = err - if attempt < dfuBinaryRecoveryAttempts { - fmt.Printf("recovering binary transfer state (attempt %d/%d): %s\n", attempt, dfuBinaryRecoveryAttempts, err) - time.Sleep(dfuBinaryRecoveryDelay) + if attempt < dfuBinaryRetries { + fmt.Printf("recovering binary transfer state (attempt %d/%d): %s\n", attempt, dfuBinaryRetries, err) + time.Sleep(dfuBinaryRetryDelay) } } return 0, lastErr } -// dfuResetBinaryBuffer abandons any partial COBS receive before another -// binary attempt or before switching to the inline sideload path. -func dfuResetBinaryBuffer() error { - var lastErr error - for attempt := 1; attempt <= dfuBinaryRecoveryAttempts; attempt++ { - _, err := card.TransactionRequest(notecard.Request{Req: "card.binary", Reset: true}) - if err == nil { - return nil - } - if !dfuBinaryTransportError(err) { - return err - } - - lastErr = err - if attempt < dfuBinaryRecoveryAttempts { - time.Sleep(dfuBinaryRecoveryDelay) - } - } - - return lastErr -} - // dfuTransferBinary stages and verifies one chunk in the Notecard's binary -// buffer. On failure it resets the buffer so that the caller can safely retry -// with a smaller chunk or use inline dfu.put. +// buffer. Its caller handles the bounded retry of a {bad-bin} validation +// failure, matching note-c's binary-store transmit path. func dfuTransferBinary(payload []byte) error { payloadEncoded, err := notecard.CobsEncodeAppend(payload, byte('\n'), byte('\n')) if err != nil { return err } - recoverTransfer := func(transferErr error) error { - if resetErr := dfuResetBinaryBuffer(); resetErr != nil { - return fmt.Errorf("%w; card.binary reset failed: %v", transferErr, resetErr) - } - return transferErr - } - _, err = card.TransactionRequest(notecard.Request{ Req: "card.binary.put", Cobs: int32(len(payloadEncoded) - 1), }) if err != nil { - return recoverTransfer(err) + return err } if err = card.SendBytes(payloadEncoded); err != nil { - return recoverTransfer(err) + return err } rsp, err := card.TransactionRequest(notecard.Request{Req: "card.binary"}) if err != nil { - return recoverTransfer(err) + return err } if int(rsp.Length) != len(payload) { - return recoverTransfer(fmt.Errorf("notecard payload is insufficient (%d sent, %d received)", len(payload), rsp.Length)) + return fmt.Errorf("notecard payload is insufficient (%d sent, %d received)", len(payload), rsp.Length) } return nil } -func dfuSmallerBinaryChunk(chunkLen int) int { - if chunkLen <= dfuBinaryChunkMin { - return 0 - } - - next := chunkLen / 2 - if next < dfuBinaryChunkMin { - next = dfuBinaryChunkMin - } - return next -} - // Side-loads a file to the DFU area of the notecard, to avoid download func dfuSideload(filename string, noBin bool, verbose bool) (err error) { @@ -249,7 +204,6 @@ func dfuSideload(filename string, noBin bool, verbose bool) (err error) { func loadBin(filetype notehub.UploadType, filename string, bin []byte, binaryMax int) (err error) { var req, rsp notecard.Request totalLen := len(bin) - binaryChunkLen := binaryMax // Clean up the name to be just the filename portion s := strings.Split(filename, "/") @@ -294,8 +248,8 @@ func loadBin(filetype notehub.UploadType, filename string, bin []byte, binaryMax // If we support binary, use the binary maximum for performance & reliability. // Note that we are guaranteed that if we support large binaries that the // notecard will tell us not to use compression. - if binaryChunkLen > 0 { - chunkLen = binaryChunkLen + if binaryMax > 0 { + chunkLen = binaryMax } else if chunkLen > dfuInlineChunkMax { chunkLen = dfuInlineChunkMax } @@ -313,7 +267,6 @@ func loadBin(filetype notehub.UploadType, filename string, bin []byte, binaryMax // Send the chunk to sideload offset := 0 lenRemaining := totalLen - binaryChunkRecoveryAttempt := 0 beganSecs := time.Now().UTC().Unix() for lenRemaining > 0 { @@ -338,31 +291,20 @@ func loadBin(filetype notehub.UploadType, filename string, bin []byte, binaryMax req.Status = fmt.Sprintf("%x", md5.Sum(*req.Payload)) // If we're doing binary, do the transaction - if binaryChunkLen > 0 { - err = dfuTransferBinary(payload) - if err != nil { - binaryChunkRecoveryAttempt++ - if binaryChunkRecoveryAttempt < dfuBinaryRecoveryAttempts { - fmt.Printf("binary transfer failed at offset %d (%s); waiting %s before retrying the same chunk (attempt %d/%d)\n", - offset, err, dfuBinaryRecoveryDelay, binaryChunkRecoveryAttempt+1, dfuBinaryRecoveryAttempts) - time.Sleep(dfuBinaryRecoveryDelay) - continue - } - - binaryChunkRecoveryAttempt = 0 - if smallerChunkLen := dfuSmallerBinaryChunk(binaryChunkLen); smallerChunkLen > 0 { - fmt.Printf("binary transfer failed at offset %d (%s); retrying with %d-byte chunks\n", offset, err, smallerChunkLen) - binaryChunkLen = smallerChunkLen - chunkLen = binaryChunkLen - continue + if binaryMax > 0 { + for attempt := 1; attempt <= dfuBinaryRetries; attempt++ { + err = dfuTransferBinary(payload) + if err == nil || !dfuBadBinError(err) { + break } - - fmt.Printf("binary transfer failed at offset %d (%s); continuing with inline dfu.put\n", offset, err) - binaryChunkLen = 0 - if chunkLen > dfuInlineChunkMax { - chunkLen = dfuInlineChunkMax + if attempt < dfuBinaryRetries { + fmt.Printf("binary transfer failed at offset %d (%s); waiting %s before retrying the same chunk (attempt %d/%d)\n", + offset, err, dfuBinaryRetryDelay, attempt+1, dfuBinaryRetries) + time.Sleep(dfuBinaryRetryDelay) } - continue + } + if err != nil { + return fmt.Errorf("binary transfer failed at offset %d after %d attempts: %w", offset, dfuBinaryRetries, err) } // Now that it's been received successfully, remove the payload and @@ -371,8 +313,6 @@ func loadBin(filetype notehub.UploadType, filename string, bin []byte, binaryMax req.Binary = true } - binaryChunkRecoveryAttempt = 0 - // Perform the request rsp, err = card.TransactionRequest(req) if err != nil { diff --git a/notecard/dfu_test.go b/notecard/dfu_test.go index 739d3fd..09e10b6 100644 --- a/notecard/dfu_test.go +++ b/notecard/dfu_test.go @@ -6,7 +6,6 @@ package main import ( "encoding/json" - "fmt" "testing" "github.com/blues/note-go/notecard" @@ -33,12 +32,12 @@ func mockCard(t *testing.T, transaction func(notecard.Request, bool, []byte) ([] }) } -func noDfuBinaryRecoveryDelay(t *testing.T) { +func noDfuBinaryRetryDelay(t *testing.T) { t.Helper() - previousDelay := dfuBinaryRecoveryDelay - dfuBinaryRecoveryDelay = 0 + previousDelay := dfuBinaryRetryDelay + dfuBinaryRetryDelay = 0 t.Cleanup(func() { - dfuBinaryRecoveryDelay = previousDelay + dfuBinaryRetryDelay = previousDelay }) } @@ -71,108 +70,76 @@ func TestDfuBinaryCapacityResetsStaleReceive(t *testing.T) { } } -func TestLoadBinShrinksBinaryChunksAfterFailedTransfer(t *testing.T) { - noDfuBinaryRecoveryDelay(t) +func TestLoadBinRetriesBadBinValidation(t *testing.T) { + noDfuBinaryRetryDelay(t) - var binaryPutSizes []int - var binaryResets int + var binaryPuts int var binarySends int - var lastBinaryPayloadLen int - var sentDFUPuts []notecard.Request + var dfuPuts []notecard.Request - mockCard(t, func(request notecard.Request, noResponse bool, raw []byte) ([]byte, error) { + mockCard(t, func(request notecard.Request, noResponse bool, _ []byte) ([]byte, error) { if noResponse { binarySends++ - if len(raw) == 0 || raw[len(raw)-1] != '\n' { - t.Fatalf("binary payload must end in a newline") - } - decoded, err := notecard.CobsDecode(raw[:len(raw)-1], byte('\n')) - if err != nil { - t.Fatalf("decode binary payload: %v", err) - } - lastBinaryPayloadLen = len(decoded) - return []byte("{}"), nil + return []byte(`{}`), nil } switch request.Req { case "dfu.put": if request.Body != nil { - return []byte(`{"length":8}`), nil + return []byte(`{"length":4}`), nil } - sentDFUPuts = append(sentDFUPuts, request) + dfuPuts = append(dfuPuts, request) return []byte(`{"pending":false}`), nil case "card.binary.put": - binaryPutSizes = append(binaryPutSizes, int(request.Cobs)) + binaryPuts++ return []byte(`{}`), nil case "card.binary": - if request.Reset { - binaryResets++ - return []byte(`{}`), nil - } - switch binarySends { - case 1, 2, 3: + if binarySends == 1 { return []byte(`{"err":"binary receive prematurely terminated {bad-bin}"}`), nil - default: - return []byte(fmt.Sprintf(`{"length":%d}`, lastBinaryPayloadLen)), nil } + return []byte(`{"length":4}`), nil default: t.Fatalf("unexpected request: %#v", request) return nil, nil } }) - firmware := make([]byte, 16390) - for i := range firmware { - firmware[i] = byte(i) - } - if err := loadBin("host", "firmware.bin", firmware, 16384); err != nil { + if err := loadBin("host", "firmware.bin", []byte("0123"), 4); err != nil { t.Fatalf("loadBin returned an error: %v", err) } - if binaryResets != 3 { - t.Fatalf("card.binary reset calls = %d, want 3", binaryResets) - } - if len(binaryPutSizes) != 6 { - t.Fatalf("binary put calls = %d, want 6 (three failed 16 KiB attempts and three 8 KiB attempts)", len(binaryPutSizes)) - } - if binaryPutSizes[0] != binaryPutSizes[1] || binaryPutSizes[1] != binaryPutSizes[2] || binaryPutSizes[2] <= binaryPutSizes[3] || binaryPutSizes[3] != binaryPutSizes[4] || binaryPutSizes[5] >= binaryPutSizes[4] { - t.Fatalf("binary COBS transfer sizes = %v, want three 16 KiB attempts followed by 8 KiB chunks", binaryPutSizes) + if binaryPuts != 2 || binarySends != 2 { + t.Fatalf("binary transfer attempts = %d puts, %d sends; want 2", binaryPuts, binarySends) } - if len(sentDFUPuts) != 3 { - t.Fatalf("dfu.put calls = %d, want 3", len(sentDFUPuts)) - } - for i, request := range sentDFUPuts { - if !request.Binary || request.Payload != nil { - t.Fatalf("dfu.put %d did not use the staged binary payload: %#v", i, request) - } + if len(dfuPuts) != 1 || !dfuPuts[0].Binary || dfuPuts[0].Payload != nil { + t.Fatalf("dfu.put did not use the validated binary payload: %#v", dfuPuts) } } -func TestLoadBinFallsBackToInlineAfterSmallestBinaryChunkFails(t *testing.T) { - noDfuBinaryRecoveryDelay(t) +func TestLoadBinStopsAfterBadBinRetryLimit(t *testing.T) { + noDfuBinaryRetryDelay(t) - var binaryResets int - var inlineDFUPuts []notecard.Request + var binaryPuts int + var binarySends int + var dfuPuts int mockCard(t, func(request notecard.Request, noResponse bool, _ []byte) ([]byte, error) { if noResponse { + binarySends++ return []byte(`{}`), nil } switch request.Req { case "dfu.put": if request.Body != nil { - return []byte(`{"length":2}`), nil + return []byte(`{"length":4}`), nil } - inlineDFUPuts = append(inlineDFUPuts, request) + dfuPuts++ return []byte(`{"pending":false}`), nil case "card.binary.put": + binaryPuts++ return []byte(`{}`), nil case "card.binary": - if request.Reset { - binaryResets++ - return []byte(`{}`), nil - } return []byte(`{"err":"binary receive prematurely terminated {bad-bin}"}`), nil default: t.Fatalf("unexpected request: %#v", request) @@ -180,19 +147,47 @@ func TestLoadBinFallsBackToInlineAfterSmallestBinaryChunkFails(t *testing.T) { } }) - if err := loadBin("host", "firmware.bin", []byte("0123"), 2); err != nil { - t.Fatalf("loadBin returned an error: %v", err) + if err := loadBin("host", "firmware.bin", []byte("0123"), 4); err == nil { + t.Fatal("loadBin succeeded after exhausting {bad-bin} retries") } - - if binaryResets != 3 { - t.Fatalf("card.binary reset calls = %d, want 3", binaryResets) + if binaryPuts != dfuBinaryRetries || binarySends != dfuBinaryRetries { + t.Fatalf("binary transfer attempts = %d puts, %d sends; want %d", binaryPuts, binarySends, dfuBinaryRetries) } - if len(inlineDFUPuts) != 2 { - t.Fatalf("inline dfu.put calls = %d, want 2", len(inlineDFUPuts)) + if dfuPuts != 0 { + t.Fatalf("dfu.put calls = %d, want 0 after failed binary validation", dfuPuts) } - for i, request := range inlineDFUPuts { - if request.Binary || request.Payload == nil || len(*request.Payload) != 2 { - t.Fatalf("dfu.put %d did not use a two-byte inline payload: %#v", i, request) +} + +func TestLoadBinDoesNotRetryNonBadBinFailure(t *testing.T) { + noDfuBinaryRetryDelay(t) + + var binaryPuts int + mockCard(t, func(request notecard.Request, noResponse bool, _ []byte) ([]byte, error) { + if noResponse { + return []byte(`{}`), nil + } + + switch request.Req { + case "dfu.put": + if request.Body != nil { + return []byte(`{"length":4}`), nil + } + return []byte(`{"pending":false}`), nil + case "card.binary.put": + binaryPuts++ + return []byte(`{}`), nil + case "card.binary": + return []byte(`{"err":"unrelated binary failure"}`), nil + default: + t.Fatalf("unexpected request: %#v", request) + return nil, nil } + }) + + if err := loadBin("host", "firmware.bin", []byte("0123"), 4); err == nil { + t.Fatal("loadBin succeeded after a non-{bad-bin} validation error") + } + if binaryPuts != 1 { + t.Fatalf("binary transfer attempts = %d, want 1 for a non-{bad-bin} error", binaryPuts) } }