EEBus meter: add MPC/LPC use cases #24082
Conversation
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey there - I've reviewed your changes and they look great!
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location> `meter/eebus_test.go:10` </location>
<code_context>
+ "github.com/evcc-io/evcc/util/test"
+)
+
+func TestEEBus(t *testing.T) {
+ acceptable := []string{
+ "eebus not configured",
</code_context>
<issue_to_address>
**suggestion (testing):** Test only covers the 'not configured' error case.
Please add tests for successful initialization and for both MGCP and MPC usage types to verify correct meter setup and dynamic use case selection.
Suggested implementation:
```golang
func TestEEBus(t *testing.T) {
acceptable := []string{
"eebus not configured",
}
// Test not configured error case
values := map[string]any{
"ski": "test-ski",
"ip": "192.0.2.2",
"usage": "grid",
"timeout": "10s",
}
if _, err := NewFromConfig(context.TODO(), "eebus", values); err != nil && !test.Acceptable(err, acceptable) {
t.Errorf("unexpected error: %v", err)
}
// Test successful initialization (simulate with minimal valid config)
successValues := map[string]any{
"ski": "valid-ski",
"ip": "192.0.2.3",
"usage": "grid",
"timeout": "10s",
}
meter, err := NewFromConfig(context.TODO(), "eebus", successValues)
if err != nil {
t.Errorf("expected successful initialization, got error: %v", err)
}
if meter == nil {
t.Error("expected meter instance, got nil")
}
// Test MGCP usage type
mgcpValues := map[string]any{
"ski": "mgcp-ski",
"ip": "192.0.2.4",
"usage": "mgcp",
"timeout": "10s",
}
mgcpMeter, err := NewFromConfig(context.TODO(), "eebus", mgcpValues)
if err != nil {
t.Errorf("MGCP usage: expected successful initialization, got error: %v", err)
}
if mgcpMeter == nil {
t.Error("MGCP usage: expected meter instance, got nil")
}
// Optionally, check for MGCP-specific behavior if available
// e.g. assert MGCP use case selection
// Test MPC usage type
mpcValues := map[string]any{
"ski": "mpc-ski",
"ip": "192.0.2.5",
"usage": "mpc",
"timeout": "10s",
}
mpcMeter, err := NewFromConfig(context.TODO(), "eebus", mpcValues)
if err != nil {
t.Errorf("MPC usage: expected successful initialization, got error: %v", err)
}
if mpcMeter == nil {
t.Error("MPC usage: expected meter instance, got nil")
}
// Optionally, check for MPC-specific behavior if available
// e.g. assert MPC use case selection
}
```
If the `NewFromConfig` function requires a real EEBus backend or network connection, you may need to mock or stub out the relevant parts for these tests to work reliably in CI. Also, if there are specific methods or fields to check for MGCP/MPC use case selection, add assertions for those.
</issue_to_address>
### Comment 2
<location> `meter/eebus.go:97` </location>
<code_context>
// UseCaseEvent implements the eebus.Device interface
func (c *EEBus) UseCaseEvent(_ spineapi.DeviceRemoteInterface, entity spineapi.EntityRemoteInterface, event eebusapi.EventType) {
- switch event {
- case mgcp.DataUpdatePower:
</code_context>
<issue_to_address>
**issue (complexity):** Consider refactoring MGCP/MPC selection into a provider interface to eliminate repeated if/else logic.
Here’s one way to collapse all the `if c.useMGCP { … } else { … }` logic into a single strategy-style provider. You end up with one unified switch in `UseCaseEvent` and four tiny `dataUpdateX` methods:
1) Define a small interface and add it to your struct:
```go
// at top of file
type dataUseCase interface {
Power(spineapi.EntityRemoteInterface) (float64, error)
EnergyConsumed(spineapi.EntityRemoteInterface) (float64, error)
CurrentPerPhase(spineapi.EntityRemoteInterface) ([]float64, error)
VoltagePerPhase(spineapi.EntityRemoteInterface) ([]float64, error)
}
type EEBus struct {
// ...
provider dataUseCase
// remove: useMGCP
}
```
2) In your constructor, wire up the right implementation once:
```go
func NewEEBus(… usage templates.Usage, …) (*EEBus, error) {
c := &EEBus{ /* … */ }
if usage == templates.UsageGrid {
c.provider = c.uc.MGCP
} else {
c.provider = c.uc.MPC
}
// …
return c, nil
}
```
3) Collapse `UseCaseEvent` into one switch with both event-IDs:
```go
func (c *EEBus) UseCaseEvent(_ spineapi.DeviceRemoteInterface, ent spineapi.EntityRemoteInterface, event eebusapi.EventType) {
switch event {
case mgcp.DataUpdatePower, mpc.DataUpdatePower:
c.dataUpdatePower(ent)
case mgcp.DataUpdateEnergyConsumed, mpc.DataUpdateEnergyConsumed:
c.dataUpdateEnergyConsumed(ent)
case mgcp.DataUpdateCurrentPerPhase, mpc.DataUpdateCurrentsPerPhase:
c.dataUpdateCurrentPerPhase(ent)
case mgcp.DataUpdateVoltagePerPhase, mpc.DataUpdateVoltagePerPhase:
c.dataUpdateVoltagePerPhase(ent)
}
}
```
4) Simplify each `dataUpdateX`:
```go
func (c *EEBus) dataUpdatePower(ent spineapi.EntityRemoteInterface) {
v, err := c.provider.Power(ent)
if err != nil {
c.log.ERROR.Println("Power:", err)
return
}
c.power.Set(v)
}
// …and similarly for EnergyConsumed, CurrentPerPhase, VoltagePerPhase…
```
This removes all of the nested `if/else` blocks while keeping MGCP vs MPC selection at construction time only.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
Das muss jemand mit Gegenstelle usprobiefen |
|
Das ging aber schnell... Wie kann ich da die Limitierung triggern? Muss ich da evcc noch mit dem controlbox simulator koppeln und darüber triggern? [EDIT] [EDIT2] |
|
Im ersten Schritt wäre es schon toll erstmal das als ext-Meter einzurichten und mit der WP zu koppeln.
Wenn du einen "EEBus-Netzzähler" hast oder diesen simulieren kannst könntest du ein weiteres Meter damit koppeln und dabei |
|
OK. Ich setze erstmal alles auf dem Raspi4 auf und baue da dann deinen Branch. Wollte ich eh schon lange machen, da ich dann evtl. aktiver mithelfen kann. Auch wenn Go aktuell noch eine Fremdsprache für mich ist. Falls das OK ist, würde ich bei Probleme ggfs. auf Slack auf dich zukommen. |
Ja, im Prinzip so. Ich bin mir aber gerade noch nicht sicher was noch innerhalb evcc ggf. fehlt damit wir das dann auch letztendlich am gekoppelten Verbraucher sehen. Hier das ist ja quasi nur der Adapter. @andig |
|
Kurzes Status-Update... nachdem ich mit den apt Packages grandios gescheitert bin, habe ich die korrekten Paket-Versionen jetzt installiert und konnte den Branch von @premultiply erfolgreich bauen und ausführen. Als nächstes geht es ans Testen des EEBUS Meters. Ich versuche immer mal wieder ein bisschen Zeit zwischen dem Familienleben zu investieren. Sonst spätestens Montag, wo ich den ganzen Tag Zeit dafür habe... |
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as resolved.
This comment was marked as resolved.
|
Hab es rausgelöscht... Assozierung in der myVAILLANT App durchgeführt: Log: Die Timeouts kommen in sehr schneller Abfolge. |
|
Das Ding ist, dass Ich teste das gerade parallel mit dem Sunny Home Manger der offensichtlich MGCP unterstützt. |
|
Müsste ich den MCP dann irgendwann im Log vorbeifliegen sehen? Kommt aktuell nichts... Oder muss ich dann einen Loadpoint mit dem Meter anlegen und im UI schauen? Nutze immer noch obige Minimal-Konfig. Produktivsystem läuft parallel auf dem anderen Raspi (EEBUS ist dort deaktiviert). |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
ping @CiNcH83 |
|
|
Jetzt, wo ich den Sobald ich das |
|
@andig Rein? |
|
@naltatis wollen wir eine |
|
Zwei Templates, eins fest mit usage: grid und eins ganz ohne würde auch funktionieren. |
| value = limit | ||
| } | ||
|
|
||
| return c.uc.LPC.SetConsumptionLimit(ucapi.LoadLimit{ |
There was a problem hiding this comment.
Achso: das müssen wir vor Merge immer noch klären! Im Zweifel erstmal ohne LPC.
There was a problem hiding this comment.
Das hier ist noch offen- würde ich in den Dim PR verschieben.
|
Ich habe den Pull Request zufällig gefunden. Es scheint genau das zu sein was ich suche (Vaillant aroTHERM Plus, Fronius SYMO Gen24 WR und BYD Akku) um überschuss limitiert in den Heizungspuffer zu bekommen. Ich hatte evcc schon aufgesetzt mit PV und Akku über den Wechselrichter und Wärmepumpe über die SensoNET API. Ich würde aber lieber über den PV-Ready Kontakt gehen und die Leistung auf den Überschuss limitieren. Ich habe den branch bereits ausgecheckt und kompiliert. Habe aber nicht wirklich verstanden wie ich jetzt weiter vorgehen muss. Den ControlBox Simulator hatte ich schon erfolgreich getestet. Da ich kein evcc Produktivsystem habe könnte ich also problemlos testen wenn mir jemand sagen würde wie ich es einrichten muss. |
Hier gibt es eine recht gute Anleitung, was zu tun ist. Der eegbus-go Fork von @meisel2000 ist etwas älter und kann noch kein MPC. Der Fork von @vollautomat kann MPC. Da sind die Aufrufparameter dann etwas anders als in der verlinkten Anleitung. Man sollte sich für eine Variante entscheiden und dann dabei bleiben. Die Variante von @vollautomat verwendet eine leicht geänderte Geräte-Kennung und hat daher eine andere SHIPID/SKI, was diverse EEBUS Geräte im Netz (wie die Vaillant aroTHERM) verwirrt. |
5 participants
Adds EEBus meter support
Features
Use Case Support:
usage: grid)api.DimmerCapabilities:
Implementation
Configuration Example
Output Example (SMA Sunny Home Manager 2.0 - MGCP)
Closes #24075
/cc @DerAndereAndi @CiNcH83