Fix battery mode calculation across loadpoints (evcc-io#12772)

core/site.go

@@ -210,7 +210,7 @@ func NewSiteFromConfig(
 	// revert battery mode on shutdown
 	shutdown.Register(func() {
 		if mode := site.GetBatteryMode(); batteryModeModified(mode) {
-			if err := site.updateBatteryMode(api.BatteryNormal); err != nil {
+			if err := site.applyBatteryMode(api.BatteryNormal); err != nil {
 				site.log.ERROR.Println("battery mode:", err)
 			}
 		}
@@ -462,19 +462,19 @@ func (site *Site) updateBatteryMeters() error {
 
 	for i, meter := range site.batteryMeters {
 		power, err := backoff.RetryWithData(meter.CurrentPower, bo())
-		if err == nil {
-			site.batteryPower += power
-			if len(site.batteryMeters) > 1 {
-				site.log.DEBUG.Printf("battery %d power: %.0fW", i+1, power)
-			}
-		} else {
+		if err != nil {
 			// power is required- return on error
 			return fmt.Errorf("battery %d power: %v", i+1, err)
 		}
 
+		site.batteryPower += power
+		if len(site.batteryMeters) > 1 {
+			site.log.DEBUG.Printf("battery %d power: %.0fW", i+1, power)
+		}
+
 		// battery energy (discharge)
 		var energy float64
-		if m, ok := meter.(api.MeterEnergy); err == nil && ok {
+		if m, ok := meter.(api.MeterEnergy); ok {
 			energy, err = m.TotalEnergy()
 			if err == nil {
 				totalEnergy += energy
@@ -768,21 +768,9 @@ func (site *Site) update(lp updater) {
 		flexiblePower = site.prioritizer.GetChargePowerFlexibility(lp)
 	}
 
-	var smartCostActive bool
-	if tariff := site.GetTariff(PlannerTariff); tariff != nil && tariff.Type() != api.TariffTypePriceStatic {
-		rates, err := tariff.Rates()
-
-		var rate api.Rate
-		if err == nil {
-			rate, err = rates.Current(time.Now())
-		}
-
-		if err == nil {
-			limit := lp.GetSmartCostLimit()
-			smartCostActive = limit != 0 && rate.Price <= limit
-		} else {
-			site.log.ERROR.Println("smartCost:", err)
-		}
+	smartCostActive, err := site.smartCostActive(lp)
+	if err != nil {
+		site.log.ERROR.Println("smartCost:", err)
 	}
 
 	if sitePower, batteryBuffered, batteryStart, err := site.sitePower(totalChargePower, flexiblePower); err == nil {
@@ -810,12 +798,8 @@ func (site *Site) update(lp updater) {
 		site.log.ERROR.Println(err)
 	}
 
-	if batMode := site.GetBatteryMode(); site.batteryDischargeControl {
-		if mode := site.determineBatteryMode(site.Loadpoints(), smartCostActive); mode != batMode {
-			if err := site.updateBatteryMode(mode); err != nil {
-				site.log.ERROR.Println("battery mode:", err)
-			}
-		}
+	if site.batteryDischargeControl {
+		site.updateBatteryMode()
 	}
 
 	site.stats.Update(site)

core/site_api.go

@@ -304,7 +304,7 @@ func (site *Site) SetBatteryDischargeControl(val bool) error {
 	if site.GetBatteryDischargeControl() != val {
 		// reset to normal when disabling
 		if mode := site.GetBatteryMode(); !val && batteryModeModified(mode) {
-			if err := site.updateBatteryMode(api.BatteryNormal); err != nil {
+			if err := site.applyBatteryMode(api.BatteryNormal); err != nil {
 				return err
 			}
 		}

core/site_battery.go

@@ -1,6 +1,8 @@
 package core
 
 import (
+	"time"
+
 	"github.com/evcc-io/evcc/api"
 	"github.com/evcc-io/evcc/core/keys"
 	"github.com/evcc-io/evcc/core/loadpoint"
@@ -30,17 +32,7 @@ func (site *Site) SetBatteryMode(batMode api.BatteryMode) {
 	}
 }
 
-func (site *Site) determineBatteryMode(loadpoints []loadpoint.API, smartCostActive bool) api.BatteryMode {
-	for _, lp := range loadpoints {
-		if lp.GetStatus() == api.StatusC && (smartCostActive || lp.IsFastChargingActive()) {
-			return api.BatteryHold
-		}
-	}
-
-	return api.BatteryNormal
-}
-
-func (site *Site) updateBatteryMode(mode api.BatteryMode) error {
+func (site *Site) applyBatteryMode(mode api.BatteryMode) error {
 	// update batteries
 	for _, meter := range site.batteryMeters {
 		if batCtrl, ok := meter.(api.BatteryController); ok {
@@ -55,3 +47,46 @@ func (site *Site) updateBatteryMode(mode api.BatteryMode) error {
 
 	return nil
 }
+
+func (site *Site) smartCostActive(lp loadpoint.API) (bool, error) {
+	tariff := site.GetTariff(PlannerTariff)
+	if tariff == nil || tariff.Type() == api.TariffTypePriceStatic {
+		return false, nil
+	}
+
+	rates, err := tariff.Rates()
+	if err != nil {
+		return false, err
+	}
+
+	rate, err := rates.Current(time.Now())
+	if err != nil {
+		return false, err
+	}
+
+	limit := lp.GetSmartCostLimit()
+	return limit != 0 && rate.Price <= limit, nil
+}
+
+func (site *Site) updateBatteryMode() {
+	mode := api.BatteryNormal
+
+	for _, lp := range site.Loadpoints() {
+		smartCostActive, err := site.smartCostActive(lp)
+		if err != nil {
+			site.log.ERROR.Println("smart cost:", err)
+			continue
+		}
+
+		if lp.GetStatus() == api.StatusC && (smartCostActive || lp.IsFastChargingActive()) {
+			mode = api.BatteryHold
+			break
+		}
+	}
+
+	if batMode := site.GetBatteryMode(); mode != batMode {
+		if err := site.applyBatteryMode(mode); err != nil {
+			site.log.ERROR.Println("battery mode:", err)
+		}
+	}
+}

core/site_battery_test.go

@@ -4,50 +4,12 @@ import (
 	"testing"
 
 	"github.com/evcc-io/evcc/api"
-	"github.com/evcc-io/evcc/core/loadpoint"
 	"github.com/evcc-io/evcc/util"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	"go.uber.org/mock/gomock"
 )
 
-func TestDetermineBatteryMode(t *testing.T) {
-	ctrl := gomock.NewController(t)
-
-	tcs := []struct {
-		chargeStatus       api.ChargeStatus
-		fastChargingActive bool
-		smartCostActive    bool
-		expBatMode         api.BatteryMode
-	}{
-		{api.StatusB, false, false, api.BatteryNormal}, // not charging | fast charge not active | smart cost not active -> bat normal
-		{api.StatusB, true, false, api.BatteryNormal},  // not charging | fast charge active 	 | smart cost not active -> bat normal
-		{api.StatusC, false, false, api.BatteryNormal}, // charging 	| fast charge not active | smart cost not active -> bat normal
-		{api.StatusC, true, false, api.BatteryHold},    // charging 	| fast charge active 	 | smart cost not active -> bat hold
-		{api.StatusB, false, true, api.BatteryNormal},  // not charging | fast charge not active | smart cost active	 -> bat normal
-		{api.StatusB, true, true, api.BatteryNormal},   // not charging | fast charge active	 | smart cost active	 -> bat normal
-		{api.StatusC, false, true, api.BatteryHold},    // charging 	| fast charge not active | smart cost active	 -> bat hold
-		{api.StatusC, true, true, api.BatteryHold},     // charging 	| fast charge active	 | smart cost active	 -> bat hold
-	}
-
-	log := util.NewLogger("foo")
-
-	for _, tc := range tcs {
-		s := &Site{
-			log: log,
-		}
-
-		lp := loadpoint.NewMockAPI(ctrl)
-		lp.EXPECT().GetStatus().Return(tc.chargeStatus).AnyTimes()
-		lp.EXPECT().IsFastChargingActive().Return(tc.fastChargingActive).AnyTimes()
-
-		loadpoints := []loadpoint.API{lp}
-
-		mode := s.determineBatteryMode(loadpoints, tc.smartCostActive)
-		assert.Equal(t, tc.expBatMode, mode, tc)
-	}
-}
-
 func TestUpdateBatteryMode(t *testing.T) {
 	expBatMode := api.BatteryHold
 
@@ -68,7 +30,7 @@ func TestUpdateBatteryMode(t *testing.T) {
 		batteryMode:   api.BatteryNormal,
 	}
 
-	err := s.updateBatteryMode(expBatMode)
+	err := s.applyBatteryMode(expBatMode)
 	require.NoError(t, err)
 	assert.Equal(t, expBatMode, s.GetBatteryMode())
 }