From 2266f430745b6143c35a65992fe303158b7de5f1 Mon Sep 17 00:00:00 2001 From: andig Date: Thu, 6 Aug 2020 16:35:47 +0200 Subject: [PATCH] Fix charged energy wrong when charge stopped (#268) --- api/api.go | 2 +- core/loadpoint.go | 14 +++-- core/loadpoint_test.go | 114 +++++++++++++++++++++++++++++++++++++++++ mock/mock_api.go | 40 ++++++++++++++- 4 files changed, 160 insertions(+), 10 deletions(-) diff --git a/api/api.go b/api/api.go index d17ecb54e..8cdaf0c01 100644 --- a/api/api.go +++ b/api/api.go @@ -2,7 +2,7 @@ package api import "time" -//go:generate mockgen -package mock -destination ../mock/mock_api.go github.com/andig/evcc/api Charger,Meter,MeterEnergy,Vehicle +//go:generate mockgen -package mock -destination ../mock/mock_api.go github.com/andig/evcc/api Charger,Meter,MeterEnergy,Vehicle,ChargeRater // ChargeMode are charge modes modeled after OpenWB type ChargeMode string diff --git a/core/loadpoint.go b/core/loadpoint.go index f3060553d..132e1e5c7 100644 --- a/core/loadpoint.go +++ b/core/loadpoint.go @@ -80,10 +80,9 @@ type LoadPoint struct { connectedTime time.Time // Time when vehicle was connected pvTimer time.Time // PV enabled/disable timer - socCharge float64 // Vehicle SoC - chargedEnergy float64 // Charged energy while connected - deltaChargedEnergy float64 // Charged energy for single cycle - chargeDuration time.Duration // Charge duration + socCharge float64 // Vehicle SoC + chargedEnergy float64 // Charged energy while connected + chargeDuration time.Duration // Charge duration } // NewLoadPointFromConfig creates a new loadpoint @@ -262,7 +261,6 @@ func (lp *LoadPoint) evChargeStartHandler() { // evChargeStopHandler sends external stop event func (lp *LoadPoint) evChargeStopHandler() { lp.log.INFO.Println("stop charging <-") - lp.chargedEnergy += lp.deltaChargedEnergy lp.notify(evChargeStop) } @@ -272,7 +270,7 @@ func (lp *LoadPoint) evVehicleConnectHandler() { // energy lp.chargedEnergy = 0 - lp.publish("chargedEnergy", 0) + lp.publish("chargedEnergy", lp.chargedEnergy) // duration lp.connectedTime = lp.clock.Now() @@ -532,7 +530,7 @@ func (lp *LoadPoint) updateChargeMeter() { // publish charged energy and duration func (lp *LoadPoint) publishChargeProgress() { if f, err := lp.chargeRater.ChargedEnergy(); err == nil { - lp.deltaChargedEnergy = 1e3 * f // convert to Wh + lp.chargedEnergy = 1e3 * f // convert to Wh } else { lp.log.ERROR.Printf("charge rater error: %v", err) } @@ -543,7 +541,7 @@ func (lp *LoadPoint) publishChargeProgress() { lp.log.ERROR.Printf("charge timer error: %v", err) } - lp.publish("chargedEnergy", lp.chargedEnergy+lp.deltaChargedEnergy) + lp.publish("chargedEnergy", lp.chargedEnergy) lp.publish("chargeDuration", lp.chargeDuration) } diff --git a/core/loadpoint_test.go b/core/loadpoint_test.go index 33a38b139..371a3a009 100644 --- a/core/loadpoint_test.go +++ b/core/loadpoint_test.go @@ -502,3 +502,117 @@ func TestSetModeAndSocAtDisconnect(t *testing.T) { ctrl.Finish() } + +// cacheExpecter can be used to verify asynchronously written values from cache +func cacheExpecter(t *testing.T, lp *LoadPoint) (*util.Cache, func(key string, val interface{})) { + // attach cache for verifying values + paramC := make(chan util.Param) + lp.uiChan = paramC + + cache := util.NewCache() + go cache.Run(paramC) + + expect := func(key string, val interface{}) { + p := cache.Get(key) + t.Logf("%s: %.f", key, p.Val) // REMOVE + if p.Val != val { + t.Errorf("%s wanted: %.0f, got %v", key, val, p.Val) + } + } + + return cache, expect +} + +func TestChargedEnergyAtDisconnect(t *testing.T) { + clock := clock.NewMock() + ctrl := gomock.NewController(t) + handler := mock.NewMockHandler(ctrl) + rater := mock.NewMockChargeRater(ctrl) + + lp := &LoadPoint{ + log: util.NewLogger("foo"), + bus: evbus.New(), + clock: clock, + chargeMeter: &Null{}, //silence nil panics + chargeRater: rater, + chargeTimer: &Null{}, //silence nil panics + HandlerConfig: HandlerConfig{ + MinCurrent: lpMinCurrent, + MaxCurrent: lpMaxCurrent, + }, + handler: handler, + status: api.StatusC, + } + + lp.Mode = api.ModeNow + handler.EXPECT().Prepare().Return() + attachListeners(t, lp) + + // attach cache for verifying values + _, expectCache := cacheExpecter(t, lp) + + // start charging at 0 kWh + handler.EXPECT().TargetCurrent().Return(int64(6)) + rater.EXPECT().ChargedEnergy().Return(0.0, nil) + handler.EXPECT().Status().Return(api.StatusC, nil) + handler.EXPECT().SyncEnabled().Return() + handler.EXPECT().Ramp(int64(16), true).Return(nil) + lp.Update(-1) + + // at 1:00h charging at 5 kWh + clock.Add(time.Hour) + handler.EXPECT().TargetCurrent().Return(int64(16)) + rater.EXPECT().ChargedEnergy().Return(5.0, nil) + handler.EXPECT().Status().Return(api.StatusC, nil) + handler.EXPECT().SyncEnabled().Return() + // handler.EXPECT().TargetCurrent().Return(int64(0)) // once more for status changes + handler.EXPECT().Ramp(int64(16), true).Return(nil) + lp.Update(-1) + expectCache("chargedEnergy", 5000.0) + + // at 1:00h stop charging at 5 kWh + clock.Add(time.Second) + handler.EXPECT().TargetCurrent().Return(int64(16)) + rater.EXPECT().ChargedEnergy().Return(5.0, nil) + handler.EXPECT().Status().Return(api.StatusB, nil) + handler.EXPECT().SyncEnabled().Return() + handler.EXPECT().TargetCurrent().Return(int64(0)) // once more for status changes + handler.EXPECT().Ramp(int64(16), true).Return(nil) + lp.Update(-1) + expectCache("chargedEnergy", 5000.0) + + // at 1:00h restart charging at 5 kWh + clock.Add(time.Second) + handler.EXPECT().TargetCurrent().Return(int64(16)) + rater.EXPECT().ChargedEnergy().Return(5.0, nil) + handler.EXPECT().Status().Return(api.StatusC, nil) + handler.EXPECT().SyncEnabled().Return() + handler.EXPECT().TargetCurrent().Return(int64(0)) // once more for status changes + handler.EXPECT().Ramp(int64(16), true).Return(nil) + lp.Update(-1) + expectCache("chargedEnergy", 5000.0) + + // at 1:30h continue charging at 7.5 kWh + clock.Add(30 * time.Minute) + handler.EXPECT().TargetCurrent().Return(int64(16)) + rater.EXPECT().ChargedEnergy().Return(7.5, nil) + handler.EXPECT().Status().Return(api.StatusC, nil) + handler.EXPECT().SyncEnabled().Return() + // handler.EXPECT().TargetCurrent().Return(int64(0)) // once more for status changes + handler.EXPECT().Ramp(int64(16), true).Return(nil) + lp.Update(-1) + expectCache("chargedEnergy", 7500.0) + + // at 2:00h stop charging at 10 kWh + clock.Add(30 * time.Minute) + handler.EXPECT().TargetCurrent().Return(int64(16)) + rater.EXPECT().ChargedEnergy().Return(10.0, nil) + handler.EXPECT().Status().Return(api.StatusB, nil) + handler.EXPECT().SyncEnabled().Return() + handler.EXPECT().TargetCurrent().Return(int64(0)) // once more for status changes + handler.EXPECT().Ramp(int64(16), true).Return(nil) + lp.Update(-1) + expectCache("chargedEnergy", 10000.0) + + ctrl.Finish() +} diff --git a/mock/mock_api.go b/mock/mock_api.go index 834757a26..f56504f3a 100644 --- a/mock/mock_api.go +++ b/mock/mock_api.go @@ -1,5 +1,5 @@ // Code generated by MockGen. DO NOT EDIT. -// Source: github.com/andig/evcc/api (interfaces: Charger,Meter,MeterEnergy,Vehicle) +// Source: github.com/andig/evcc/api (interfaces: Charger,Meter,MeterEnergy,Vehicle,ChargeRater) // Package mock is a generated GoMock package. package mock @@ -232,3 +232,41 @@ func (mr *MockVehicleMockRecorder) Title() *gomock.Call { mr.mock.ctrl.T.Helper() return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Title", reflect.TypeOf((*MockVehicle)(nil).Title)) } + +// MockChargeRater is a mock of ChargeRater interface +type MockChargeRater struct { + ctrl *gomock.Controller + recorder *MockChargeRaterMockRecorder +} + +// MockChargeRaterMockRecorder is the mock recorder for MockChargeRater +type MockChargeRaterMockRecorder struct { + mock *MockChargeRater +} + +// NewMockChargeRater creates a new mock instance +func NewMockChargeRater(ctrl *gomock.Controller) *MockChargeRater { + mock := &MockChargeRater{ctrl: ctrl} + mock.recorder = &MockChargeRaterMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use +func (m *MockChargeRater) EXPECT() *MockChargeRaterMockRecorder { + return m.recorder +} + +// ChargedEnergy mocks base method +func (m *MockChargeRater) ChargedEnergy() (float64, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "ChargedEnergy") + ret0, _ := ret[0].(float64) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// ChargedEnergy indicates an expected call of ChargedEnergy +func (mr *MockChargeRaterMockRecorder) ChargedEnergy() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ChargedEnergy", reflect.TypeOf((*MockChargeRater)(nil).ChargedEnergy)) +}