fix(ocpp): share lastPowerW accept rule with forecast - #1173
Conversation
Dispatch and forecast now accept the same Power.Active.Import sample: present, unphased or summed, finite, >= 0, and not older than the last accepted timestamp. 2.0.1 energy/current without power keeps last watts instead of writing 0. 1.6 Available/unplug zeros lastPowerW like 2.0.1. Closes #1156 Signed-off-by: Fredrik Ahlgren <fredrik@sourceful-labs.com>
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. |
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_06cd0f25-b276-442d-aa77-83fb08ae49d3) |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0ce94b09a1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // accepted timestamp. Samples from before this socket or in the future cannot | ||
| // revive a stale or synthesized reading. The caller holds h.mu. | ||
| func (s *chargerState) recordPower(w float64, measured, received time.Time) bool { | ||
| if math.IsNaN(w) || math.IsInf(w, 0) || w < 0 || measured.IsZero() || measured.After(received) || measured.Before(s.powerConnectedAt) || measured.UnixMilli() <= s.forecastPower.MeasuredAtMS { |
There was a problem hiding this comment.
When the charger clock is even slightly ahead of the CSMS clock, every otherwise-current sample satisfies measured.After(received) and is rejected; a clock behind the CSMS can likewise fail the connection-time check for an extended period. Because recordPower is now the sole writer of lastPowerW, this also suppresses dispatch telemetry and can let the home battery discharge while the EV is charging. Use a quantified skew tolerance rather than comparing the two machines' wall clocks with zero tolerance.
AGENTS.md reference: AGENTS.md:L37-L37
Useful? React with 👍 / 👎.
| case core.ChargePointStatusAvailable, core.ChargePointStatusUnavailable: | ||
| s.connected = false | ||
| s.charging = false | ||
| s.clearMeasuredPower() |
There was a problem hiding this comment.
When an OCPP 1.6 charge point has multiple connectors, an Available, Unavailable, or Faulted notification for one idle connector now clears the charger-wide lastPowerW, even if another connector is still drawing power. Since req.ConnectorId is ignored and state is shared by charge-point ID, this publishes zero and removes the EV-discharge safety clamp; track and aggregate connector state, or clear power only when no connector remains active.
Useful? React with 👍 / 👎.
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_120a76f4-07d6-47a1-bff0-3042621436d0) |
1 participant
Accepted text proposal
Issue or Discussion: #1156
Maintainer comment that accepted this scope: issue body on #1156.
What changed
OCPP
lastPowerW(dispatch DerEV) and forecast power now share one accept rule forPower.Active.Import:2.0.1 messages that carry energy/current but no power keep the last accepted watts instead of writing 0.
1.6 Available/Unavailable/Faulted and disconnect zero
lastPowerWthe same way 2.0.1 already did, without recording a measured zero.The test that required dispatch to keep a stale 9000 W after a newer 700 W sample now requires both paths to keep 700 W.
Why
Dispatch was last-sample-wins. A per-phase value, a negative import, an older timestamp, a 2.0.1 energy-only sample, or a 1.6 unplug could publish a phantom EV load and suppress home-battery discharge.
Boundaries and safety
Out of scope: allowlist, mTLS, DuckDB, #1133 (other DERs).
Stale or incomplete meter samples are ignored. Unplug/disconnect/stop still push 0 W for dispatch and mark forecast power unknown so a synthetic zero is not treated as a measurement. A failed or missing power measurand does not overwrite the last accepted watts.
Verification
Covers out-of-order 700 W vs 9000 W, 2.0.1 energy/current without power, 1.6 Available and disconnect, phase sum vs last-phase-wins, and negative import.
Checklist
Closes #1156
Note
Medium Risk
Changes how EV charging watts feed dispatch and forecast, which can affect home-battery discharge; behavior is narrowed with extensive tests but touches core energy telemetry paths.
Overview
OCPP dispatch (
lastPowerW/ DerEV) and forecast power now update through one sharedrecordPowerpath instead of last-sample-wins dispatch vs a separate forecast gate.Power.Active.Importis accepted only when it is finite, non‑negative, timestamp-valid (not future, not before connect, not older than the last accepted sample), and resolved viameterPowerW(unphased total wins; otherwise sum per-phase samples). Energy-only, negative, NaN, stale/out-of-order, or phase-only values that used to overwrite watts no longer publish phantom EV load.clearMeasuredPowerzeros dispatch watts and marks forecast power unknown on unplug-style events (1.6 Available/Unavailable/Faulted, disconnect, stop; 2.0.1 aligned) without treating that as a measured zero. OCPP 2.0.1 transaction/meter paths no longer infer 0 W when a batch has energy/current but no power.Tests cover shared rules for 1.6 and 2.0.1, including out-of-order 700 W vs 9000 W on both telemetry and dispatch.
Reviewed by Cursor Bugbot for commit 6b566dd. Bugbot is set up for automated code reviews on this repo. Configure here.