Skip to content

fix: recover verified uploads after failed settings restoration #697

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
DRSDavidSoft wants to merge 1 commit into
base: fix/strip-register-constraints
Choose a base branch
from
Draft

fix: recover verified uploads after failed settings restoration #697

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 17 additions & 14 deletions Tools/Controller/cmd/controller/runtime_configuration.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -294,8 +294,9 @@ func commandOptions(store *appconfig.Store, fallbackProject string) control.Comm
ArduinoConfig: config.Programming.ToolchainConfig,
Avrdude: config.Programming.Avrdude,
AvrdudeConf: config.Programming.AvrdudeConf,
Programmer: configuredProgrammer(config),
HostConfig: store.Current,
AvrdudeBackend: config.Programming.AvrdudeBackend, AvrdudeAdapter: config.Programming.AvrdudeAdapter,
Programmer: configuredProgrammer(config),
HostConfig: store.Current,
BuzzerRuntime: func(settings native.Settings, haveBoard bool) appconfig.BuzzerRuntimeStatus {
state := store.BuzzerRuntimeState()
var backend, executable, backendError string
Expand Down Expand Up @@ -352,14 +353,15 @@ func commandOptions(store *appconfig.Store, fallbackProject string) control.Comm
ArduinoConfig: current.Programming.ToolchainConfig,
Avrdude: current.Programming.Avrdude,
AvrdudeConf: current.Programming.AvrdudeConf,
Programmer: configuredProgrammer(current),
HostConfig: store.Current,
BuzzerRuntime: options.BuzzerRuntime,
UpdateHostConfig: options.UpdateHostConfig,
Macros: options.Macros,
InitializeBoard: options.InitializeBoard,
BlankBoard: options.BlankBoard,
USBaspDriver: options.USBaspDriver,
AvrdudeBackend: current.Programming.AvrdudeBackend, AvrdudeAdapter: current.Programming.AvrdudeAdapter,
Programmer: configuredProgrammer(current),
HostConfig: store.Current,
BuzzerRuntime: options.BuzzerRuntime,
UpdateHostConfig: options.UpdateHostConfig,
Macros: options.Macros,
InitializeBoard: options.InitializeBoard,
BlankBoard: options.BlankBoard,
USBaspDriver: options.USBaspDriver,
}
}
return options
Expand Down Expand Up @@ -517,10 +519,11 @@ func apiOptions(
ToolchainConfig: config.Programming.ToolchainConfig,
Avrdude: config.Programming.Avrdude,
AvrdudeConf: config.Programming.AvrdudeConf,
Programmer: configuredProgrammer(config),
MotionDoorPolicy: config.Safety.MotionDoorPolicy,
RF: config.RF,
OSActions: config.OSActions,
AvrdudeBackend: config.Programming.AvrdudeBackend, AvrdudeAdapter: config.Programming.AvrdudeAdapter,
Programmer: configuredProgrammer(config),
MotionDoorPolicy: config.Safety.MotionDoorPolicy,
RF: config.RF,
OSActions: config.OSActions,
LCDPresentation: controllerapi.LCDPresentationOptions{
Enabled: config.UI.LCDServiceEnabled,
Debounce: time.Duration(config.UI.LCDPromptDebounceMS) * time.Millisecond,
Expand Down
27 changes: 16 additions & 11 deletions Tools/Controller/controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -258,6 +258,8 @@ type Options struct {
ToolchainConfig string `json:"toolchain_config,omitempty"`
Avrdude string `json:"avrdude,omitempty"`
AvrdudeConf string `json:"avrdude_conf,omitempty"`
AvrdudeBackend string `json:"avrdude_backend,omitempty"`
AvrdudeAdapter string `json:"avrdude_adapter,omitempty"`
Programmer string `json:"programmer,omitempty"`
Macros []Macro `json:"macros,omitempty"`
StripEffects []StripEffect `json:"strip_effects,omitempty"`
Expand Down Expand Up @@ -717,10 +719,11 @@ func New(options Options) *Client {
ArduinoConfig: options.ToolchainConfig,
Avrdude: options.Avrdude,
AvrdudeConf: options.AvrdudeConf,
Programmer: options.Programmer,
HostConfig: client.currentHostConfig,
UpdateHostConfig: client.updateHostConfig,
Outputs: client.outputs,
AvrdudeBackend: options.AvrdudeBackend, AvrdudeAdapter: options.AvrdudeAdapter,
Programmer: options.Programmer,
HostConfig: client.currentHostConfig,
UpdateHostConfig: client.updateHostConfig,
Outputs: client.outputs,
}
client.engine = control.NewCommandEngine(runtime, control.CommandOptions{
ProjectPath: options.ProjectPath,
Expand All @@ -732,11 +735,12 @@ func New(options Options) *Client {
ArduinoConfig: options.ToolchainConfig,
Avrdude: options.Avrdude,
AvrdudeConf: options.AvrdudeConf,
Programmer: options.Programmer,
HostConfig: client.currentHostConfig,
UpdateHostConfig: client.updateHostConfig,
Resolve: client.currentCommandOptions,
Outputs: client.outputs,
AvrdudeBackend: options.AvrdudeBackend, AvrdudeAdapter: options.AvrdudeAdapter,
Programmer: options.Programmer,
HostConfig: client.currentHostConfig,
UpdateHostConfig: client.updateHostConfig,
Resolve: client.currentCommandOptions,
Outputs: client.outputs,
})
automationContext, cancelAutomations := context.WithCancel(context.Background())
go func() {
Expand Down Expand Up @@ -826,8 +830,9 @@ func (client *Client) ApplyHostOptions(options Options) bool {
FirmwareFeatures: firmwareFeatures,
FirmwareFeaturesError: firmwareFeaturesErr,
ArduinoCLI: options.ToolchainCLI, Avrdude: options.Avrdude,
ArduinoConfig: options.ToolchainConfig,
AvrdudeConf: options.AvrdudeConf,
ArduinoConfig: options.ToolchainConfig,
AvrdudeConf: options.AvrdudeConf,
AvrdudeBackend: options.AvrdudeBackend, AvrdudeAdapter: options.AvrdudeAdapter,
Programmer: options.Programmer,
HostConfig: client.currentHostConfig,
UpdateHostConfig: client.updateHostConfig,
Expand Down
8 changes: 4 additions & 4 deletions Tools/Controller/controller_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,15 +94,15 @@ func TestPublicOptionsExposeCanonicalFirmwareFeatureStatus(t *testing.T) {
}

func TestToolchainConfigurationSurvivesPublicFacadeUpdates(t *testing.T) {
client := New(Options{ToolchainCLI: "cli", ToolchainConfig: "managed.yaml", Avrdude: "avr", AvrdudeConf: "avr.conf"})
client := New(Options{ToolchainCLI: "cli", ToolchainConfig: "managed.yaml", Avrdude: "avr", AvrdudeConf: "avr.conf", AvrdudeBackend: "cli", AvrdudeAdapter: "adapter"})
defer client.Shutdown()
options := client.currentCommandOptions()
if options.ArduinoConfig != "managed.yaml" || options.ArduinoCLI != "cli" || options.Avrdude != "avr" || options.AvrdudeConf != "avr.conf" {
if options.ArduinoConfig != "managed.yaml" || options.ArduinoCLI != "cli" || options.Avrdude != "avr" || options.AvrdudeConf != "avr.conf" || options.AvrdudeBackend != "cli" || options.AvrdudeAdapter != "adapter" {
t.Fatalf("initial programming paths lost: %#v", options)
}
client.ApplyHostOptions(Options{ToolchainCLI: "updated-cli", ToolchainConfig: "updated.yaml", Avrdude: "updated-avr", AvrdudeConf: "updated.conf"})
client.ApplyHostOptions(Options{ToolchainCLI: "updated-cli", ToolchainConfig: "updated.yaml", Avrdude: "updated-avr", AvrdudeConf: "updated.conf", AvrdudeBackend: "library", AvrdudeAdapter: "updated-adapter"})
options = client.currentCommandOptions()
if options.ArduinoConfig != "updated.yaml" || options.ArduinoCLI != "updated-cli" || options.Avrdude != "updated-avr" || options.AvrdudeConf != "updated.conf" {
if options.ArduinoConfig != "updated.yaml" || options.ArduinoCLI != "updated-cli" || options.Avrdude != "updated-avr" || options.AvrdudeConf != "updated.conf" || options.AvrdudeBackend != "library" || options.AvrdudeAdapter != "updated-adapter" {
t.Fatalf("updated programming paths lost: %#v", options)
}
}
Expand Down
15 changes: 8 additions & 7 deletions Tools/Controller/host/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,13 +39,14 @@ func controllerOptionsFromConfig(
ToolchainConfig: config.Programming.ToolchainConfig,
Avrdude: config.Programming.Avrdude,
AvrdudeConf: config.Programming.AvrdudeConf,
Programmer: config.Programming.Programmer,
Macros: publicMacros(config.Macros),
Melodies: append([]controller.Melody(nil), config.Melodies...),
StatusEffects: append([]controller.StatusLEDEffect(nil), config.StatusEffects...),
Scripts: cloneStrings(config.Scripts),
Automations: publicAutomations(config.Automations),
MotionDoorPolicy: config.Safety.MotionDoorPolicy,
AvrdudeBackend: config.Programming.AvrdudeBackend, AvrdudeAdapter: config.Programming.AvrdudeAdapter,
Programmer: config.Programming.Programmer,
Macros: publicMacros(config.Macros),
Melodies: append([]controller.Melody(nil), config.Melodies...),
StatusEffects: append([]controller.StatusLEDEffect(nil), config.StatusEffects...),
Scripts: cloneStrings(config.Scripts),
Automations: publicAutomations(config.Automations),
MotionDoorPolicy: config.Safety.MotionDoorPolicy,
LCDPresentation: controller.LCDPresentationOptions{
Enabled: config.UI.LCDServiceEnabled,
Debounce: time.Duration(config.UI.LCDPromptDebounceMS) * time.Millisecond,
Expand Down
4 changes: 3 additions & 1 deletion Tools/Controller/host/config_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,10 @@ func TestControllerConfigRetainsToolchainConfiguration(t *testing.T) {
config.Programming.ToolchainConfig = "selected-data.yaml"
config.Programming.Avrdude = "avr"
config.Programming.AvrdudeConf = "avr.conf"
config.Programming.AvrdudeBackend = "cli"
config.Programming.AvrdudeAdapter = "adapter"
got, err := controllerOptionsFromConfig(config, "config.json")
if err != nil || got.ToolchainConfig != "selected-data.yaml" || got.ToolchainCLI != "cli" || got.Avrdude != "avr" || got.AvrdudeConf != "avr.conf" {
if err != nil || got.ToolchainConfig != "selected-data.yaml" || got.ToolchainCLI != "cli" || got.Avrdude != "avr" || got.AvrdudeConf != "avr.conf" || got.AvrdudeBackend != "cli" || got.AvrdudeAdapter != "adapter" {
t.Fatalf("configured paths lost: %#v %v", got, err)
}
}
7 changes: 6 additions & 1 deletion Tools/Controller/internal/control/commands.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,8 @@ type CommandOptions struct {
ArduinoConfig string
Avrdude string
AvrdudeConf string
AvrdudeBackend string
AvrdudeAdapter string
Programmer string
HostConfig func() appconfig.Config
BuzzerRuntime func(native.Settings, bool) appconfig.BuzzerRuntimeStatus
Expand Down Expand Up @@ -4298,6 +4300,7 @@ func programCommandRun(
Method: method, Operation: operation, FQBN: options.FQBN,
ArduinoCLI: options.ArduinoCLI, ArduinoConfig: options.ArduinoConfig,
Avrdude: options.Avrdude, AvrdudeConf: options.AvrdudeConf,
AvrdudeBackend: options.AvrdudeBackend, AvrdudeAdapter: options.AvrdudeAdapter,
Programmer: options.Programmer,
USBaspAutoSlow: true,
}
Expand Down Expand Up @@ -4720,7 +4723,8 @@ func safeFlashCommand(
Method: method, Port: port, Operation: programmer.OperationBackup,
FQBN: options.FQBN, Programmer: options.Programmer,
ArduinoCLI: options.ArduinoCLI, ArduinoConfig: options.ArduinoConfig, Avrdude: options.Avrdude,
AvrdudeConf: options.AvrdudeConf,
AvrdudeConf: options.AvrdudeConf,
AvrdudeBackend: options.AvrdudeBackend, AvrdudeAdapter: options.AvrdudeAdapter,
USBaspAutoSlow: true,
ApplicationHash: snapshot.Hello.BuildHash,
ApplicationIdentitySchema: snapshot.Hello.IdentitySchema,
Expand Down Expand Up @@ -5019,6 +5023,7 @@ func recoverProgrammingCommand(
HexPath: firmwarePath, FQBN: options.FQBN,
Programmer: options.Programmer, ArduinoCLI: options.ArduinoCLI, ArduinoConfig: options.ArduinoConfig,
Avrdude: options.Avrdude, AvrdudeConf: options.AvrdudeConf,
AvrdudeBackend: options.AvrdudeBackend, AvrdudeAdapter: options.AvrdudeAdapter,
USBaspAutoSlow: true,
}
verifyErr := programmer.VerifyFlashReadbackWithRunner(
Expand Down
6 changes: 5 additions & 1 deletion Tools/Controller/internal/control/programming_lifecycle.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1174,7 +1174,11 @@ func findFailedProgrammingSession(
return session, err
}
targetSHA256 = strings.ToLower(strings.TrimSpace(targetSHA256))
matching := session.HostResult == "failed" && session.SafeStateApplied &&
// A verified write can still lose the connection while restoring settings.
// Recovery performs fresh flash readback before attempting restoration again.
recoverable := session.HostResult == "failed" ||
(session.HostResult == "succeeded" && session.Phase == "restore-failed-safe")
matching := recoverable && session.SafeStateApplied &&
strings.EqualFold(session.TargetFirmwareSHA256, targetSHA256)
if !matching {
return nil, fmt.Errorf(
Expand Down
30 changes: 30 additions & 0 deletions Tools/Controller/internal/control/programming_lifecycle_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -880,6 +880,36 @@ func TestFindRetryableProgrammingSessionUsesNewestMarkerBeforeOlderConflicts(t *
}
}

func TestProgrammingRecoveryAcceptsVerifiedWriteWithFailedRestore(t *testing.T) {
paths, firmware := programmingLifecycleFixture(t)
document, err := programmer.LoadIntelHex(firmware)
if err != nil {
t.Fatal(err)
}
identity := programmingIdentity(connectedProgrammingSnapshot(0).Port)
session := &ProgrammingSession{
Format: programmingMarkerFormat, PreparedAt: time.Now().UTC(),
Device: identity, TargetFirmwareSHA256: document.SourceSHA256,
SafeStateApplied: true, Phase: "restore-failed-safe", HostResult: "succeeded",
}
if _, err := persistProgrammingMarker(paths, session); err != nil {
t.Fatal(err)
}
if got, err := findFailedProgrammingSession(paths, identity, document.SourceSHA256); err != nil || got == nil {
t.Fatalf("verified write with failed restore must permit fresh-readback recovery: %v", err)
}
if _, err := findFailedProgrammingSession(paths, identity, strings.Repeat("0", 64)); err == nil {
t.Fatal("recovery accepted a different image")
}
session.Phase = "restored"
if _, err := persistProgrammingMarker(paths, session); err != nil {
t.Fatal(err)
}
if _, err := findFailedProgrammingSession(paths, identity, document.SourceSHA256); err == nil {
t.Fatal("completed success was treated as failed restoration")
}
}

func TestProgrammingLifecycleFailedProgrammerResultRetainsLatchAndMarker(t *testing.T) {
paths, firmware := programmingLifecycleFixture(t)
device := &fakeProgrammingDevice{
Expand Down
Loading