Skip to content

HEMS: add per-hems tests for nil statements when unconfigured or not connected#31430

Draft
andig wants to merge 1 commit into
chore/curtail-dimm-apifrom
test/hems-nil-statements
Draft

HEMS: add per-hems tests for nil statements when unconfigured or not connected#31430
andig wants to merge 1 commit into
chore/curtail-dimm-apifrom
test/hems-nil-statements

Conversation

@andig

@andig andig commented Jul 2, 2026

Copy link
Copy Markdown
Member

Adds unit tests per HEMS asserting that Curtailed()/Dimmed() and the underlying power functions (CurtailedPercent, MaxConsumptionPower, MaxProductionPower) return nil when curtailment or dimming are not configured (FNN, Relay) or when no upstream controlbox has connected (EEBus).

  • FNN: without W3 no curtailment statement, without W4 no dimming statement — passes
  • Relay: never makes a curtailment statement, no dimming statement before the relay state is known — passes
  • EEBus: TestNotConnected fails intentionally — MaxConsumptionPower, CurtailedPercent and hence Dimmed()/Curtailed() report values even though no controlbox ever connected. Only MaxProductionPower already returns nil. Tests expose the bug; no fix included per request.

Based on #31428.

🤖 Generated with Claude Code

@andig andig added the enhancement New feature or request label Jul 2, 2026

@sourcery-ai sourcery-ai Bot left a comment

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.

Hey - I've left some high level feedback:

  • In relay_test.go, assert.Equal(t, new(1e3), c.MaxConsumptionPower()) will not compile because new expects a type, not a value; consider using a small helper like ptr(1000.0) or taking the address of a typed literal instead.
  • Since TestNotConnected in eebus_test.go is intentionally failing to highlight a bug, consider marking it with t.Skip and a clear TODO or gating it behind a build tag so that the main test suite can still pass until the underlying issue is fixed.
  • The boolG helper in fnn_test.go is a useful pattern; if similar boolean getters are needed in other HEMS tests, consider consolidating it in a shared test utility to avoid duplication.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- In `relay_test.go`, `assert.Equal(t, new(1e3), c.MaxConsumptionPower())` will not compile because `new` expects a type, not a value; consider using a small helper like `ptr(1000.0)` or taking the address of a typed literal instead.
- Since `TestNotConnected` in `eebus_test.go` is intentionally failing to highlight a bug, consider marking it with `t.Skip` and a clear TODO or gating it behind a build tag so that the main test suite can still pass until the underlying issue is fixed.
- The `boolG` helper in `fnn_test.go` is a useful pattern; if similar boolean getters are needed in other HEMS tests, consider consolidating it in a shared test utility to avoid duplication.

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.

@andig

andig commented Jul 2, 2026

Copy link
Copy Markdown
Member Author

EEBus: TestNotConnected fails intentionally — MaxConsumptionPower, CurtailedPercent and hence Dimmed()/Curtailed() report values even though no controlbox ever connected. Only MaxProductionPower already returns nil. Tests expose the bug; no fix included per request.

@premultiply was würden wir hier erwarten? Genau dafür gibts ja egtl. die Failsafe Values?

@andig andig marked this pull request as draft July 2, 2026 19:48
@premultiply

Copy link
Copy Markdown
Member

In dem Fall würde Passivität herrschen (dauerhaft nicht gedimmt) bzw. eine statische Vorgabe des Anlagenbetreibers wäre umzusetzen (z.B dauerhaft 100, 70, 60, 50 oder 0%. In anderen Ländern durchaus auch andere / freie Werte, häufig dann auch als Leistungswert mit dem Netzbetreiber vereinbart).

Fallback ist etwas anderes: Das gilt erst wenn diese Daten mindestens einmal von der Steuerbox auch tatsächlich übermittelt wurden und dann die Verbindung zu dieser bzw. zum Netzbetreiber temporär abbricht.

@github-actions github-actions Bot added the stale Outdated and ready to close label Jul 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request stale Outdated and ready to close

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants