diff --git a/README.md b/README.md index 0861f35..cd43939 100644 --- a/README.md +++ b/README.md @@ -106,7 +106,10 @@ Firmware from 3.5.0 on keeps the iCE40 gateware and the ESP32-C3 Wi-Fi image in W25Q flash rather than in its image. After writing the firmware, `flash-self` waits for the pod's USB console and installs the ones it lacks from the same release (or from `blobs/` next to a local `.bin`); `install-blobs` does that step on its own, and `set-wifi` runs it -first when the ESP32-C3 image is missing. `flash-self` also refuses an image that does not +first when the ESP32-C3 image is missing. If the flashed pod answers but does not list its +blob slots (its `blob_status` fails), `flash-self` still exits 0, since the firmware is in, but +prints a warning that names the blobs it skipped; install them with `benchpod install-blobs`, +which treats the same failure as an error. `flash-self` also refuses an image that does not fit the pod's flash (a 1 MB STM32H563 cannot take firmware built for the 2 MB part). Releases can publish a signed manifest, `.sig`, next to `bench_pod_stm32.bin` and each @@ -1304,6 +1307,8 @@ Install the gateware and ESP32-C3 images the pod's firmware goes with (USB: over pod's USB network adapter when it is up, firmware 3.9.0+, else its console). By default they come from the release that matches the firmware the pod runs. Each blob's signature is reported; for now a missing or failing one never stops it. +When the pod does not list its blob slots (its `blob_status` command fails), it installs +nothing and exits non-zero with the pod's reason. | Flag | Default | Purpose | |-------------|-----------------|----------------------------------------------------------------| diff --git a/cmd/benchpod-cli/blobs.go b/cmd/benchpod-cli/blobs.go index 82a0f86..e9a62a3 100644 --- a/cmd/benchpod-cli/blobs.go +++ b/cmd/benchpod-cli/blobs.go @@ -5,6 +5,7 @@ import ( "crypto/sha256" "encoding/hex" "encoding/json" + "errors" "fmt" "io" "os" @@ -136,6 +137,43 @@ func blobsToInstall(m blobsManifest, slots []serialconsole.BlobSlot, force bool) return out } +// blobStatusError is a failed read of the pod's blob slots (blob_status, or the console's +// `blobs`): nothing can be compared, so nothing is installed. +type blobStatusError struct{ err error } + +func (e *blobStatusError) Error() string { + return fmt.Sprintf("the pod did not list its blob slots (blob_status: %v), so no blobs were installed", e.err) +} + +func (e *blobStatusError) Unwrap() error { return e.err } + +// blobNamesAt names the blobs the manifest at src lists ("gw0, gw1, gw2, esp"), or a generic +// phrase when the manifest cannot be read. +func blobNamesAt(ctx context.Context, src blobSource) string { + m, err := loadBlobsManifest(ctx, src) + var names []string + if err == nil { + for _, e := range m.Blobs { + if e.Size > 0 && e.File != "" { + names = append(names, e.Name) + } + } + } + if len(names) == 0 { + return "the gateware and ESP32-C3 images that go with this firmware" + } + return strings.Join(names, ", ") +} + +// blobStatusWarning is what flash-self prints when the flashed pod did not list its blob +// slots: the firmware is in, the blobs are not, and how to retry. +func blobStatusWarning(ctx context.Context, src blobSource, err error) string { + return fmt.Sprintf("flash-self: WARNING: the firmware is flashed, but reading the pod's blob slots failed (%v),\n"+ + " so these blobs were NOT installed: %s.\n"+ + " The pod may run old or missing gateware until they are. Install them with `benchpod install-blobs`.", + errors.Unwrap(err), blobNamesAt(ctx, src)) +} + // installBlobs brings the pod's blob slots in line with the manifest at src, over the USB // console. only, when non-empty, limits it to those slot names. func installBlobs(ctx context.Context, console blobTarget, src blobSource, force bool, only []string) error { @@ -147,7 +185,10 @@ func installBlobs(ctx context.Context, console blobTarget, src blobSource, force func installBlobsList(ctx context.Context, console blobTarget, src blobSource, force bool, only []string) ([]blobEntry, error) { slots, err := console.Blobs(ctx) if err != nil { - return nil, err + if errors.Is(err, serialconsole.ErrNoBlobSlots) { + return nil, err + } + return nil, &blobStatusError{err: err} } m, err := loadBlobsManifest(ctx, src) if err != nil { @@ -263,7 +304,10 @@ func newInstallBlobsCmd(g *globalFlags) *cobra.Command { if strings.TrimSpace(only) != "" { sel = strings.Split(only, ",") } - before, _ := target.Blobs(ctx) + before, err := target.Blobs(ctx) + if err != nil && !errors.Is(err, serialconsole.ErrNoBlobSlots) { + return fmt.Errorf("install-blobs: %w", &blobStatusError{err: err}) + } installed, err := installBlobsList(ctx, target, src, force, sel) if len(installed) > 0 { g.reportFirmware(blobsEvent(ctx, target, before, installed)) diff --git a/cmd/benchpod-cli/flash_self.go b/cmd/benchpod-cli/flash_self.go index f511754..f478eb0 100644 --- a/cmd/benchpod-cli/flash_self.go +++ b/cmd/benchpod-cli/flash_self.go @@ -599,9 +599,16 @@ func installBlobsAfterFlash(g *globalFlags, src blobSource) (after podVersions, fmt.Fprintln(os.Stderr, "flash-self: this firmware keeps its blobs built in; nothing to install") return after, installed, nil } - return after, installed, fmt.Errorf("flash-self: the firmware is flashed, but reading the pod's blob slots failed, so its blobs are not installed (retry with `benchpod install-blobs`): %w", err) + // The firmware is in: warn, naming what was skipped, rather than fail the flash. + fmt.Fprintln(os.Stderr, blobStatusWarning(ctx, src, &blobStatusError{err: err})) + return after, installed, nil } if installed, err = installBlobsList(ctx, target, src, false, nil); err != nil { + var bs *blobStatusError + if errors.As(err, &bs) { + fmt.Fprintln(os.Stderr, blobStatusWarning(ctx, src, bs)) + return after, installed, nil + } return after, installed, fmt.Errorf("flash-self: the firmware is flashed, but installing its blobs failed (retry with `benchpod install-blobs`): %w", err) } return after, installed, nil diff --git a/cmd/benchpod-cli/flash_self_network.go b/cmd/benchpod-cli/flash_self_network.go index e595358..e772f78 100644 --- a/cmd/benchpod-cli/flash_self_network.go +++ b/cmd/benchpod-cli/flash_self_network.go @@ -267,8 +267,11 @@ func runFlashSelfNetwork(g *globalFlags, f *flashSelfFlags, changed func(string) if _, berr := target.Blobs(ctx); errors.Is(berr, serialconsole.ErrNoBlobSlots) { fmt.Fprintln(os.Stderr, "flash-self: this firmware keeps its blobs built in; nothing to install") } else if berr != nil { - err = fmt.Errorf("flash-self: the firmware is updated, but reading the pod's blob slots failed, so its blobs are not installed (retry with `benchpod install-blobs`): %w", berr) - } else if installed, berr = installBlobsList(ctx, target, src, false, nil); berr != nil { + // The firmware is in: warn, naming what was skipped, rather than fail the update. + fmt.Fprintln(os.Stderr, blobStatusWarning(ctx, src, &blobStatusError{err: berr})) + } else if installed, berr = installBlobsList(ctx, target, src, false, nil); errors.As(berr, new(*blobStatusError)) { + fmt.Fprintln(os.Stderr, blobStatusWarning(ctx, src, berr)) + } else if berr != nil { err = fmt.Errorf("flash-self: the firmware is updated, but installing its blobs failed (retry with `benchpod install-blobs`): %w", berr) } } diff --git a/cmd/benchpod-cli/flash_self_network_test.go b/cmd/benchpod-cli/flash_self_network_test.go index f357a97..d3a7043 100644 --- a/cmd/benchpod-cli/flash_self_network_test.go +++ b/cmd/benchpod-cli/flash_self_network_test.go @@ -2,10 +2,12 @@ package main import ( "bufio" + "context" "crypto/sha256" "encoding/base64" "encoding/hex" "encoding/json" + "errors" "fmt" "net" "os" @@ -14,6 +16,8 @@ import ( "sync" "testing" "time" + + "github.com/embeddedci-com/benchpod-cli/internal/serialconsole" ) // fwNetPod is a pod's network API as flash-self --network uses it: status, identity, ota_* for @@ -25,6 +29,7 @@ type fwNetPod struct { flashKB int refuse string // error reply to ota_begin corrupt bool // ota_end reports a hash mismatch + blobErr string // error reply to blob_status (default "unknown cmd": blobs built in) staged []byte sha string state string @@ -77,6 +82,9 @@ func (p *fwNetPod) handle(req map[string]any) (string, bool) { case "identity_public": return okReply(map[string]any{"public": "pubkey-1"}), false case "blob_status": + if p.blobErr != "" { + return errReply(p.blobErr), false + } return errReply("unknown cmd"), false case "ota_begin": if p.refuse != "" { @@ -214,3 +222,89 @@ func TestFlashSelfNetworkRefusesCloudAndDFUFlags(t *testing.T) { t.Fatalf("enter-dfu err = %v", err) } } + +// blobsDirWith writes a blobs manifest listing names (files need not exist: nothing is fetched +// when the slots cannot be read). +func blobsDirWith(t *testing.T, names ...string) string { + dir := t.TempDir() + var entries []string + for _, n := range names { + entries = append(entries, fmt.Sprintf(`{"name":%q,"file":"blob-%s.bin","size":10,"sha256":"00"}`, n, n)) + } + m := `{"format":1,"blobs":[` + strings.Join(entries, ",") + `]}` + if err := os.WriteFile(filepath.Join(dir, blobsManifestName), []byte(m), 0o644); err != nil { + t.Fatal(err) + } + return dir +} + +// A pod that answers blob_status with an error (a dev build said "reply too large") must not +// look like one with nothing to install: flash-self warns, naming the skipped blobs and the +// retry, and keeps its exit status (the firmware itself is in). +func TestFlashSelfNetworkWarnsWhenBlobStatusFails(t *testing.T) { + fastNetworkFlash(t) + pod := &fwNetPod{t: t, version: "3.9.0", flashKB: 2048, blobErr: "reply too large"} + addr := pod.start() + file, _ := fwImage(t, 2000, 1024) + dir := blobsDirWith(t, "gw0", "gw1", "gw2", "esp") + var err error + out := captureStderr(t, func() { + err = runFlashSelfNetwork(&globalFlags{connection: addr, noFirmwareReport: true}, + &flashSelfFlags{network: true, blobsDir: dir}, noFlagChanged, []string{file}) + }) + if err != nil { + t.Fatalf("the firmware was flashed; want a warning, got err = %v", err) + } + for _, want := range []string{"WARNING", "reply too large", "NOT installed: gw0, gw1, gw2, esp", "benchpod install-blobs"} { + if !strings.Contains(out, want) { + t.Fatalf("stderr lacks %q:\n%s", want, out) + } + } + if strings.Contains(out, "already holds every blob") || strings.Contains(out, "built in") { + t.Fatalf("a failed blob_status read as nothing to install:\n%s", out) + } + if n := strings.Count(pod.cmds(), "ota_begin"); n != 1 { + t.Fatalf("installed blobs without a slot listing: %s", pod.cmds()) + } +} + +// install-blobs (installBlobsList) fails when the pod does not list its slots. +func TestInstallBlobsFailsWhenBlobStatusFails(t *testing.T) { + pod := &fwNetPod{t: t, version: "3.10.3", flashKB: 2048, blobErr: "reply too large"} + addr := pod.start() + ctx := context.Background() + lan, err := openLAN(ctx, addr) + if err != nil { + t.Fatal(err) + } + defer lan.s.Close() + target := lanBlobs{lan: lan, addr: addr} + installed, err := installBlobsList(ctx, target, blobSource{base: blobsDirWith(t, "gw0", "gw2")}, false, nil) + if err == nil || !strings.Contains(err.Error(), "reply too large") || !strings.Contains(err.Error(), "no blobs were installed") { + t.Fatalf("err = %v, want the blob_status failure", err) + } + var bs *blobStatusError + if !errors.As(err, &bs) || len(installed) != 0 { + t.Fatalf("err = %T %v, installed = %v", err, err, installed) + } + if strings.Contains(pod.cmds(), "ota_begin") { + t.Fatalf("sent a blob without a slot listing: %s", pod.cmds()) + } +} + +// "unknown cmd" still means firmware with its blobs built in, not a failure. +func TestInstallBlobsBuiltInIsNotABlobStatusFailure(t *testing.T) { + pod := &fwNetPod{t: t, version: "3.4.0", flashKB: 2048} + addr := pod.start() + ctx := context.Background() + lan, err := openLAN(ctx, addr) + if err != nil { + t.Fatal(err) + } + defer lan.s.Close() + _, err = installBlobsList(ctx, lanBlobs{lan: lan, addr: addr}, blobSource{base: blobsDirWith(t, "gw0")}, false, nil) + var bs *blobStatusError + if !errors.Is(err, serialconsole.ErrNoBlobSlots) || errors.As(err, &bs) { + t.Fatalf("err = %v, want ErrNoBlobSlots", err) + } +} diff --git a/cmd/benchpod-cli/flash_self_test.go b/cmd/benchpod-cli/flash_self_test.go index 54fdfaa..b7d31ad 100644 --- a/cmd/benchpod-cli/flash_self_test.go +++ b/cmd/benchpod-cli/flash_self_test.go @@ -113,3 +113,29 @@ func TestInstallBlobsAfterFlashFailsWhenTheConsoleNeverReturns(t *testing.T) { t.Fatalf("err = %v, want an error naming install-blobs", err) } } + +// After a DFU flash, a pod that does not list its blob slots gets a warning naming the skipped +// blobs and the retry; flash-self keeps its exit status (the firmware itself is in). +func TestInstallBlobsAfterFlashWarnsWhenTheSlotsAreNotListed(t *testing.T) { + oldWait, oldRetry, oldOpen, oldSkip := flashedConsoleWait, flashedConsoleRetry, openFlashedConsole, skipUSBNetForBlobs + t.Cleanup(func() { + flashedConsoleWait, flashedConsoleRetry, openFlashedConsole, skipUSBNetForBlobs = oldWait, oldRetry, oldOpen, oldSkip + }) + skipUSBNetForBlobs = true + openFlashedConsole = func(*globalFlags) (*serialconsole.Console, error) { + return serialconsole.NewConsole(&textConsole{status: " device : benchpod\r\n"}), nil + } + var err error + var installed []blobEntry + out := captureStderr(t, func() { + _, installed, err = installBlobsAfterFlash(&globalFlags{}, blobSource{base: blobsDirWith(t, "gw0", "gw2", "esp")}) + }) + if err != nil || len(installed) != 0 { + t.Fatalf("err = %v, installed = %v; want a warning only", err, installed) + } + for _, want := range []string{"WARNING", "NOT installed: gw0, gw2, esp", "benchpod install-blobs"} { + if !strings.Contains(out, want) { + t.Fatalf("stderr lacks %q:\n%s", want, out) + } + } +}