Battery control: apply battery mode per battery and only on change (#32967)

core/site.go

@@ -116,6 +116,7 @@ type Site struct {
 	batteryMode              api.BatteryMode             // Battery mode (runtime only, not persisted)
 	batteryModeExternal      api.BatteryMode             // Battery mode (external, runtime only, not persisted)
 	batteryModeExternalTimer time.Time                   // Battery mode timer for external control
+	batteryModeApplied       map[string]api.BatteryMode  // Battery mode last applied per battery meter
 	suggestions              map[string]types.Suggestion // Optimizer suggestions by device key
 	suggestionActions        map[string]string           // last notified actionable optimizer action by device key
 

core/site_battery.go

@@ -146,10 +146,14 @@ func (site *Site) batteryMaxSocReached(dev config.Device[api.Meter]) (bool, erro
 // api.BatteryCharge:
 //
 //	The current soc is validated against max soc.
-//	In case max soc is reached, hold mode is applied.
+//	In case max soc is reached, hold mode is applied to that battery only.
 func (site *Site) applyBatteryMode(mode api.BatteryMode) error {
 	fromToCharge := mode == api.BatteryCharge || mode == api.BatteryUnknown && site.batteryMode == api.BatteryCharge
 
+	if site.batteryModeApplied == nil {
+		site.batteryModeApplied = make(map[string]api.BatteryMode)
+	}
+
 	for _, dev := range site.batteryMeters {
 		meter := dev.Instance()
 
@@ -158,8 +162,11 @@ func (site *Site) applyBatteryMode(mode api.BatteryMode) error {
 			continue
 		}
 
+		// mode is per battery, max soc is validated individually
+		devMode := mode
+
 		// validate max soc
-		if fromToCharge && mode != api.BatteryHold {
+		if fromToCharge && devMode != api.BatteryHold {
 			ok, err := site.batteryMaxSocReached(dev)
 			if err != nil && !errors.Is(err, api.ErrNotAvailable) {
 				return err
@@ -167,18 +174,25 @@ func (site *Site) applyBatteryMode(mode api.BatteryMode) error {
 
 			// put battery into hold mode when soc limit reached
 			if ok {
-				// TODO do this only once
-				mode = api.BatteryHold
+				devMode = api.BatteryHold
 			}
 		}
 
-		if mode != api.BatteryUnknown {
-			if err := batCtrl.SetBatteryMode(mode); err == nil {
-				site.log.DEBUG.Printf("set battery %s mode: %s", deviceTitleOrName(dev), mode)
-			} else if !errors.Is(err, api.ErrNotAvailable) {
+		// don't re-apply the mode the battery is already in
+		name := dev.Config().Name
+		if devMode == api.BatteryUnknown || devMode == site.batteryModeApplied[name] {
+			continue
+		}
+
+		if err := batCtrl.SetBatteryMode(devMode); err != nil {
+			if !errors.Is(err, api.ErrNotAvailable) {
 				return err
 			}
+			continue
 		}
+
+		site.batteryModeApplied[name] = devMode
+		site.log.DEBUG.Printf("set battery %s mode: %s", deviceTitleOrName(dev), devMode)
 	}
 
 	return nil

core/site_battery_test.go

@@ -85,6 +85,75 @@ func TestApplyBatteryMode(t *testing.T) {
 	}
 }
 
+// battery meter with soc, controller and soc limits
+func batteryControlMock(ctrl *gomock.Controller, soc, maxSoc float64) (api.Meter, *api.MockBatteryController) {
+	batSoc := api.NewMockBattery(ctrl)
+	batSoc.EXPECT().Soc().Return(soc, nil).AnyTimes()
+
+	batSocLimit := api.NewMockBatterySocLimiter(ctrl)
+	batSocLimit.EXPECT().GetSocLimits().Return(0.0, maxSoc).AnyTimes()
+
+	batCon := api.NewMockBatteryController(ctrl)
+
+	return &struct {
+		api.Meter
+		api.Battery
+		api.BatteryController
+		api.BatterySocLimiter
+	}{
+		Battery:           batSoc,
+		BatteryController: batCon,
+		BatterySocLimiter: batSocLimit,
+	}, batCon
+}
+
+// TestBatteryHoldAppliedOnce guards that reaching max soc during grid charge switches
+// the battery to hold mode once instead of on every update
+func TestBatteryHoldAppliedOnce(t *testing.T) {
+	ctrl := gomock.NewController(t)
+
+	bat, batCon := batteryControlMock(ctrl, 90, 80)
+
+	site := &Site{
+		log:           util.NewLogger("foo"),
+		batteryMeters: []config.Device[api.Meter]{config.NewStaticDevice(config.Named{Name: "bat"}, bat)},
+		batteryMode:   api.BatteryCharge,
+	}
+
+	batCon.EXPECT().SetBatteryMode(api.BatteryHold).Times(1)
+
+	for range 3 {
+		site.updateBatteryMode(true, api.Rate{})
+	}
+
+	ctrl.Finish()
+}
+
+// TestBatteryHoldNotShared guards that one battery reaching max soc does not put the
+// remaining batteries into hold mode
+func TestBatteryHoldNotShared(t *testing.T) {
+	ctrl := gomock.NewController(t)
+
+	full, fullCon := batteryControlMock(ctrl, 90, 80)
+	empty, emptyCon := batteryControlMock(ctrl, 50, 80)
+
+	site := &Site{
+		log: util.NewLogger("foo"),
+		batteryMeters: []config.Device[api.Meter]{
+			config.NewStaticDevice(config.Named{Name: "full"}, full),
+			config.NewStaticDevice(config.Named{Name: "empty"}, empty),
+		},
+		batteryMode: api.BatteryCharge,
+	}
+
+	fullCon.EXPECT().SetBatteryMode(api.BatteryHold).Times(1)
+	emptyCon.EXPECT().SetBatteryMode(gomock.Any()).Times(0)
+
+	site.updateBatteryMode(true, api.Rate{})
+
+	ctrl.Finish()
+}
+
 func TestRequiredExternalBatteryMode(t *testing.T) {
 	for _, tc := range []struct {
 		internal, external, new api.BatteryMode
@@ -184,8 +253,10 @@ func TestExternalBatteryModeChange(t *testing.T) {
 		assert.Equal(t, site.batteryModeExternal, api.BatteryUnknown)
 		assert.False(t, site.batteryModeExternalTimer.IsZero())
 
-		// battery switched back to normal mode
-		batCon.EXPECT().SetBatteryMode(api.BatteryNormal).Times(1)
+		// battery switched back to normal mode unless already applied in step 2
+		if tc.expected != api.BatteryNormal {
+			batCon.EXPECT().SetBatteryMode(api.BatteryNormal).Times(1)
+		}
 		site.updateBatteryMode(false, api.Rate{})
 
 		// timer disabled