Skip to content

easee: warn on rogue CommandResponse not triggered by evcc#27916

Merged
andig merged 16 commits intoevcc-io:masterfrom
GrimmiMeloni:feat/easee-log-rogue-commandresponse
Mar 7, 2026
Merged

easee: warn on rogue CommandResponse not triggered by evcc#27916
andig merged 16 commits intoevcc-io:masterfrom
GrimmiMeloni:feat/easee-log-rogue-commandresponse

Conversation

@GrimmiMeloni
Copy link
Copy Markdown
Collaborator

@GrimmiMeloni GrimmiMeloni commented Mar 4, 2026

Summary

  • Replaces the shared unbuffered cmdC channel with a pendingTicks map[int64]chan SignalRCommandResponse — each outgoing API command registers its tick to a per-tick buffered channel
  • CommandResponse() is now the single routing point: known tick → delivers to channel; unknown tick → emits WARN log identifying the charger serial, tick value, and a note that another system may be controlling the charger
  • Helps users detect third-party apps (Easee app, another evcc instance, partner integrations) sending commands to their charger unnoticed

Test Plan

  • All existing TestEasee_* tests pass (go test ./charger/ -run TestEasee)
  • TestEasee_CommandResponse_rogue — verifies unknown tick logs WARN without panic
  • TestEasee_CommandResponse_legitimate — verifies known tick is delivered to the registered channel
  • Verify go vet ./charger/ is clean
  • Observe WARN log in a real evcc trace when Easee app sends a command in parallel

🤖 Generated with Claude Code

GrimmiMeloni and others added 5 commits March 4, 2026 07:29
…ves loop

- waitForTickResponse now accepts a <-chan parameter instead of reading c.cmdC
- postJSONAndWait creates and registers a per-tick channel via registerPendingTick/unregisterPendingTick
- CommandResponse dispatches to per-tick channels in pendingTicks map, warns on rogue responses
- Test updated: e.cmdC removed from newEasee helper and TestEasee_waitForTickResponse; TestEasee_postJsonAndWait goroutine now waits for pendingTicks registration

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@GrimmiMeloni GrimmiMeloni self-assigned this Mar 4, 2026
@GrimmiMeloni GrimmiMeloni added enhancement New feature or request devices Specific device support labels Mar 4, 2026
@GrimmiMeloni
Copy link
Copy Markdown
Collaborator Author

WIP - will test this on my local build first.

GrimmiMeloni and others added 10 commits March 4, 2026 22:09
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… Phases1p3p

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
start_charging causes the Easee API to return commandId:25 (LOCATION)
with ticks that never exactly match the charger's CommandResponse —
causing a false rogue warning and a waitForTickResponse timeout.

Add pendingByID as a fallback lookup in CommandResponse, checked after
ticks-match fails. postJSONAndWait now registers the same channel in
both pendingTicks (by ticks) and pendingByID (by ObservationID from
cmd.CommandId), so the LOCATION CommandResponse from start_charging is
correctly delivered to the waiting caller instead of being flagged rogue.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@GrimmiMeloni GrimmiMeloni added the ux User experience/ interface label Mar 6, 2026
If the circuit settings POST fails, the pre-registered orphan counter for
CIRCUIT_MAX_CURRENT_P1 was leaked, silently swallowing a future rogue
CommandResponse instead of logging it as a warning.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@GrimmiMeloni GrimmiMeloni requested a review from andig March 7, 2026 00:00
@GrimmiMeloni
Copy link
Copy Markdown
Collaborator Author

tested locally, tweaked some minor things, looking good on my end.

@andig andig marked this pull request as ready for review March 7, 2026 09:03
@andig andig merged commit 16d1258 into evcc-io:master Mar 7, 2026
13 checks passed
Copy link
Copy Markdown
Contributor

@sourcery-ai sourcery-ai Bot left a comment

Choose a reason for hiding this comment

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

Hey - I've found 2 issues

Prompt for AI Agents
Please address the comments from this code review:

## Individual Comments

### Comment 1
<location path="charger/easee.go" line_range="443-444" />
<code_context>
+	chID, idOk := c.pendingByID[obsID]
+	c.cmdMu.Unlock()
+
+	if tickOk {
+		chTick <- res
+		return
+	}
</code_context>
<issue_to_address>
**issue:** Using a blocking send to the tick channel may deadlock if multiple responses are received for the same tick.

When `tickOk` is true, this does an unguarded `chTick <- res`. Since `chTick` has a buffer of 1 in `postJSONAndWait`, a second `CommandResponse` for the same `Ticks` value would block indefinitely and stall the SignalR handler. Consider a non-blocking send (e.g. `select { case chTick <- res: default: /* log/drop duplicate */ }`) or otherwise handling unexpected duplicates explicitly instead of risking a blocking send.
</issue_to_address>

### Comment 2
<location path="charger/easee_test.go" line_range="228-237" />
<code_context>

 		if tc.cmdResp != nil {
 			go func() {
-				e.cmdC <- *tc.cmdResp
+				// wait for postJSONAndWait to register the per-tick channel
+				var ch chan easee.SignalRCommandResponse
+				for {
+					e.cmdMu.Lock()
+					ch = e.pendingTicks[tc.cmdResp.Ticks]
+					e.cmdMu.Unlock()
+					if ch != nil {
+						break
+					}
+					time.Sleep(time.Millisecond)
+				}
+				ch <- *tc.cmdResp
</code_context>
<issue_to_address>
**issue (testing):** Add a timeout in the polling loop to avoid a potentially hanging test

The spin loop that waits for `pendingTicks[tc.cmdResp.Ticks]` has no upper bound, so if registration regresses or the tick value changes, this test can busy-wait indefinitely and hang the suite. Please add a timeout (e.g., `time.After` in a `select`) and fail the test if the channel is not found in time, so issues show up as test failures rather than stuck runs.
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Comment thread charger/easee.go
Comment on lines +443 to +444
if tickOk {
chTick <- res
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.

issue: Using a blocking send to the tick channel may deadlock if multiple responses are received for the same tick.

When tickOk is true, this does an unguarded chTick <- res. Since chTick has a buffer of 1 in postJSONAndWait, a second CommandResponse for the same Ticks value would block indefinitely and stall the SignalR handler. Consider a non-blocking send (e.g. select { case chTick <- res: default: /* log/drop duplicate */ }) or otherwise handling unexpected duplicates explicitly instead of risking a blocking send.

Comment thread charger/easee_test.go
Comment on lines 228 to +237
if tc.cmdResp != nil {
go func() {
e.cmdC <- *tc.cmdResp
// wait for postJSONAndWait to register the per-tick channel
var ch chan easee.SignalRCommandResponse
for {
e.cmdMu.Lock()
ch = e.pendingTicks[tc.cmdResp.Ticks]
e.cmdMu.Unlock()
if ch != nil {
break
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.

issue (testing): Add a timeout in the polling loop to avoid a potentially hanging test

The spin loop that waits for pendingTicks[tc.cmdResp.Ticks] has no upper bound, so if registration regresses or the tick value changes, this test can busy-wait indefinitely and hang the suite. Please add a timeout (e.g., time.After in a select) and fail the test if the channel is not found in time, so issues show up as test failures rather than stuck runs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

devices Specific device support enhancement New feature or request ux User experience/ interface

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants