From b8130ea48d2022e1c15ce965fc95f46a9ffc22e8 Mon Sep 17 00:00:00 2001 From: andig Date: Sat, 8 Aug 2026 14:56:43 +0200 Subject: [PATCH] Shelly: fix PV energy for devices with reverse power measurement enabled (#32550) --- meter/shelly.go | 25 ++++-- meter/shelly/connection.go | 2 + meter/shelly/connection_test.go | 140 ++++++++++++++++++++++++++++++++ meter/shelly/gen1.go | 10 +++ meter/shelly/gen2.go | 68 ++++++++++++++-- meter/shelly/gen2_test.go | 86 ++++++++++++++++++++ meter/shelly_test.go | 15 ++-- 7 files changed, 326 insertions(+), 20 deletions(-) create mode 100644 meter/shelly/connection_test.go diff --git a/meter/shelly.go b/meter/shelly.go index fe54f2cca..4801e80c1 100644 --- a/meter/shelly.go +++ b/meter/shelly.go @@ -49,12 +49,17 @@ func NewShellyFromConfig(other map[string]any) (api.Meter, error) { // making their totals unsuitable for bidirectional grid metering. if !(c.usage == "grid" && c.conn.IsThreePhase()) { total, ret := c.conn.TotalEnergy, c.conn.ReturnEnergy - if c.usage == "pv" { - // reverse direction + // production is measured in return direction, unless the device has no return + // register at all or already reverses the direction itself + if c.usage == "pv" && c.conn.HasReturnEnergy() && !c.conn.IsReversed() { total, ret = ret, total } implement.Has(c, implement.MeterEnergy(total)) - implement.Has(c, implement.MeterReturnEnergy(ret)) + + // without a return register the second reading is a constant zero + if c.conn.HasReturnEnergy() { + implement.Has(c, implement.MeterReturnEnergy(ret)) + } } if phases, ok := c.conn.Generation.(shelly.Phases); ok { @@ -88,16 +93,20 @@ func (c *Shelly) CurrentPower() (float64, error) { if err != nil { return 0, err } - return c.currentPowerForUsage(power, c.conn.SignedPower()), nil + return c.currentPowerForUsage(power, c.conn.SignedPower(), c.conn.IsReversed()), nil } -// PV usage inverts directional power, otherwise the magnitude is used. -func (c *Shelly) currentPowerForUsage(power float64, signed bool) float64 { +// PV usage inverts directional power unless the device already reverses it, otherwise the magnitude is used. +func (c *Shelly) currentPowerForUsage(power float64, signed, reversed bool) float64 { if c.usage != "pv" { return power } - if signed { + switch { + case !signed: + return math.Abs(power) + case reversed: + return power + default: return -power } - return math.Abs(power) } diff --git a/meter/shelly/connection.go b/meter/shelly/connection.go index 46150008a..c584b9127 100644 --- a/meter/shelly/connection.go +++ b/meter/shelly/connection.go @@ -19,6 +19,8 @@ type Generation interface { api.MeterEnergy api.MeterReturnEnergy IsThreePhase() bool + IsReversed() bool + HasReturnEnergy() bool } type Phases interface { diff --git a/meter/shelly/connection_test.go b/meter/shelly/connection_test.go new file mode 100644 index 000000000..7185c9280 --- /dev/null +++ b/meter/shelly/connection_test.go @@ -0,0 +1,140 @@ +package shelly + +import ( + "encoding/json" + "net/http" + "net/http/httptest" + "strings" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// shellyServer emulates a gen2+ device serving the given rpc responses. Methods +// not listed are reported as unavailable by Shelly.ListMethods and answered 404. +func shellyServer(t *testing.T, rpc map[string]string) *httptest.Server { + t.Helper() + + methods := make([]string, 0, len(rpc)) + for m := range rpc { + methods = append(methods, m) + } + + return httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + + switch r.URL.Path { + case "/shelly": + json.NewEncoder(w).Encode(map[string]any{"gen": 2, "model": "SNSW-001X16EU"}) + + case "/rpc/Shelly.ListMethods": + json.NewEncoder(w).Encode(Gen2Methods{Methods: methods}) + + default: + method := strings.TrimPrefix(r.URL.Path, "/rpc/") + res, ok := rpc[method] + if !ok { + http.Error(w, `{"code":-105,"message":"no handler"}`, http.StatusNotFound) + return + } + w.Write([]byte(res)) + } + })) +} + +// TestNakedSwitchConnection asserts that a switch without power measurement +// (Shelly Plus 1) connects - its status has neither aenergy nor ret_aenergy. +func TestNakedSwitchConnection(t *testing.T) { + srv := shellyServer(t, map[string]string{ + "Switch.GetStatus": `{"id":0,"source":"init","output":true,"temperature":{"tC":45.2,"tF":113.4}}`, + "Switch.GetConfig": `{"id":0,"name":null,"in_mode":"follow","initial_state":"match_input","auto_on":false}`, + }) + defer srv.Close() + + conn, err := NewConnection(srv.URL, "", "", 0, time.Second) + require.NoError(t, err, "naked switch must connect") + + assert.False(t, conn.IsReversed()) + assert.False(t, conn.HasReturnEnergy()) + + enabled, err := conn.Enabled() + require.NoError(t, err) + assert.True(t, enabled) + + total, err := conn.TotalEnergy() + require.NoError(t, err) + assert.Zero(t, total) + + ret, err := conn.ReturnEnergy() + require.NoError(t, err) + assert.Zero(t, ret) +} + +// TestSwitchConnectionStatusError asserts that a temporarily unavailable switch +// status does not break connecting- the error surfaces on read instead. +func TestSwitchConnectionStatusError(t *testing.T) { + // Switch.GetStatus advertised but erroring, e.g. component busy right after boot + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/shelly": + json.NewEncoder(w).Encode(map[string]any{"gen": 2, "model": "SNSW-001X16EU"}) + case "/rpc/Shelly.ListMethods": + json.NewEncoder(w).Encode(Gen2Methods{Methods: []string{"Switch.GetStatus", "Switch.GetConfig"}}) + case "/rpc/Switch.GetConfig": + w.Write([]byte(`{"id":0}`)) + default: + http.Error(w, `{"code":-114,"message":"component not found"}`, http.StatusNotFound) + } + })) + defer srv.Close() + + conn, err := NewConnection(srv.URL, "", "", 0, time.Second) + require.NoError(t, err, "status error must not break connecting") + + _, err = conn.CurrentPower() + require.Error(t, err, "error surfaces on read") +} + +// TestPlugConnection covers the metered plug from #32213: aenergy without ret_aenergy. +func TestPlugConnection(t *testing.T) { + srv := shellyServer(t, map[string]string{ + "Switch.GetStatus": `{"id":0,"output":true,"apower":399.2,"voltage":240.8,"current":1.886,"aenergy":{"total":2574466.629}}`, + "Switch.GetConfig": `{"id":0,"power_limit":2800,"voltage_limit":280}`, + }) + defer srv.Close() + + conn, err := NewConnection(srv.URL, "", "", 0, time.Second) + require.NoError(t, err) + + assert.False(t, conn.IsReversed()) + assert.False(t, conn.HasReturnEnergy(), "plug has no return register") + + total, err := conn.TotalEnergy() + require.NoError(t, err) + assert.Equal(t, 2574.466629, total) +} + +// TestReversedSwitchConnection covers a switch with device-side reverse measurement. +func TestReversedSwitchConnection(t *testing.T) { + srv := shellyServer(t, map[string]string{ + "Switch.GetStatus": `{"id":0,"output":true,"apower":-350,"aenergy":{"total":10000},"ret_aenergy":{"total":4000}}`, + "Switch.GetConfig": `{"id":0,"reverse":true}`, + }) + defer srv.Close() + + conn, err := NewConnection(srv.URL, "", "", 0, time.Second) + require.NoError(t, err) + + assert.True(t, conn.IsReversed()) + assert.True(t, conn.HasReturnEnergy()) + + total, err := conn.TotalEnergy() + require.NoError(t, err) + assert.Equal(t, 6.0, total) + + ret, err := conn.ReturnEnergy() + require.NoError(t, err) + assert.Equal(t, 4.0, ret) +} diff --git a/meter/shelly/gen1.go b/meter/shelly/gen1.go index 65175a15e..a8e007ef5 100644 --- a/meter/shelly/gen1.go +++ b/meter/shelly/gen1.go @@ -144,6 +144,16 @@ func (c *gen1) ReturnEnergy() (float64, error) { return c.energy(energy) / 1000, nil } +// IsReversed reports whether the device reverses its measurement direction. Gen1 devices cannot. +func (c *gen1) IsReversed() bool { + return false +} + +// HasReturnEnergy reports whether the device measures energy in the return direction +func (c *gen1) HasReturnEnergy() bool { + return true +} + // IsThreePhase reports whether the device is a three-phase energy meter. func (c *gen1) IsThreePhase() bool { res, err := c.status.Get() diff --git a/meter/shelly/gen2.go b/meter/shelly/gen2.go index e98841cfc..a34cc6822 100644 --- a/meter/shelly/gen2.go +++ b/meter/shelly/gen2.go @@ -28,6 +28,10 @@ type Gen2Methods struct { Methods []string } +type Gen2Config struct { + Reverse bool +} + type Gen2SwitchStatus struct { Output bool Apower float64 @@ -36,11 +40,23 @@ type Gen2SwitchStatus struct { Aenergy struct { Total float64 } - Ret_Aenergy struct { + // nil on devices without reverse energy metering- they omit the register entirely + Ret_Aenergy *struct { Total float64 } } +// switchEnergy splits the switch registers into import and return energy (kWh). +// https://shelly-api-docs.shelly.cloud/gen2/ComponentsAndServices/Switch#status +// NOTE: energy added to ret_aenergy is also added to aenergy, so aenergy holds +// both directions. Without the register aenergy is import only. +func switchEnergy(res Gen2SwitchStatus) (total, ret float64) { + if res.Ret_Aenergy == nil { + return res.Aenergy.Total / 1000, 0 + } + return max(0, res.Aenergy.Total-res.Ret_Aenergy.Total) / 1000, res.Ret_Aenergy.Total / 1000 +} + type Gen2EMStatus struct { TotalActPower float64 `json:"total_act_power"` ACurrent float64 `json:"a_current"` @@ -84,6 +100,7 @@ type gen2 struct { switchchannel int model string methods []string + reversed bool switchstatus util.Cacheable[Gen2SwitchStatus] em1status func() (Gen2EM1Status, error) em1data func() (Gen2EM1Data, error) @@ -139,6 +156,27 @@ func newGen2(helper *request.Helper, uri, model string, channel int, user, passw } else { c.switchstatus = util.ResettableCached(apiCall[Gen2SwitchStatus](c, c.switchchannel, "Switch.GetStatus"), cache) } + // device-side "Reverse power measurement" setting (requires restart, hence static) + var cfgMethod string + cfgChannel := channel + switch { + case c.hasEM1Endpoint(): + cfgMethod = "EM1.GetConfig" + case c.hasMethod("PM1.GetStatus"): + cfgMethod = "PM1.GetConfig" + case c.hasMethod("Switch.GetStatus"): + cfgMethod = "Switch.GetConfig" + cfgChannel = c.switchchannel + } + + if c.hasMethod(cfgMethod) { + var cfg Gen2Config + if err := c.execCmd(cfgChannel, cfgMethod, &cfg); err != nil { + return nil, err + } + c.reversed = cfg.Reverse + } + c.em1status = util.Cached(apiCall[Gen2EM1Status](c, channel, "EM1.GetStatus"), cache) c.em1data = util.Cached(apiCall[Gen2EM1Data](c, channel, "EM1Data.GetStatus"), cache) c.emstatus = util.Cached(apiCall[Gen2EMStatus](c, channel, "EM.GetStatus"), cache) @@ -232,10 +270,8 @@ func (c *gen2) TotalEnergy() (float64, error) { case c.hasSwitchEndpoint(): res, err := c.switchstatus.Get() - // https://shelly-api-docs.shelly.cloud/gen2/ComponentsAndServices/Switch#status - // NOTE: ret_aenergy - the active energy added to this container is also added to aenergy container. - // All the consumed energy is collected in aenergy regardless of the direction(consumed or returned) of the active energy. - return max(0, res.Aenergy.Total-res.Ret_Aenergy.Total) / 1000, err + total, _ := switchEnergy(res) + return total, err default: return 0, fmt.Errorf("unknown shelly model: %s", c.model) @@ -255,7 +291,8 @@ func (c *gen2) ReturnEnergy() (float64, error) { case c.hasSwitchEndpoint(): res, err := c.switchstatus.Get() - return res.Ret_Aenergy.Total / 1000, err + _, ret := switchEnergy(res) + return ret, err default: return 0, fmt.Errorf("unknown shelly model: %s", c.model) @@ -340,6 +377,25 @@ func (c *gen2) IsThreePhase() bool { return c.hasEMEndpoint() } +// IsReversed reports whether the device-side "Reverse power measurement" setting is enabled +func (c *gen2) IsReversed() bool { + return c.reversed +} + +// HasReturnEnergy reports whether the device measures energy in the return direction. +// Plain plugs omit ret_aenergy entirely, so the (cached) status decides. +func (c *gen2) HasReturnEnergy() bool { + switch { + case c.hasEM1Endpoint(), c.hasEMEndpoint(): + return true + case c.hasSwitchEndpoint(): + res, err := c.switchstatus.Get() + return err == nil && res.Ret_Aenergy != nil + default: + return false + } +} + // Gen2+ models using EM1.GetStatus endpoint for power and EM1Data.GetStatus for energy // https://shelly-api-docs.shelly.cloud/gen2/ComponentsAndServices/EM1#em1getstatus-example // https://shelly-api-docs.shelly.cloud/gen2/ComponentsAndServices/EM1Data#em1datagetstatus-example diff --git a/meter/shelly/gen2_test.go b/meter/shelly/gen2_test.go index b0e577486..fa2995522 100644 --- a/meter/shelly/gen2_test.go +++ b/meter/shelly/gen2_test.go @@ -2,8 +2,11 @@ package shelly import ( "encoding/json" + "errors" "testing" + "time" + "github.com/evcc-io/evcc/util" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -80,3 +83,86 @@ func TestUnmarshalGen2StatusResponse(t *testing.T) { assert.Equal(t, 0, parseAddOnSwitchID(channel, res)) } } + +func TestSwitchEnergy(t *testing.T) { + tests := []struct { + name string + status string + total, ret float64 + hasReturnReg bool + }{ + { + // Shelly Plug S Gen3 as pv meter, https://github.com/evcc-io/evcc/issues/32213 + // no ret_aenergy register: aenergy is production, swapping it would report 0 + name: "plug without reverse metering", + status: `{"id":0,"source":"init","output":true,"apower":399.2,"voltage":240.8,"current":1.886,"aenergy":{"total":2574466.629,"by_minute":[7200.069,6121.044,5088.795],"minute_ts":1786033200},"temperature":{"tC":46.3,"tF":115.4}}`, + total: 2574.466629, + }, + { + // aenergy holds both directions, so import is the difference + name: "switch with reverse metering", + status: `{"id":0,"output":true,"apower":-350,"aenergy":{"total":10000},"ret_aenergy":{"total":4000}}`, + total: 6, + ret: 4, + hasReturnReg: true, + }, + { + // pure production: everything lands in ret_aenergy, import must not go negative + name: "switch measuring return only", + status: `{"id":0,"output":true,"aenergy":{"total":4000},"ret_aenergy":{"total":4000}}`, + ret: 4, + hasReturnReg: true, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + var res Gen2SwitchStatus + require.NoError(t, json.Unmarshal([]byte(tc.status), &res)) + + assert.Equal(t, tc.hasReturnReg, res.Ret_Aenergy != nil, "ret_aenergy presence") + + total, ret := switchEnergy(res) + assert.Equal(t, tc.total, total, "total energy") + assert.Equal(t, tc.ret, ret, "return energy") + + // same values through the endpoint dispatch + c := &gen2{ + methods: []string{"Switch.GetStatus"}, + switchstatus: util.ResettableCached(func() (Gen2SwitchStatus, error) { + return res, nil + }, time.Minute), + } + + assert.Equal(t, tc.hasReturnReg, c.HasReturnEnergy(), "HasReturnEnergy") + + totalEnergy, err := c.TotalEnergy() + require.NoError(t, err) + assert.Equal(t, tc.total, totalEnergy, "TotalEnergy") + + returnEnergy, err := c.ReturnEnergy() + require.NoError(t, err) + assert.Equal(t, tc.ret, returnEnergy, "ReturnEnergy") + }) + } +} + +// a failed read yields the zero status, whose ret_aenergy register is nil too +func TestSwitchEnergyReadError(t *testing.T) { + c := &gen2{ + methods: []string{"Switch.GetStatus"}, + switchstatus: util.ResettableCached(func() (Gen2SwitchStatus, error) { + return Gen2SwitchStatus{}, errors.New("offline") + }, time.Minute), + } + + require.NotPanics(t, func() { + total, err := c.TotalEnergy() + require.Error(t, err) + assert.Zero(t, total) + + ret, err := c.ReturnEnergy() + require.Error(t, err) + assert.Zero(t, ret) + }) +} diff --git a/meter/shelly_test.go b/meter/shelly_test.go index 7ebf35cd4..a23bfb8cd 100644 --- a/meter/shelly_test.go +++ b/meter/shelly_test.go @@ -8,23 +8,26 @@ import ( func TestShellyCurrentPowerForUsage(t *testing.T) { tests := []struct { - name string - usage string - signed bool - power float64 - want float64 + name string + usage string + signed bool + reversed bool + power float64 + want float64 }{ {name: "grid keeps sign", usage: "grid", power: -350, want: -350}, {name: "unsigned pv uses absolute value", usage: "pv", power: -350, want: 350}, {name: "unsigned pv keeps positive values", usage: "pv", power: 350, want: 350}, {name: "signed pv inverts positive values", usage: "pv", signed: true, power: 350, want: -350}, {name: "signed pv inverts negative values", usage: "pv", signed: true, power: -350, want: 350}, + {name: "signed reversed pv keeps sign", usage: "pv", signed: true, reversed: true, power: 350, want: 350}, + {name: "signed reversed pv keeps negative sign", usage: "pv", signed: true, reversed: true, power: -350, want: -350}, } for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { m := &Shelly{usage: tc.usage} - assert.Equal(t, tc.want, m.currentPowerForUsage(tc.power, tc.signed)) + assert.Equal(t, tc.want, m.currentPowerForUsage(tc.power, tc.signed, tc.reversed)) }) } }