Fix Shelly unreachable: roll back engine state on failed actions
Without this, a failed Execute() left engine state diverged from hardware. SyncHardwareState would then misread the mismatch as a manual override and apply a 1-hour lockout — causing either a stuck-on or stuck-off loop. Now Execute() returns per-action []error. The control loop calls Engine.RollbackAction() for each failed action, keeping engine state in sync with hardware so the next cycle simply retries. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -38,26 +38,30 @@ func NewActuator(cfg *config.Config, vc *viessmann.Client, logger *slog.Logger)
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// Execute performs a list of switching actions.
|
// Execute performs a list of switching actions and returns one error per action
|
||||||
func (a *Actuator) Execute(ctx context.Context, actions []engine.Action) error {
|
// (nil on success). Failed actions do not prevent subsequent actions from running.
|
||||||
for _, action := range actions {
|
// The caller should roll back engine state for any failed action using
|
||||||
|
// Engine.RollbackAction so that the next SyncHardwareState cycle does not mistake
|
||||||
|
// the divergence for a manual override.
|
||||||
|
func (a *Actuator) Execute(ctx context.Context, actions []engine.Action) []error {
|
||||||
|
errs := make([]error, len(actions))
|
||||||
|
for i, action := range actions {
|
||||||
if err := a.executeOne(ctx, action); err != nil {
|
if err := a.executeOne(ctx, action); err != nil {
|
||||||
a.logger.Error("action failed",
|
a.logger.Error("action failed",
|
||||||
"consumer", action.Consumer,
|
"consumer", action.Consumer,
|
||||||
"turn_on", action.TurnOn,
|
"turn_on", action.TurnOn,
|
||||||
"error", err,
|
"error", err,
|
||||||
)
|
)
|
||||||
// Continue with other actions even if one fails
|
errs[i] = err
|
||||||
continue
|
continue
|
||||||
}
|
}
|
||||||
|
|
||||||
a.logger.Info("action executed",
|
a.logger.Info("action executed",
|
||||||
"consumer", action.Consumer,
|
"consumer", action.Consumer,
|
||||||
"turn_on", action.TurnOn,
|
"turn_on", action.TurnOn,
|
||||||
"reason", action.Reason,
|
"reason", action.Reason,
|
||||||
)
|
)
|
||||||
}
|
}
|
||||||
return nil
|
return errs
|
||||||
}
|
}
|
||||||
|
|
||||||
func (a *Actuator) executeOne(ctx context.Context, action engine.Action) error {
|
func (a *Actuator) executeOne(ctx context.Context, action engine.Action) error {
|
||||||
|
|||||||
@@ -88,11 +88,11 @@ func TestExecuteTurnOnSGReady(t *testing.T) {
|
|||||||
|
|
||||||
act := testActuator(t, ipFrom(sgSrv), ipFrom(dummySrv), ipFrom(dummySrv))
|
act := testActuator(t, ipFrom(sgSrv), ipFrom(dummySrv), ipFrom(dummySrv))
|
||||||
|
|
||||||
err := act.Execute(context.Background(), []engine.Action{
|
errs := act.Execute(context.Background(), []engine.Action{
|
||||||
{Consumer: engine.ConsumerSGReady, TurnOn: true, Reason: "test"},
|
{Consumer: engine.ConsumerSGReady, TurnOn: true, Reason: "test"},
|
||||||
})
|
})
|
||||||
if err != nil {
|
if errs[0] != nil {
|
||||||
t.Fatalf("Execute failed: %v", err)
|
t.Fatalf("Execute failed: %v", errs[0])
|
||||||
}
|
}
|
||||||
|
|
||||||
if !sg.state {
|
if !sg.state {
|
||||||
@@ -112,11 +112,11 @@ func TestExecuteTurnOffWallboxA(t *testing.T) {
|
|||||||
|
|
||||||
act := testActuator(t, ipFrom(dummySrv), ipFrom(wbASrv), ipFrom(dummySrv))
|
act := testActuator(t, ipFrom(dummySrv), ipFrom(wbASrv), ipFrom(dummySrv))
|
||||||
|
|
||||||
err := act.Execute(context.Background(), []engine.Action{
|
errs := act.Execute(context.Background(), []engine.Action{
|
||||||
{Consumer: engine.ConsumerWallboxA, TurnOn: false, Reason: "import"},
|
{Consumer: engine.ConsumerWallboxA, TurnOn: false, Reason: "import"},
|
||||||
})
|
})
|
||||||
if err != nil {
|
if errs[0] != nil {
|
||||||
t.Fatalf("Execute failed: %v", err)
|
t.Fatalf("Execute failed: %v", errs[0])
|
||||||
}
|
}
|
||||||
|
|
||||||
if wbA.state {
|
if wbA.state {
|
||||||
@@ -164,13 +164,14 @@ func TestExecuteMultipleActions(t *testing.T) {
|
|||||||
|
|
||||||
act := testActuator(t, ipFrom(sgSrv), ipFrom(wbASrv), ipFrom(wbBSrv))
|
act := testActuator(t, ipFrom(sgSrv), ipFrom(wbASrv), ipFrom(wbBSrv))
|
||||||
|
|
||||||
err := act.Execute(context.Background(), []engine.Action{
|
for i, err := range act.Execute(context.Background(), []engine.Action{
|
||||||
{Consumer: engine.ConsumerSGReady, TurnOn: true},
|
{Consumer: engine.ConsumerSGReady, TurnOn: true},
|
||||||
{Consumer: engine.ConsumerWallboxA, TurnOn: true},
|
{Consumer: engine.ConsumerWallboxA, TurnOn: true},
|
||||||
{Consumer: engine.ConsumerWallboxB, TurnOn: true},
|
{Consumer: engine.ConsumerWallboxB, TurnOn: true},
|
||||||
})
|
}) {
|
||||||
if err != nil {
|
if err != nil {
|
||||||
t.Fatalf("Execute failed: %v", err)
|
t.Fatalf("Execute action %d failed: %v", i, err)
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
if !sg.state || !wbA.state || !wbB.state {
|
if !sg.state || !wbA.state || !wbB.state {
|
||||||
|
|||||||
@@ -755,6 +755,34 @@ func (e *Engine) ApplyOverride(consumer Consumer, on bool, duration time.Duratio
|
|||||||
)
|
)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// RollbackAction reverts the engine's internal state for an action that the actuator
|
||||||
|
// failed to execute. Without this, the engine believes the switch happened, diverges
|
||||||
|
// from hardware, and SyncHardwareState will misinterpret the next read-back as a
|
||||||
|
// manual override and apply a 1-hour lockout.
|
||||||
|
//
|
||||||
|
// After rollback the engine state matches hardware again, so the next cycle's
|
||||||
|
// SyncHardwareState sees no mismatch and the action is simply retried.
|
||||||
|
func (e *Engine) RollbackAction(action Action) {
|
||||||
|
cs, ok := e.consumers[action.Consumer]
|
||||||
|
if !ok {
|
||||||
|
return
|
||||||
|
}
|
||||||
|
e.logger.Warn("rolling back engine state after failed action",
|
||||||
|
"consumer", action.Consumer,
|
||||||
|
"turn_on", action.TurnOn,
|
||||||
|
)
|
||||||
|
cs.Active = !action.TurnOn
|
||||||
|
if action.TurnOn {
|
||||||
|
// Turn-on failed: undo activation side-effects
|
||||||
|
cs.ActivatedAt = time.Time{}
|
||||||
|
cs.ProactiveCharging = false
|
||||||
|
cs.ProbeStartGridW = 0
|
||||||
|
cs.LowPowerCycles = 0
|
||||||
|
}
|
||||||
|
// Turn-off failed: just restore Active=true. ActivatedAt is preserved
|
||||||
|
// (shutdown code doesn't reset it), so min-runtime stays correct.
|
||||||
|
}
|
||||||
|
|
||||||
// isHeatingPeriod returns true if heating is appropriate given the current month
|
// isHeatingPeriod returns true if heating is appropriate given the current month
|
||||||
// and outdoor temperature. If HeatingMinAmbientC is configured (> 0), ambient
|
// and outdoor temperature. If HeatingMinAmbientC is configured (> 0), ambient
|
||||||
// temperatures above that threshold suppress SG-Ready even within the heating months.
|
// temperatures above that threshold suppress SG-Ready even within the heating months.
|
||||||
|
|||||||
14
main.go
14
main.go
@@ -361,8 +361,10 @@ func runCycle(
|
|||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|
||||||
if err := act.Execute(ctx, actions); err != nil {
|
for i, err := range act.Execute(ctx, actions) {
|
||||||
logger.Error("execution failed", "error", err)
|
if err != nil {
|
||||||
|
eng.RollbackAction(actions[i])
|
||||||
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -520,8 +522,8 @@ func wwResetHandler(act *actuator.Actuator, eng *engine.Engine, cfg *config.Conf
|
|||||||
TargetTempC: cfg.Strategic.WWBaseC,
|
TargetTempC: cfg.Strategic.WWBaseC,
|
||||||
Reason: "manual WW reset via web UI",
|
Reason: "manual WW reset via web UI",
|
||||||
}
|
}
|
||||||
if err := act.Execute(ctx, []engine.Action{action}); err != nil {
|
if errs := act.Execute(ctx, []engine.Action{action}); errs[0] != nil {
|
||||||
logger.Error("WW reset: actuator failed", "error", err)
|
logger.Error("WW reset: actuator failed", "error", errs[0])
|
||||||
} else {
|
} else {
|
||||||
logger.Info("WW boost reset", "base_c", cfg.Strategic.WWBaseC, "locked_until", midnight.Format("15:04"))
|
logger.Info("WW boost reset", "base_c", cfg.Strategic.WWBaseC, "locked_until", midnight.Format("15:04"))
|
||||||
}
|
}
|
||||||
@@ -611,8 +613,8 @@ func overrideHandler(act *actuator.Actuator, eng *engine.Engine, logger *slog.Lo
|
|||||||
TurnOn: turnOn,
|
TurnOn: turnOn,
|
||||||
Reason: fmt.Sprintf("manual override via web UI (%s)", duration),
|
Reason: fmt.Sprintf("manual override via web UI (%s)", duration),
|
||||||
}
|
}
|
||||||
if err := act.Execute(ctx, []engine.Action{action}); err != nil {
|
if errs := act.Execute(ctx, []engine.Action{action}); errs[0] != nil {
|
||||||
logger.Error("web UI override failed", "consumer", consumerKey, "state", stateVal, "error", err)
|
logger.Error("web UI override failed", "consumer", consumerKey, "state", stateVal, "error", errs[0])
|
||||||
http.Error(w, "switch failed — check logs", http.StatusInternalServerError)
|
http.Error(w, "switch failed — check logs", http.StatusInternalServerError)
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user