From f5adefe753092486f4ff684b6e388445f85b19ec Mon Sep 17 00:00:00 2001 From: andig Date: Thu, 4 Jul 2024 19:50:24 +0200 Subject: [PATCH] Loadpoint: sync phases only if switchable (#14690) --- api/api.go | 2 +- api/mock.go | 80 ++++++++++++++++- core/loadpoint.go | 33 ++++--- core/loadpoint_sync_test.go | 174 ++++++++++++++++++++++++++++++++++++ 4 files changed, 273 insertions(+), 16 deletions(-) diff --git a/api/api.go b/api/api.go index cc6a9ea7ff..df264285b0 100644 --- a/api/api.go +++ b/api/api.go @@ -7,7 +7,7 @@ import ( "time" ) -//go:generate mockgen -package api -destination mock.go github.com/evcc-io/evcc/api Charger,ChargeState,CurrentLimiter,PhaseSwitcher,Identifier,Meter,MeterEnergy,PhaseCurrents,Vehicle,ChargeRater,Battery,Tariff,BatteryController,Circuit +//go:generate mockgen -package api -destination mock.go github.com/evcc-io/evcc/api Charger,ChargeState,CurrentLimiter,CurrentGetter,PhaseSwitcher,PhaseGetter,Identifier,Meter,MeterEnergy,PhaseCurrents,Vehicle,ChargeRater,Battery,Tariff,BatteryController,Circuit // Meter provides total active power in W type Meter interface { diff --git a/api/mock.go b/api/mock.go index 9b81c990ad..eb6f3c7caf 100644 --- a/api/mock.go +++ b/api/mock.go @@ -1,9 +1,9 @@ // Code generated by MockGen. DO NOT EDIT. -// Source: github.com/evcc-io/evcc/api (interfaces: Charger,ChargeState,CurrentLimiter,PhaseSwitcher,Identifier,Meter,MeterEnergy,PhaseCurrents,Vehicle,ChargeRater,Battery,Tariff,BatteryController,Circuit) +// Source: github.com/evcc-io/evcc/api (interfaces: Charger,ChargeState,CurrentLimiter,CurrentGetter,PhaseSwitcher,PhaseGetter,Identifier,Meter,MeterEnergy,PhaseCurrents,Vehicle,ChargeRater,Battery,Tariff,BatteryController,Circuit) // // Generated by this command: // -// mockgen -package api -destination mock.go github.com/evcc-io/evcc/api Charger,ChargeState,CurrentLimiter,PhaseSwitcher,Identifier,Meter,MeterEnergy,PhaseCurrents,Vehicle,ChargeRater,Battery,Tariff,BatteryController,Circuit +// mockgen -package api -destination mock.go github.com/evcc-io/evcc/api Charger,ChargeState,CurrentLimiter,CurrentGetter,PhaseSwitcher,PhaseGetter,Identifier,Meter,MeterEnergy,PhaseCurrents,Vehicle,ChargeRater,Battery,Tariff,BatteryController,Circuit // // Package api is a generated GoMock package. @@ -173,6 +173,44 @@ func (mr *MockCurrentLimiterMockRecorder) GetMinMaxCurrent() *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetMinMaxCurrent", reflect.TypeOf((*MockCurrentLimiter)(nil).GetMinMaxCurrent)) } +// MockCurrentGetter is a mock of CurrentGetter interface. +type MockCurrentGetter struct { + ctrl *gomock.Controller + recorder *MockCurrentGetterMockRecorder +} + +// MockCurrentGetterMockRecorder is the mock recorder for MockCurrentGetter. +type MockCurrentGetterMockRecorder struct { + mock *MockCurrentGetter +} + +// NewMockCurrentGetter creates a new mock instance. +func NewMockCurrentGetter(ctrl *gomock.Controller) *MockCurrentGetter { + mock := &MockCurrentGetter{ctrl: ctrl} + mock.recorder = &MockCurrentGetterMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockCurrentGetter) EXPECT() *MockCurrentGetterMockRecorder { + return m.recorder +} + +// GetMaxCurrent mocks base method. +func (m *MockCurrentGetter) GetMaxCurrent() (float64, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetMaxCurrent") + ret0, _ := ret[0].(float64) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetMaxCurrent indicates an expected call of GetMaxCurrent. +func (mr *MockCurrentGetterMockRecorder) GetMaxCurrent() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetMaxCurrent", reflect.TypeOf((*MockCurrentGetter)(nil).GetMaxCurrent)) +} + // MockPhaseSwitcher is a mock of PhaseSwitcher interface. type MockPhaseSwitcher struct { ctrl *gomock.Controller @@ -210,6 +248,44 @@ func (mr *MockPhaseSwitcherMockRecorder) Phases1p3p(arg0 any) *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Phases1p3p", reflect.TypeOf((*MockPhaseSwitcher)(nil).Phases1p3p), arg0) } +// MockPhaseGetter is a mock of PhaseGetter interface. +type MockPhaseGetter struct { + ctrl *gomock.Controller + recorder *MockPhaseGetterMockRecorder +} + +// MockPhaseGetterMockRecorder is the mock recorder for MockPhaseGetter. +type MockPhaseGetterMockRecorder struct { + mock *MockPhaseGetter +} + +// NewMockPhaseGetter creates a new mock instance. +func NewMockPhaseGetter(ctrl *gomock.Controller) *MockPhaseGetter { + mock := &MockPhaseGetter{ctrl: ctrl} + mock.recorder = &MockPhaseGetterMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockPhaseGetter) EXPECT() *MockPhaseGetterMockRecorder { + return m.recorder +} + +// GetPhases mocks base method. +func (m *MockPhaseGetter) GetPhases() (int, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetPhases") + ret0, _ := ret[0].(int) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetPhases indicates an expected call of GetPhases. +func (mr *MockPhaseGetterMockRecorder) GetPhases() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetPhases", reflect.TypeOf((*MockPhaseGetter)(nil).GetPhases)) +} + // MockIdentifier is a mock of Identifier interface. type MockIdentifier struct { ctrl *gomock.Controller diff --git a/core/loadpoint.go b/core/loadpoint.go index 41dbcd6435..dcfea1d771 100644 --- a/core/loadpoint.go +++ b/core/loadpoint.go @@ -711,8 +711,8 @@ func (lp *Loadpoint) syncCharger() error { ) // use chargers actual set current if available - cg, ok := lp.charger.(api.CurrentGetter) - if ok { + cg, isCg := lp.charger.(api.CurrentGetter) + if isCg { if current, err = cg.GetMaxCurrent(); err == nil { // smallest adjustment most PWM-Controllers can do is: 100%÷256×0,6A = 0.234A if math.Abs(lp.chargeCurrent-current) > 0.23 { @@ -728,9 +728,9 @@ func (lp *Loadpoint) syncCharger() error { } // use measured phase currents as fallback if charger does not provide max current or does not currently relay from vehicle (TWC3) - if !ok || errors.Is(err, api.ErrNotAvailable) { + if !isCg || errors.Is(err, api.ErrNotAvailable) { // validate if current too high by at least 1A - if current := lp.GetMaxPhaseCurrent(); current > lp.chargeCurrent+1.0 { + if current := lp.GetMaxPhaseCurrent(); current >= lp.chargeCurrent+1.0 { if shouldBeConsistent { lp.log.WARN.Printf("charger logic error: current mismatch (got %.3gA measured, expected %.3gA)", current, lp.chargeCurrent) } @@ -740,17 +740,21 @@ func (lp *Loadpoint) syncCharger() error { } // sync phases - phases := lp.GetPhases() - if shouldBeConsistent && phases > 0 { - // fallback active phases from measured phases + _, isPs := lp.charger.(api.PhaseSwitcher) + if phases := lp.GetPhases(); isPs && shouldBeConsistent && phases > 0 { + // fallback to active phases from measured phases chargerPhases := lp.measuredPhases if chargerPhases == 2 { chargerPhases = 3 } - if pg, ok := lp.charger.(api.PhaseGetter); ok { - if cp, err := pg.GetPhases(); err == nil { - chargerPhases = cp + pg, isPg := lp.charger.(api.PhaseGetter) + if isPg { + if chargerPhases, err = pg.GetPhases(); err == nil { + if chargerPhases > 0 && chargerPhases != phases { + lp.log.WARN.Printf("charger logic error: phases mismatch (got %d, expected %d)", chargerPhases, phases) + lp.setPhases(chargerPhases) + } } else { if errors.Is(err, api.ErrNotAvailable) { return nil @@ -759,9 +763,12 @@ func (lp *Loadpoint) syncCharger() error { } } - if chargerPhases > 0 && chargerPhases != phases { - lp.log.WARN.Printf("charger logic error: phases mismatch (got %d, expected %d)", chargerPhases, phases) - lp.setPhases(chargerPhases) + // use measured phase currents for active phases as fallback if charger does not provide phases + if !isPg || errors.Is(err, api.ErrNotAvailable) { + if chargerPhases > phases { + lp.log.WARN.Printf("charger logic error: phases mismatch (got %d measured, expected %d)", chargerPhases, phases) + lp.setPhases(chargerPhases) + } } } diff --git a/core/loadpoint_sync_test.go b/core/loadpoint_sync_test.go index 995d28aacb..f95fc3c33d 100644 --- a/core/loadpoint_sync_test.go +++ b/core/loadpoint_sync_test.go @@ -3,6 +3,7 @@ package core import ( "testing" + evbus "github.com/asaskevich/EventBus" "github.com/benbjohnson/clock" "github.com/evcc-io/evcc/api" "github.com/evcc-io/evcc/util" @@ -47,3 +48,176 @@ func TestSyncCharger(t *testing.T) { assert.Equal(t, tc.corrected, lp.enabled) } } + +func TestSyncChargerCurrentsByGetter(t *testing.T) { + tc := []struct { + lpCurrent, actualCurrent, outCurrent float64 + }{ + {6, 5, 5}, // force + {6, 6.1, 6}, + {6, 6.5, 6.5}, + {6, 7, 7}, + } + + for _, tc := range tc { + ctrl := gomock.NewController(t) + t.Logf("%+v", tc) + + ch := api.NewMockCharger(ctrl) + cg := api.NewMockCurrentGetter(ctrl) + + charger := struct { + api.Charger + api.CurrentGetter + }{ + ch, cg, + } + + ch.EXPECT().Enabled().Return(true, nil) + cg.EXPECT().GetMaxCurrent().Return(tc.actualCurrent, nil).MaxTimes(1) + + lp := &Loadpoint{ + log: util.NewLogger("foo"), + bus: evbus.New(), + clock: clock.New(), + charger: charger, + status: api.StatusC, + enabled: true, + phases: 3, + chargeCurrent: tc.lpCurrent, + } + + require.NoError(t, lp.syncCharger()) + assert.Equal(t, tc.outCurrent, lp.chargeCurrent) + } +} + +func TestSyncChargerCurrentsByMeasurement(t *testing.T) { + tc := []struct { + lpCurrent float64 + chargeCurrents []float64 + outCurrent float64 + }{ + {6, []float64{5, 0, 0}, 6}, // ignore + {6, []float64{6.1, 0, 0}, 6}, + {6, []float64{6.5, 0, 0}, 6}, + {6, []float64{7, 0, 0}, 7}, + } + + for _, tc := range tc { + ctrl := gomock.NewController(t) + t.Logf("%+v", tc) + + charger := api.NewMockCharger(ctrl) + charger.EXPECT().Enabled().Return(true, nil) + + lp := &Loadpoint{ + log: util.NewLogger("foo"), + bus: evbus.New(), + clock: clock.New(), + charger: charger, + status: api.StatusC, + enabled: true, + phases: 3, + chargeCurrent: tc.lpCurrent, + chargeCurrents: tc.chargeCurrents, + } + + require.NoError(t, lp.syncCharger()) + assert.Equal(t, tc.outCurrent, lp.chargeCurrent) + } +} + +func TestSyncChargerPhasesByGetter(t *testing.T) { + tc := []struct { + lpPhases, actualPhases, outPhases int + }{ + {0, 0, 0}, + {1, 0, 1}, + {1, 1, 1}, + {1, 3, 3}, + {3, 0, 3}, + {3, 1, 1}, // force + {3, 3, 3}, + } + + for _, tc := range tc { + ctrl := gomock.NewController(t) + t.Logf("%+v", tc) + + ch := api.NewMockCharger(ctrl) + ps := api.NewMockPhaseSwitcher(ctrl) + pg := api.NewMockPhaseGetter(ctrl) + + charger := struct { + api.Charger + api.PhaseSwitcher + api.PhaseGetter + }{ + ch, ps, pg, + } + + ch.EXPECT().Enabled().Return(true, nil) + pg.EXPECT().GetPhases().Return(tc.actualPhases, nil).MaxTimes(1) + + lp := &Loadpoint{ + log: util.NewLogger("foo"), + bus: evbus.New(), + clock: clock.New(), + charger: charger, + status: api.StatusC, + enabled: true, + phases: tc.lpPhases, + measuredPhases: tc.actualPhases, + } + + require.NoError(t, lp.syncCharger()) + assert.Equal(t, tc.outPhases, lp.phases) + } +} + +func TestSyncChargerPhasesByMeasurement(t *testing.T) { + tc := []struct { + lpPhases, actualPhases, outPhases int + }{ + {0, 0, 0}, + {1, 0, 1}, + {1, 1, 1}, + {1, 3, 3}, + {3, 0, 3}, + {3, 1, 3}, // ignore + {3, 3, 3}, + } + + ctrl := gomock.NewController(t) + + for _, tc := range tc { + t.Logf("%+v", tc) + + ch := api.NewMockCharger(ctrl) + ps := api.NewMockPhaseSwitcher(ctrl) + + charger := struct { + api.Charger + api.PhaseSwitcher + }{ + ch, ps, + } + + ch.EXPECT().Enabled().Return(true, nil) + + lp := &Loadpoint{ + log: util.NewLogger("foo"), + bus: evbus.New(), + clock: clock.New(), + charger: charger, + status: api.StatusC, + enabled: true, + phases: tc.lpPhases, + measuredPhases: tc.actualPhases, + } + + require.NoError(t, lp.syncCharger()) + assert.Equal(t, tc.outPhases, lp.phases) + } +}