Repository navigation
Revert "fix(drivers): restart a driver that stays offline (#1628)" - #1771
Merged
Merged
Conversation
This reverts commit b271a76. #1628 has not shipped. Review found that it breaks the driver offline alert and restarts sleeping devices without limit: - A restart calls tel.Remove, which deletes the driver's health. The notifier skips a driver that has never succeeded, so the "driver offline" push never fires for a device that stays down. - Backoff ends at every 5 minutes with no cap. An inverter that sleeps at night is torn down and started again all night, and each restart sends driver_default_mode to a dead device and clears its readings. - A failed add of a cloud driver runs driver_init every 5 minutes. The problem it set out to fix stays real: a driver that fails to start at boot is never retried. A smaller change will retry only that case. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
frahlg
added
the
no-changeset
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
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.
Reverts #1628 before the next beta. It has not shipped, so users see no change against v0.154.2-beta.1 and the PR carries
no-changeset.Why
Review of master since v0.154.2-beta.1 found three problems:
restartLockedcallsremoveLocked, which callstelemetry.Store.Remove(registry.go:1236) and deletes the driver's health. The new record has noLastSuccessand no ticks, and the notifier skips such a driver as never started (notifications/service.go, "Cold start"). A device that stays down is restarted every few minutes and its owner is never told.driver_default_modeto a dead device and clears its readings.driver_initevery 5 minutes, a lockout risk with bad credentials.The review also found that a restart holds the driver's lifecycle gate for 15–45 s on a dead Modbus host, which stalls a Settings save, and a narrow race with driver repository activation.
What remains
The problem #1628 set out to fix is real: a Pixii stayed offline after a power outage because a driver that fails to start at boot is never retried. A follow-up PR will retry only that case (enabled, not running → retry add with backoff), without the stale-restart rules.
Test
make verify: 77 Go packages pass, build and vet clean.go test ./internal/api ./cmd/ftwpass.🤖 Generated with Claude Code