Mode Redesign: rename pv to smart, replace minpv with always charge (BC) - #32490
Mode Redesign: rename pv to smart, replace minpv with always charge (BC)#32490naltatis wants to merge 22 commits into
Conversation
github-actions
Bot
added
enhancement
naltatis
changed the title
naltatis
changed the title
|
I like this and think it makes it clearer what happens. I am not sure that a new user gets what always charge does. Maybe add the information that this done with minimal power, when no other sources are available. Alternative idea change the wording to "Charge without interruptions". |
|
"Always charge" has the gray subline "never pause, min _A" (3rd screenshot). The "until end of session" and "stays on permanently" messages (green, 4th screenshot) replace this message temporarily when user toggles the "1x" button. Same mechanic as battery boost enable/disable. @StefanSchoof Do you think "without interruptions" is more clear than "never pause"? |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 3 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="assets/js/components/Loadpoints/Mode.vue" line_range="216" />
<code_context>
+ padding-right: 0.5em;
+}
+/* full ring around the whole pill when the mode button is focused */
+.smart-pill:has(.smart-btn:focus-visible) {
+ outline: var(--bs-focus-ring-width) solid var(--bs-focus-ring-color);
+}
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Use of :has selector may cause inconsistent focus styling across browsers
Because the focus ring depends on `.smart-pill:has(.smart-btn:focus-visible)`, it will not work in browsers without `:has` support (e.g., some Firefox versions and older browsers), so the mode button’s focus style will be inconsistent across platforms. If you need reliable accessible focus styling, consider a solution that doesn’t use `:has`, such as toggling a class on focus in Vue or styling the inner `.smart-btn` outline instead of the pill.
Suggested implementation:
```
.smart-pill .chevron-btn {
flex-grow: 0;
flex-basis: auto;
gap: 3px;
border-radius: 0 18px 18px 0;
padding-left: 0.3em;
padding-right: 0.5em;
}
/* full ring around the whole pill when the mode button is focused, using a class instead of :has */
.smart-pill.smart-pill--focused {
outline: var(--bs-focus-ring-width) solid var(--bs-focus-ring-color);
}
/* equal width buttons */
.btn {
flex-basis: 0;
}
.btn:hover {
color: var(--evcc-gray);
}
.btn:focus-visible {
outline: var(--bs-focus-ring-width) solid var(--bs-focus-ring-color);
}
```
To fully implement this and keep focus styling accessible and consistent, you will also need to:
1. In the Vue template, ensure the pill container has the `smart-pill` class, e.g. `<div class="smart-pill">…</div>`.
2. On the mode button (`.smart-btn`), add `@focus` and `@blur` (and optionally `@focus-visible` handling if you differentiate) listeners that toggle the `smart-pill--focused` class on the parent pill:
- On focus (or when you detect focus-visible), add `smart-pill--focused` to the pill wrapper.
- On blur, remove `smart-pill--focused`.
3. If the component uses script setup or options API, add the corresponding methods or reactive state to manage adding/removing the class, keeping with the existing conventions in `Mode.vue`.
This way, the focus ring around the pill does not rely on `:has` and will work across browsers that lack support for that selector.
</issue_to_address>
### Comment 2
<location path="core/loadpoint.go" line_range="222" />
<code_context>
lp.mode = api.ModeOff
}
+ // migrate deprecated default modes; a legacy default also determines the always charge state
+ switch lp.mode {
+ case api.ModeMinPV:
</code_context>
<issue_to_address>
**issue (complexity):** Consider introducing small helpers to centralize legacy mode migration, alwaysCharge lifecycle, and pvMaxCurrent strategy to make the behavior easier to understand and maintain.
The migration and `alwaysCharge` handling are indeed scattered and partially duplicated. You can reduce complexity without changing behavior by centralizing the migration logic and making `pvMaxCurrent` explicit about its strategy.
### 1. Centralize legacy mode migration + alwaysCharge normalization
The same migration from `ModeMinPV`/`ModePV` → `ModeSmart` and `AlwaysChargeOn` is currently done in:
- `NewLoadpointFromConfig` (default mode)
- `restoreSettings` (persisted mode)
- likely also in `SetMode` / other callers
You can encapsulate this in a single helper and use it everywhere, removing the repeated `switch` blocks and ordering concerns:
```go
// normalizeModeAndAlwaysCharge handles legacy modes and their alwaysCharge implications.
func normalizeModeAndAlwaysCharge(mode api.ChargeMode, ac api.AlwaysCharge) (api.ChargeMode, api.AlwaysCharge) {
switch mode {
case api.ModeMinPV:
// legacy "min pv" => smart + continuous charging
return api.ModeSmart, api.AlwaysChargeOn
case api.ModePV:
// legacy "pv" => smart + obey thresholds
return api.ModeSmart, ac
default:
return mode, ac
}
}
```
Use it in `NewLoadpointFromConfig`:
```go
// choose sane default if mode is not set
if lp.mode = lp.DefaultMode; lp.mode == "" {
lp.mode = api.ModeOff
}
// migrate deprecated default modes; a legacy default also determines the always charge state
lp.mode, lp.alwaysCharge = normalizeModeAndAlwaysCharge(lp.mode, lp.alwaysCharge)
```
And in `restoreSettings`:
```go
// always charge before mode: mode migration below must not overwrite an already restored value
if v, err := lp.settings.String(keys.AlwaysCharge); err == nil && v != "" {
if ac, err := api.AlwaysChargeString(v); err == nil {
lp.setAlwaysCharge(ac)
}
}
if v, err := lp.settings.String(keys.Mode); err == nil && v != "" && lp.DefaultMode == api.ModeEmpty {
if mode, err := api.ChargeModeString(v); err == nil {
mode, ac := normalizeModeAndAlwaysCharge(mode, lp.GetAlwaysCharge())
lp.setAlwaysCharge(ac) // ensures normalized alwaysCharge is persisted
lp.setMode(mode) // setMode persists the result
}
}
```
If `SetMode` also has to deal with legacy values, it can call the same helper rather than re‑implementing the migration.
This removes the comment-driven ordering dependency (“always charge before mode”) and makes the migration behavior easy to audit in one place.
### 2. Make pvMaxCurrent explicit about strategy instead of reading lp.alwaysCharge
Right now `pvMaxCurrent` implicitly reads `lp.alwaysCharge.Active()`:
```go
func (lp *Loadpoint) pvMaxCurrent(sitePower, batteryBoostPower float64, batteryBuffered, batteryStart bool) float64 {
// ...
alwaysCharge := lp.alwaysCharge.Active()
// ...
}
```
This spreads policy decisions between `Update`/`pvMaxCurrent` and makes tests depend on internal state. Instead, pass the strategy in explicitly:
```go
// pvMaxCurrent calculates the maximum target current for smart mode.
func (lp *Loadpoint) pvMaxCurrent(alwaysCharge bool, sitePower, batteryBoostPower float64, batteryBuffered, batteryStart bool) float64 {
// read only once to simplify testing
minCurrent := lp.effectiveMinCurrent()
maxCurrent := lp.effectiveMaxCurrent()
// ...
if battery := batteryStart || batteryBuffered && lp.charging(); (alwaysCharge || battery) && targetCurrent < minCurrent {
// ...
}
if !alwaysCharge && lp.enabled && targetCurrent < minCurrent {
// ...
}
if !alwaysCharge && !lp.enabled {
// ...
}
return targetCurrent
}
```
Call site in the main update loop:
```go
case mode == api.ModeSmart:
// ...
targetCurrent := lp.pvMaxCurrent(lp.alwaysCharge.Active(), sitePower, batteryBoostPower, batteryBuffered, batteryStart)
// ...
```
You keep all semantics but:
- `pvMaxCurrent` becomes easier to reason about and test purely from parameters.
- The “smart mode vs always charge” interaction is centralized at the call site instead of hidden in the helper.
### 3. Encapsulate the session lifecycle of alwaysCharge
The session-scoped behavior for `AlwaysChargeOnce` is currently handled in `evVehicleDisconnectHandler`, and persistence semantics in `setAlwaysCharge`. You can encapsulate that into a small helper to make the lifecycle clearer:
```go
// resetSessionAlwaysCharge handles session-scoped semantics (AlwaysChargeOnce).
func (lp *Loadpoint) resetSessionAlwaysCharge() {
if lp.GetAlwaysCharge() == api.AlwaysChargeOnce {
if err := lp.SetAlwaysCharge(api.AlwaysChargeOff); err != nil {
lp.log.ERROR.Printf("always charge: %v", err)
}
}
}
```
Then in `evVehicleDisconnectHandler`:
```go
func (lp *Loadpoint) evVehicleDisconnectHandler(... ) {
// ...
lp.resetSessionAlwaysCharge()
// reset session limits, plan etc.
}
```
If `setAlwaysCharge` already contains the “persist only `On`” rule, you keep that intact; this helper just centralizes “when do we reset `AlwaysChargeOnce`”.
These small helpers should reduce the cognitive load (migration, strategy, lifecycle) while preserving the new functionality and not reverting the feature.
</issue_to_address>
### Comment 3
<location path="core/loadpoint_api.go" line_range="171" />
<code_context>
lp.log.DEBUG.Printf("set charge mode: %s", string(mode))
+ // normalize deprecated aliases; must happen before the change check since
+ // pv/minpv carry an always charge side effect even when mode stays smart
+ switch mode {
</code_context>
<issue_to_address>
**issue (complexity):** Consider refactoring mode handling to normalize legacy PV/MinPV and encapsulate always‑charge state changes in clearer helpers with explicit side‑effects.
You can reduce the added complexity by separating pure policy/normalization from side‑effects and by centralizing the “always charge” semantics.
### 1. Extract a pure mode/always‑charge normalization helper
Move the deprecated alias normalization (including the always‑charge side effect) into a pure helper that returns the normalized pair, and let `SetMode` only apply side‑effects based on that result:
```go
// normalizeLegacyMode maps PV/MinPV to Smart and derives alwaysCharge.
func (lp *Loadpoint) normalizeLegacyMode(mode api.ChargeMode) (api.ChargeMode, api.AlwaysCharge) {
if mode == api.ModePV || mode == api.ModeMinPV {
ac := api.AlwaysChargeOff
if mode == api.ModeMinPV {
ac = api.AlwaysChargeOn
}
return api.ModeSmart, ac
}
return mode, lp.alwaysCharge
}
func (lp *Loadpoint) SetMode(mode api.ChargeMode) {
lp.Lock()
defer lp.Unlock()
if _, err := api.ChargeModeString(mode.String()); err != nil {
lp.log.ERROR.Printf("invalid charge mode: %s", string(mode))
return
}
lp.log.DEBUG.Printf("set charge mode: %s", string(mode))
normalizedMode, ac := lp.normalizeLegacyMode(mode)
// only enforce alwaysCharge if the charger supports it
if ac != lp.alwaysCharge && !lp.chargerHasFeature(api.SwitchDevice) && !lp.chargerHasFeature(api.Continuous) {
lp.setAlwaysChargeCore(ac) // see next section
}
if lp.mode != normalizedMode {
lp.setMode(normalizedMode)
lp.batteryBoost = boostDisabled
lp.publish(keys.BatteryBoost, false)
switch normalizedMode {
case api.ModeNow, api.ModeOff:
lp.resetPhaseTimer()
lp.resetPVTimer()
lp.setPlanActive(false)
case api.ModeSmart:
if lp.alwaysCharge.Active() {
lp.resetPVTimer()
}
}
lp.requestUpdate()
}
}
```
This makes it clearer that:
- legacy PV/MinPV are normalized first,
- the resulting `(mode, alwaysCharge)` pair is then applied,
- side‑effects (timers, `requestUpdate`) happen after normalization.
You can reuse `normalizeLegacyMode` in config/restore paths to avoid duplicating the PV/MinPV handling there.
### 2. Narrow `setAlwaysCharge` to a core state change and make side‑effects explicit
Right now `setAlwaysCharge` handles state change, publishing, persistence, PV timer reset, and `requestUpdate()`. Splitting this into a core setter plus a wrapper makes the side‑effects more obvious and reusable:
```go
// setAlwaysChargeCore changes state + publish + persist (no timers, no updates).
func (lp *Loadpoint) setAlwaysChargeCore(ac api.AlwaysCharge) {
if lp.alwaysCharge == ac {
return
}
lp.alwaysCharge = ac
lp.publish(keys.AlwaysCharge, ac)
persisted := api.AlwaysChargeOff
if ac == api.AlwaysChargeOn {
persisted = api.AlwaysChargeOn
}
lp.settings.SetString(keys.AlwaysCharge, string(persisted))
}
// SetAlwaysCharge sets the always charge state with explicit side effects.
func (lp *Loadpoint) SetAlwaysCharge(ac api.AlwaysCharge) error {
lp.Lock()
defer lp.Unlock()
if lp.chargerHasFeature(api.SwitchDevice) || lp.chargerHasFeature(api.Continuous) {
return errors.New("always charge is not supported by this charger")
}
lp.log.DEBUG.Println("set always charge:", ac)
lp.setAlwaysChargeCore(ac)
if ac.Active() {
lp.resetPVTimer()
}
lp.requestUpdate()
return nil
}
```
Then in `SetMode` (after normalization) you can call `setAlwaysChargeCore` or `SetAlwaysCharge` depending on whether you need timers/updates. This reduces hidden side‑effects and makes the interaction between mode changes, always‑charge, and timers more predictable and easier to reason about.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
For the English language I cannot say. In German I would think "Keine Unterbrechungen" is slightly better then "nie pausieren". But I think both will work. |
|
Continuous/Unterbrechungsfrei? |
|
JFYI: At least in HA the possibility to show/hide (activate/deactivate) switches is quite limited. With this change, the 'always active' select option should be only visible/usable when Mode is set to While it's possible to render a Sensor/Select as disabled - to connect that property to the value of another entity is at least for me) quite unusual. A possible workaround would be to combine mode + activeCharge (for real wallbox loadoints) into a single select - and make two API calls... But I also does not like this idea. Right now I think, I will follow the simple approach, to add the new select for ActiveCharge - and just show it all the time - and let it up to users, to be aware, that this new select has only a effect, when mode I am just sharing this here for the other devs who might have to think about, how they can integrate this change (when there is no option to alter the GUI) |
|
@StefanSchoof @andig I've now switched to "no interruptions" / "keine Unterbrechungen". I avoided using the |
|
is there a 'simpler' way to test this (the HA integration adjustments), than checkout the evcc |
no, not right now. |
|
During the discussion #3530, there was significant demand for a fourth mode based purely on PV output. I am wondering, that this mode is no longer planned? It could also be called “Eco Mode” and expanded to include (optional) low-cost grid charging, making it usable for users without a PV system as well. I definitely see the advantage of a fourth mode, for the following reasons: “Smart Mode” suggests that charging and control are forecast-based and intelligent. That is not yet the case. Once the Optimizer has reached a level of development where it can be used for control purposes, I see its application solely in the context of planned charging. This is because its goal is always to cover the planned energy consumption as cost-effectively as possible over a certain time horizon. For unplanned charging, the amount of energy I charge into the electric car is undefined, so it’s difficult to optimize charging costs in this scenario. A pure PV/Eco mode based on deterministic rules—just as the PV mode currently works—would thus be an alternative control approach that would be lost with the use of the Optimizer. |
Resolved in 888f9fc. I merged 🤖 Generated with Claude Code |
|
Done for the Garmin app as ticket #184 and released as v2.0.6-beta5 to the beta channel. I plan to release it to the stable channel in a couple of days. I have not implemented a switch between the old and new mode labels. The new version will use the new labels for off and now immediately. For the Garmin app, this is not really a breaking change. While the app maps known modes to display labels, it simply displays the API value if it encounters an unknown mode. |
|
Done for the Homey app: https://github.com/rdvnit/com.evcc.io/tree/feature/evcc-smart-mode The app detects the new mode schema through The changes will be included in the next Homey app release. |
pairs with evcc-io/evcc#32490 and mirrors targets/widget/Loadpoint.swift's handling from evcc-io#246, which this branch picked up via a rebase onto main. Detects smart-mode servers via loadpoints[].alwaysCharge, switching the selector to off/smart/now (with device-class labels for continuous heat pumps and switchable devices) while old servers keep off/pv/minpv/now unchanged. A read-only "∞" marks the Smart chip when Always charge is on/once, no toggle in the widget yet, matching iOS. Also fixes a bug the rebase's auto-merge introduced: the frozen pv/minpv legacy-label loop wrote straight into `strings`, a variable that in this branch is now built later from an intermediate `translations` map (added here to share resolved strings between the iOS and Android generators) - it was referencing `strings` before its declaration.
# Conflicts: # core/site_optimizer.go
|
Reviewed all remaining 1.
|
@pencoe "always change" is now an explicit option on vehicle level as well. Can be on/off/keep. Note: layout change extracted to #33169 
|
|
It's now ensured that existing loadpoints with no current control but minpv set cant get stuck in always-charge. |
|
@andig all review items plus some new findings/edge cases are addressed now. |
There was a problem hiding this comment.
Hey - I've found 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="core/loadpoint_vehicle.go" line_range="187-190" />
<code_context>
lp.SetMode(mode)
}
+ // vehicle always charge applies after mode, since deprecated pv/minpv modes reset it
+ if ac := vs.GetAlwaysCharge(); ac != "" {
+ if err := lp.SetAlwaysCharge(ac); err != nil {
+ lp.log.WARN.Printf("vehicle always charge: %v", err)
+ }
+ }
</code_context>
<issue_to_address>
**issue (broader_impact):** The YAML `onIdentify` action's Always charge setting is never applied when a vehicle is identified. This code only reads `vs.GetAlwaysCharge()` from persisted per-vehicle settings, so an `onIdentify` configuration requesting `on` or `off` is ignored.
**Triggers:** When a vehicle uses an `onIdentify` configuration with an Always charge value but has no persisted per-vehicle override.
**Suggested fix:** Read the Always charge value from `v.OnIdentified()` and apply it with the same precedence as the mode override, while letting an explicit persisted vehicle setting override the YAML value.
</issue_to_address>Sourcery assessment
Needs a human reviewer. 1 finding to address first, and the migration and new always-charge state can persistently force a loadpoint to draw minimum current, causing unwanted grid or home-battery consumption across sessions and vehicle changes. Reverting the code would not undo energy already consumed, although the persisted settings can be repaired.
Blocking findings: core/loadpoint_vehicle.go:190
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Findings from ponytail review: the route regex already restricts the value to on|off, and the nil settings guard in the legacy default mode seed had no reachable caller.
implements #3530, pairs with #33169
depends on #33169
Renames the charge modes around what they do instead of how they are powered. The
pvmode becomessmart. Theminpvmode is removed and its behavior (never pause, keep at least minimum power) moves into a new per-loadpoint "Always charge" option attached to the Smart mode. A growing share of users has no PV at all and uses evcc for dynamic tariffs and planning only; "Smart" covers all of that without presuming solar.Mode wording
"Smart" is the same wording for all device classes. Only the first and last mode adapt to the device:
API values are always
off,smart,nowregardless of device class. The labels are UI-only.♾️ Always charge
off,on,onceonbehaves exactly like the formerminpv: charge continuously at least at minimum current, no pauseonceis session-scoped: resets automatically on vehicle disconnect, not persisted across restartskeep as is,offoron, applied on identification right after the vehicle's default mode (vehicle settings modal, compact layout from Vehicle settings: compact rows layout #33169)🚨 Breaking change
/api/state, websocket, MQTT,GETresponses) now reportsmartinstead ofpv;minpvno longer appears anywhere on the read path{{.mode}}now receivesmartpv/minpvon the read side need updatingPOST /api/vehicles/{name}/mode/{pv|minpv}) maps both tosmartwithout touching always chargeminpvmaps tosmartwithout always charge on every path, including stored and yaml defaults. Such a device now follows surplus in smart mode instead of running permanently; usenowfor the old behaviorschema.jsononly listsoff,smart,now; legacy yaml values still work but get flagged by editorsdefaultMode(mode:in yaml, "Default mode" in the config UI):pv/minpvnormalize tosmart. Always charge is a persistent loadpoint setting and is no longer reset on disconnect; a legacyminpvdefault seeds it onceLegacy compatibility
All write paths keep accepting the old values and translate them:
pvmaps tosmartand turns Always charge offminpvmaps tosmartand turns Always charge onsmartleaves Always charge untouchedmode:loadpoint default, vehicleonIdentified) and vehicle default modespv/minpvkeep their exact former behaviorminpvbecomessmartwith Always charge on and is written backminpvbecomessmartwith per-vehicle Always chargeon,pvbecomessmartwithoff, so a second vehicle does not inherit the first one's settingFeature detection
External integrations (HA, ...) or clients (iOS App) can detect if an evcc instance already has the new "mode redesign" via presence of
loadpoints[].alwaysChargein/api/state. If it exists the new mode schema is used and wording (see table above) and API behavior (minpv>alwaysCharge) should be used.Endpoints
POST /api/loadpoints/{id}/alwayscharge/{off|on|once}(400 for devices without current control)loadpoints/{id}/alwaysChargealwaysChargeper loadpointPOST /api/vehicles/{name}/alwayscharge/{off|on},DELETE /api/vehicles/{name}/alwayschargevehicles[].alwaysChargePOST /api/loadpoints/{id}/mode/{mode}acceptsoff|smart|nowplus deprecatedpv|minpv, response echoes the normalized modeChargeMode/AlwaysChargeschemas, hand-writtenChargeModeInputdocuments the deprecated aliasesScreenshots
on/off switch
charger and heatpump (continuous)
dark mode
always active once
smart + always charge configured, fast mode selected
Takeaways
TODO