Skip to content

chore: simplify external battery mode#20554

Merged
andig merged 4 commits into
masterfrom
fix/batmode
Apr 13, 2025
Merged

chore: simplify external battery mode#20554
andig merged 4 commits into
masterfrom
fix/batmode

Conversation

@andig
Copy link
Copy Markdown
Member

@andig andig commented Apr 12, 2025

Follow-up to #20455

@andig andig added the bug Something isn't working label Apr 12, 2025
@iseeberg79
Copy link
Copy Markdown
Contributor

iseeberg79 commented Apr 12, 2025

damit nimmst du das frühe Setzen vor dem Kontrol-Intervall raus; der Watchdog wird aber nie wieder gestartet, weil der Timer nie einen Reset erhält und nie wieder Zero ist. Der eigentliche Watchdog läuft jetzt im separaten Thread, der auch requiredBatteryMode aufruft. Was ich meine: der Watchdog hat keine Aufgabe mehr ...

Darum hab ich die andere Alternative vorgeschlagen... müsste einen sauberen roten Faden haben...

@andig
Copy link
Copy Markdown
Member Author

andig commented Apr 12, 2025

Der eigentliche Watchdog läuft jetzt im separaten thread

Da läuft gar kein Watchdog sondern es wird einfach wieder der interne Batteriemodus hergestellt. Oder ich verstehe Deine Begrifflichkeiten nicht.

Comment thread core/site_api.go
Comment on lines +367 to +375
// start watchdog if not running
if !disable && site.batteryModeExternalTimer.IsZero() {
go func() {
for range time.Tick(time.Second) {
if site.batteryModeWatchdogExpired() {
return
}
}()
}
}
}()
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Das ist der Code, der wg. der Bedingung nur einmal ausgeführt werden wird und jetzt nutzlos ist. Die Logik wird vom requireBatteryMode-Thread übernommen, der batteryModeWatchdogExpired regelmäßig ruft und für die Zurücksetzung sorgt

Folgendes wirkt postiv auf das UI, das eine Änderung (=! unknown) sofort anzeigt - der interne Modus wird sofort umgesetzt. Das würde ich beibehalten:

		if !disable {
			site.setBatteryMode(mode)
		}

aus batteryModeWatchdogExpired - da setzt du ja jetzt den externen Modus zurück:

...
	if elapsed > time.Minute && !site.batteryModeExternalTimer.IsZero() {
		site.SetBatteryModeExternal(api.BatteryUnknown)
		return true
	}
...

Das geht natürlich, hattest du aber vorher zugunsten des go-Threads (watchdog) mal aus einem vorherigen Entwurf verworfen.

Fazit: nicht site.setBatteryMode sollte weg, sondern die parallele go-Routine - das ändert nichts am Verhalten

Copy link
Copy Markdown
Contributor

@iseeberg79 iseeberg79 Apr 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ich dreh' mich den ganzen Tag und such' ... du hast den Reset des expiration-Timers in SetBatteryMode eingebaut?

	if site.batteryModeExternal == api.BatteryUnknown {
		site.batteryModeExternalTimer = time.Time{}
	}

Das Snippet hab ich vermisst und erklärt, warum ich obiges angenommen hab...

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Genau. Der WD ist dann schon geendet, der Timer zeigt aber an dass noch kein Reset des internen Modus passiert ist.

@andig andig merged commit 1eafc45 into master Apr 13, 2025
6 checks passed
@andig andig deleted the fix/batmode branch April 13, 2025 11:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants