Feat/battery loop

[webalexeu]

No description provided.

[sourcery-ai]

Hey - I've found 5 issues, and left some high level feedback:

• In batteryFastTick, the goroutine launched in the for i := range snap.batteries loop closes over the loop variable i instead of taking it as a parameter, which can lead to races and writing power/err results into the wrong slot; pass i into the goroutine (e.g. go func(i int) { ... }(i)).
• batteryFastTick holds batteryPlanMu across potentially slow operations (meter reads, power commands, stopBatteries), which can block buildBatterySnapshot and make snapshots stale; consider narrowing the critical section (copy the snapshot under the lock, then release it before doing I/O and control).

Prompt for AI Agents

Please address the comments from this code review:

## Overall Comments
- In batteryFastTick, the goroutine launched in the `for i := range snap.batteries` loop closes over the loop variable `i` instead of taking it as a parameter, which can lead to races and writing power/err results into the wrong slot; pass `i` into the goroutine (e.g. `go func(i int) { ... }(i)`).
- batteryFastTick holds batteryPlanMu across potentially slow operations (meter reads, power commands, stopBatteries), which can block buildBatterySnapshot and make snapshots stale; consider narrowing the critical section (copy the snapshot under the lock, then release it before doing I/O and control).

## Individual Comments

### Comment 1
<location path="core/site_api.go" line_range="388" />
<code_context>
+	return site.batteryCalibrationCharge
+}
+
+// SetBatteryCalibrationCharge enables/disables the one-shot LFP calibration charge.
+// When active, maxSoc is bypassed and the battery charges to 100%. Auto-disables
+// when aggregate SoC reaches 99%. Not persisted — resets to false on restart.
</code_context>
<issue_to_address>
**issue:** Calibration auto-disable comment disagrees with implementation threshold

The comment states that calibration auto-disables at 99% aggregate SoC, but `buildBatterySnapshot` turns `batteryCalibrationCharge` off at `site.battery.Soc >= 100`. Please align the comment and the threshold so it’s clear exactly when calibration stops.
</issue_to_address>

### Comment 2
<location path="docs/agents/battery-management.md" line_range="154-159" />
<code_context>
+Linearly reduces charge power in the last 10% of SoC before `maxSoc`. Mimics the CC/CV charging profile that protects lithium cells from stress near full charge.
+
+```
+taperFactor = (maxSoc - currentSoc) / chargeTaperRange   (clamped to minimum 0.10)
+chargePower = requestedPower × taperFactor
+```
+
+- **Taper range**: 5% SoC below maxSoc
+- **Minimum factor**: 25% of requested power (never fully stopped by taper)
+- **Per-battery**: applied individually using each battery's `BatterySocLimiter.GetSocLimits()`
+- Applied after the hard-cap from `BatteryPowerLimiter`
</code_context>
<issue_to_address>
**issue:** The documented minimum taper factor is inconsistent (0.10 vs 25%).

The formula comment says `taperFactor` is clamped to a minimum of 0.10, while the bullet below defines a "Minimum factor" of 25% of requested power. These look like the same lower bound but use different values. Please clarify which minimum is correct and update the docs to be consistent, or explicitly explain if they refer to different concepts.
</issue_to_address>

### Comment 3
<location path="core/site_battery_fast.go" line_range="116" />
<code_context>
+	}
+}
+
+func (site *Site) batteryFastTick() {
+	site.batteryPlanMu.Lock()
+	defer site.batteryPlanMu.Unlock()
</code_context>
<issue_to_address>
**issue (complexity):** Consider extracting the meter-guard/target computation from `batteryFastTick` and splitting `fastControl` into smaller helpers so each step of the fast loop is handled by a focused, testable function.

You can reduce the cognitive load without changing behavior by carving out a few focused helpers from the monolithic functions. Two low‑risk wins:

---

### 1. Split `batteryFastTick` into guard + planning

`batteryFastTick` currently handles snapshot age, meter guards, direction choice, and calling the controller in one go. Extracting the guard + target computation into a pure helper keeps the main tick readable and easier to test.

```go
// small data carrier for the fast loop
type fastLoopState struct {
	gridPower, battPower         float64
	dischargeTarget, chargeTarget float64
	desired                      batteryPlanDirection
}

// pure-ish helper: all meter guards + target computation
func (site *Site) computeFastLoopState(snap *batterySnapshot) (*fastLoopState, bool) {
	gridPower, err := site.gridMeter.CurrentPower()
	if err != nil {
		batteryLog.DEBUG.Printf("solar power (fast): grid power: %v", err)
		return nil, false
	}

	firstTick := !site.batteryGuardValid
	if !firstTick && gridPower == site.batteryLastGrid {
		batteryLog.TRACE.Printf("solar power (fast): stale grid %.0fW, skip", gridPower)
		return nil, false
	}

	powers := make([]float64, len(snap.batteries))
	errs := make([]error, len(snap.batteries))
	var wg sync.WaitGroup
	for i := range snap.batteries {
		i := i
		wg.Add(1)
		go func() {
			defer wg.Done()
			powers[i], errs[i] = snap.batteries[i].meter.CurrentPower()
		}()
	}
	wg.Wait()

	var battPower float64
	for i, e := range snap.batteries {
		if errs[i] != nil {
			batteryLog.DEBUG.Printf("solar power (fast): %s power: %v", e.name, errs[i])
			return nil, false
		}
		battPower += powers[i]
	}

	dGrid, dBatt := gridPower-site.batteryLastGrid, battPower-site.batteryLastBatt
	site.batteryLastGrid, site.batteryLastBatt, site.batteryGuardValid = gridPower, battPower, true
	if !firstTick && math.Abs(dBatt) > fastLoopSkewThreshold && math.Abs(dGrid+dBatt) > fastLoopSkewThreshold {
		batteryLog.TRACE.Printf("solar power (fast): meters inconsistent (Δgrid %.0fW, Δbattery %.0fW), skip", dGrid, dBatt)
		return nil, false
	}

	dischargeTarget := battPower + gridPower + snap.dischargeOffset - snap.dischargeEvExcluded
	chargeTarget := -battPower - (gridPower + snap.chargeOffset)

	desired := batteryPlanIdle
	switch {
	case dischargeTarget > snap.threshold && dischargeTarget >= chargeTarget:
		desired = batteryPlanDischarge
	case chargeTarget > snap.threshold:
		desired = batteryPlanCharge
	}

	return &fastLoopState{
		gridPower:       gridPower,
		battPower:       battPower,
		dischargeTarget: dischargeTarget,
		chargeTarget:    chargeTarget,
		desired:         desired,
	}, true
}
```

Then `batteryFastTick` becomes mostly orchestration, with much less nesting:

```go
func (site *Site) batteryFastTick() {
	site.batteryPlanMu.Lock()
	defer site.batteryPlanMu.Unlock()

	snap := site.batterySnapshot
	if snap == nil || !snap.enabled || len(snap.batteries) == 0 || time.Since(snap.created) > batterySnapshotMaxAge {
		// existing logging here
		return
	}

	if site.batteryStopped == nil {
		site.batteryStopped = make(map[string]int)
	}

	st, ok := site.computeFastLoopState(snap)
	if !ok {
		return
	}

	batteryLog.TRACE.Printf("solar power (fast): grid=%.0fW batt=%.0fW dis=%.0fW chg=%.0fW committed=%s desired=%s",
		st.gridPower, st.battPower, st.dischargeTarget, st.chargeTarget,
		batteryPlanDirectionString(site.batteryFastDirection), batteryPlanDirectionString(st.desired))

	direction := site.batteryArbitrateDirection(st.desired)

	switch direction {
	case batteryPlanCharge:
		site.fastControl(snap, batteryPlanCharge, math.Max(0, st.chargeTarget))
	case batteryPlanDischarge:
		site.fastControl(snap, batteryPlanDischarge, math.Max(0, st.dischargeTarget))
	default:
		site.stopBatteries(fastEntriesAll(snap))
		site.batteryChargeActive, site.batteryDischargeActive = nil, nil
		site.batteryChargeTier, site.batteryDischargeTier = 0, 0
	}
	site.batteryFastDirection = direction
}
```

This isolates the meter guard / skew logic and target computation into something you can unit‑test independently without touching direction arbitration or dispatch.

---

### 2. Decompose `fastControl` into small, composable steps

`fastControl` currently does: eligibility filtering → tiering/sticky/concentration → scaling/tapering → dispatch/stop. Splitting this into 2–3 focused helpers keeps behavior but flattens nesting.

For example:

```go
func (site *Site) filterEligible(snap *batterySnapshot, dir batteryPlanDirection) (active, ineligible []fastEntry) {
	charge := dir == batteryPlanCharge
	for i := range snap.batteries {
		e := &snap.batteries[i]
		fe := fastEntry{batterySnapEntry: e}
		if charge {
			if !snap.calibration && e.hasSocLimit && e.maxSoc > 0 && e.socOK && e.soc >= e.maxSoc {
				ineligible = append(ineligible, fe)
				continue
			}
		} else {
			if !e.socOK || (e.hasSocLimit && e.soc <= e.minSoc) {
				ineligible = append(ineligible, fe)
				continue
			}
		}
		active = append(active, fe)
	}
	return
}

func (site *Site) applyTieringAndSticky(
	snap *batterySnapshot,
	dir batteryPlanDirection,
	target float64,
	active []fastEntry,
) (selected, toStop []fastEntry) {
	charge := dir == batteryPlanCharge
	if snap.pool {
		return active, nil
	}

	capOf := func(e fastEntry) float64 {
		if charge {
			return e.chargeCap
		}
		return e.dischargeCap
	}

	var maxPerBat float64
	for _, e := range active {
		if c := capOf(e); c > 0 && (maxPerBat == 0 || c < maxPerBat) {
			maxPerBat = c
		}
	}

	selected = active

	if maxPerBat > 0 && snap.tiering {
		tierPerBat := maxPerBat * batteryTierFraction
		tier := &site.batteryChargeTier
		activeNames := &site.batteryChargeActive
		if !charge {
			tier = &site.batteryDischargeTier
			activeNames = &site.batteryDischargeActive
		}
		*tier = computeTier(target, tierPerBat, *tier, len(active))
		needed := *tier

		if needed < len(active) {
			var stop []fastEntry
			selected, stop = site.selectSticky(active, needed, charge, activeNames)
			toStop = append(toStop, stop...)
		} else {
			*activeNames = nil
		}
	} else if maxPerBat == 0 {
		share := target / float64(len(active))
		if share < minEffectiveShare && len(active) > 1 {
			best := 0
			for i, e := range active {
				if charge {
					if e.socOK && (i == 0 || e.soc < active[best].soc) {
						best = i
					}
				} else if e.socOK && (i == 0 || e.soc > active[best].soc) {
					best = i
				}
			}
			for i, e := range active {
				if i != best {
					toStop = append(toStop, e)
				}
			}
			selected = active[best : best+1]
		}
	}
	return
}
```

Then `fastControl` becomes a thin pipeline:

```go
func (site *Site) fastControl(snap *batterySnapshot, dir batteryPlanDirection, target float64) {
	active, ineligible := site.filterEligible(snap, dir)

	deferStop := append([]fastEntry{}, ineligible...)
	if len(active) == 0 {
		site.stopBatteries(fastEntriesAll(snap))
		return
	}

	selected, moreStop := site.applyTieringAndSticky(snap, dir, target, active)
	deferStop = append(deferStop, moreStop...)

	charge := dir == batteryPlanCharge
	commands := site.computeCommands(snap, selected, dir, target)
	site.sendBatteries(selected, charge, commands)

	var total float64
	for _, c := range commands {
		total += c
	}
	batteryLog.DEBUG.Printf("solar power (fast): %s %.0fW across %d/%d batteries (grid target %.0fW)",
		batteryPlanDirectionString(dir), total, len(selected), len(snap.batteries), target)

	site.stopBatteries(deferStop)
}
```

`computeCommands` can hold the cap + taper logic you already have, but in a small, self‑contained function.

This keeps all existing semantics (tiering, sticky, tapering, stop‑guard), but breaks them into named steps aligned with the reviewer’s suggested responsibilities (`filterEligible`, `applyTieringAndSticky`, `computeCommands`), which reduces branching in each function and makes the module easier to reason about and test.
</issue_to_address>

### Comment 4
<location path="core/site.go" line_range="82" />
<code_context>
+	bufferSoc                float64              // continue charging on battery above this Soc (EV loadpoints)
+	bufferStartSoc           float64              // start charging on battery above this Soc (EV loadpoints)
+	batteryDischargeControl  bool                 // prevent battery discharge for fast and planned charging
+	batterySolarControl      bool                 // actively charge from surplus / discharge to cover loads
+	batteryCalibrationCharge bool                 // one-shot: bypass maxSoc and charge to 100% for LFP calibration (not persisted)
+	batteryControlDeadBand   float64              // minimum surplus/deficit (W) to start charge or discharge (stability dead band)
</code_context>
<issue_to_address>
**issue (complexity):** Consider encapsulating the new battery solar/fast-loop configuration and state into a dedicated controller struct that `Site` delegates to, instead of expanding `Site` with many loosely related fields and behaviors.

You can reduce the added complexity without changing behavior by encapsulating the new battery‑solar/fast‑loop state into a dedicated struct and letting `Site` delegate to it.

### 1. Group the new fields into a controller struct

Instead of adding all battery‑solar/fast‑loop fields directly on `Site`, move them into a dedicated type:

```go
type BatterySolarController struct {
	// configuration
	solarControl      bool
	calibrationCharge bool
	controlDeadBand   float64
	solarPool         bool
	solarTiering      bool
	solarSticky       bool
	solarTapering     bool

	// tiered / sticky state
	chargeTier      int
	dischargeTier   int
	chargeActive    []string
	dischargeActive []string
	stopped         map[string]int

	// snapshot / fast‑loop contract
	mu        sync.Mutex
	snapshot  *batterySnapshot
	direction batteryPlanDirection

	// fast‑loop metering guard
	lastGrid   float64
	lastBatt   float64
	guardValid bool

	// flip timing / backoff
	flipSince            time.Time
	lastFlipRequest      time.Time
	flipBackoff          time.Duration
}
```

Attach it to `Site`:

```go
type Site struct {
	// ...

	prioritySoc             float64
	bufferSoc               float64
	bufferStartSoc          float64
	batteryDischargeControl bool
	batteryGridChargeLimit  *float64

	batterySolar *BatterySolarController

	// cached state ...
}
```

This keeps the god‑struct surface area down and makes the conceptual boundary of “battery solar control” obvious.

### 2. Delegate settings load/restore into the controller

Move the solar‑related settings wiring out of `Site.restoreSettings` into a method on `BatterySolarController`. The `Site` owns the controller but doesn’t need to know about all flags:

```go
func (c *BatterySolarController) LoadSettings(settings *util.Settings) error {
	// defaults
	c.solarPool = false
	c.solarTiering = true
	c.solarSticky = true
	c.solarTapering = true

	if v, err := settings.Bool(keys.BatterySolarControl); err == nil {
		c.solarControl = v
	}
	if v, err := settings.Float(keys.BatteryControlDeadBand); err == nil {
		c.controlDeadBand = v
	}
	if v, err := settings.Bool(keys.BatterySolarPool); err == nil {
		c.solarPool = v
	}
	if v, err := settings.Bool(keys.BatterySolarTiering); err == nil {
		c.solarTiering = v
	}
	if v, err := settings.Bool(keys.BatterySolarSticky); err == nil {
		c.solarSticky = v
	}
	if v, err := settings.Bool(keys.BatterySolarTapering); err == nil {
		c.solarTapering = v
	}

	return nil
}
```

`Site.restoreSettings` then becomes simpler:

```go
func (site *Site) restoreSettings() error {
	// existing non‑battery settings...

	if err := site.batterySolar.LoadSettings(settings); err != nil {
		return err
	}

	// rest of method...
	return nil
}
```

### 3. Delegate publishing of solar‑related state

Similarly, move the publish calls into the controller, so `prepare` doesn’t need to know every flag:

```go
func (c *BatterySolarController) Publish(publish func(key string, val any)) {
	publish(keys.BatterySolarControl, c.solarControl)
	publish(keys.BatteryCalibrationCharge, c.calibrationCharge)
	publish(keys.BatteryControlDeadBand, c.controlDeadBand)
	publish(keys.BatterySolarPool, c.solarPool)
	publish(keys.BatterySolarTiering, c.solarTiering)
	publish(keys.BatterySolarSticky, c.solarSticky)
	publish(keys.BatterySolarTapering, c.solarTapering)
}
```

Then in `Site.prepare`:

```go
func (site *Site) prepare() {
	if err := site.restoreSettings(); err != nil {
		site.log.ERROR.Println(err)
	}

	// other publishes...
	site.batterySolar.Publish(site.publish)
}
```

### 4. Encapsulate fast‑loop direction / flip logic

The fast‑loop‑specific fields and logic (`batteryFastDirection`, `batteryLastGrid`, `batteryLastBatt`, `batteryGuardValid`, `batteryFlipSince`, `lastBatteryFlipRequest`, `batteryFlipBackoff`) can live behind methods on `BatterySolarController` so `Site.updateBatteryMode` and `batteryFastLoop` just call into it:

```go
func (c *BatterySolarController) Plan(
	now time.Time,
	sitePower float64,
	sitePowerValid bool,
	batt types.BatteryState,
) batteryPlan {
	c.mu.Lock()
	defer c.mu.Unlock()

	// use c.direction, c.lastGrid, c.flipSince, c.lastFlipRequest, c.flipBackoff, etc.
	// to compute the next plan, respecting dead band and backoff

	return plan
}
```

Then adjust `Site.updateBatteryMode` to delegate:

```go
func (site *Site) updateBatteryMode(
	gridChargeActive bool,
	rate *planner.Rate,
	sitePower float64,
	sitePowerValid bool,
) {
	if !site.batterySolar.solarControl {
		// existing non‑solar logic
		return
	}

	plan := site.batterySolar.Plan(time.Now(), sitePower, sitePowerValid, site.battery)

	// apply plan to underlying batteries / modes (unchanged)
}
```

This preserves all behavior but:

- keeps `Site` focused on orchestration and wiring,
- makes the battery solar controller a clear, testable unit,
- groups tightly coupled flags/state so you don’t have to reason about them scattered across `Site`.
</issue_to_address>

### Comment 5
<location path="core/site_battery.go" line_range="165" />
<code_context>
+	}
+}
+
+// buildBatterySnapshot reads the slow-moving per-battery state (SoC, limits, power caps) and
+// the current config, then publishes a snapshot for the fast loop under batteryPlanMu. It
+// commands no power and chooses no direction - those are the fast loop's job.
</code_context>
<issue_to_address>
**issue (complexity):** Consider extracting small helper functions from buildBatterySnapshot and separating snapshot updates from mode decisions to make the battery control flow easier to scan and maintain without changing behavior.

You can reduce the perceived complexity without changing behavior by factoring out a few narrowly‑scoped helpers and loosening the coupling between mode logic and snapshot building.

### 1. Split `buildBatterySnapshot` into small helpers

Right now `buildBatterySnapshot` handles:

- EV power aggregation
- Calibration charge completion
- Residual/offset calculations
- Snapshot assembly
- Per‑battery capability discovery

You can keep the same behavior but move each concern into a helper with a clear contract. That makes the main function easier to scan and reason about.

Example (shortened for illustration only):

```go
func (site *Site) buildBatterySnapshot(rate api.Rate) {
	evPower, evPowerFast := site.computeEvPowers()

	site.maybeFinishCalibrationCharge()

	chargeOffset, dischargeOffset, dischargeEvExcluded := site.computeBatteryOffsets(rate, evPower, evPowerFast)

	snap := &batterySnapshot{
		enabled:             true,
		pool:                site.batterySolarPool,
		tiering:             site.batterySolarTiering,
		sticky:              site.batterySolarSticky,
		tapering:            site.batterySolarTapering,
		calibration:         site.batteryCalibrationCharge,
		chargeOffset:        chargeOffset,
		dischargeOffset:     dischargeOffset,
		dischargeEvExcluded: dischargeEvExcluded,
		threshold:           standbyPower + site.batteryControlDeadBand,
		created:             time.Now(),
	}

	site.fillBatteryEntries(snap)

	site.batteryPlanMu.Lock()
	site.batterySnapshot = snap
	site.batteryPlanMu.Unlock()

	batteryLog.TRACE.Printf(
		"battery snapshot: %d batteries soc=%.0f%% chargeOff=%.0fW dischargeOff=%.0fW evExcl=%.0fW threshold=%.0fW",
		len(snap.batteries), site.battery.Soc, chargeOffset, dischargeOffset, dischargeEvExcluded, snap.threshold,
	)
}
```

Helpers (kept short and focused):

```go
func (site *Site) computeEvPowers() (evPower, evPowerFast float64) {
	for _, lp := range site.loadpoints {
		if lp.IsHeating() {
			continue
		}
		p := lp.GetChargePower()
		evPower += p
		if lp.GetStatus() != api.StatusA && lp.IsFastChargingActive() {
			evPowerFast += p
		}
	}
	return
}

func (site *Site) maybeFinishCalibrationCharge() {
	if site.batteryCalibrationCharge && site.battery.Soc >= 100 {
		site.Lock()
		site.batteryCalibrationCharge = false
		site.Unlock()
		site.publish(keys.BatteryCalibrationCharge, false)
		batteryLog.DEBUG.Printf("battery calibration charge complete (soc %.0f%%)", site.battery.Soc)
	}
}

func (site *Site) computeBatteryOffsets(
	rate api.Rate, evPower, evPowerFast float64,
) (chargeOffset, dischargeOffset, dischargeEvExcluded float64) {
	residual := site.GetResidualPower()

	if site.battery.Soc >= site.prioritySoc {
		chargeOffset = residual
	}
	if site.dischargeControlActive(rate) {
		dischargeEvExcluded = evPowerFast
	} else if !(site.bufferSoc > 0 && site.battery.Soc > site.bufferSoc) {
		dischargeEvExcluded = evPower
	}
	return chargeOffset, residual, dischargeEvExcluded
}

func (site *Site) fillBatteryEntries(snap *batterySnapshot) {
	for _, dev := range site.batteryMeters {
		ctrl, ok := api.Cap[api.BatteryPowerController](dev.Instance())
		if !ok {
			continue
		}
		e := batterySnapEntry{
			ctrl:  ctrl,
			meter: dev.Instance(),
			name:  dev.Config().Name,
		}
		if bat, ok := api.Cap[api.Battery](dev.Instance()); ok {
			if soc, err := bat.Soc(); err == nil {
				e.soc, e.socOK = soc, true
			}
		}
		if limiter, ok := api.Cap[api.BatterySocLimiter](dev.Instance()); ok {
			e.hasSocLimit = true
			e.minSoc, e.maxSoc = limiter.GetSocLimits()
		}
		if limiter, ok := api.Cap[api.BatteryPowerLimiter](dev.Instance()); ok {
			e.chargeCap, e.dischargeCap = limiter.GetPowerLimits()
		}
		snap.batteries = append(snap.batteries, e)
	}
}
```

This keeps all logic in the same place but makes each step self‑documenting and individually testable.

### 2. Decouple mode decisions from snapshot updates

`updateBatteryMode` currently decides the mode and also manages the snapshot for the fast loop. To reduce implicit coupling, you can keep the behavior but make the separation explicit: one function decides modes, another updates the snapshot.

Minimal change example:

```go
func (site *Site) updateBatteryMode(batteryGridChargeActive bool, rate api.Rate, sitePower float64, sitePowerValid bool) {
	batteryMode := site.requiredBatteryMode(batteryGridChargeActive, rate, sitePower)

	// existing HEMS dimming + applyBatteryMode logic stays as-is...
	// ...

	site.updateBatterySnapshot(rate)
}

func (site *Site) updateBatterySnapshot(rate api.Rate) {
	if !site.batterySolarControl {
		site.batteryPlanMu.Lock()
		site.batterySnapshot = nil
		site.batteryPlanMu.Unlock()
		return
	}
	site.buildBatterySnapshot(rate)
}
```

This keeps all current behavior (including when the snapshot is built/cleared) but makes the file’s responsibilities clearer and prepares for moving snapshot/selection concerns into a dedicated controller later if needed.
</issue_to_address>

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

[sourcery-ai]

issue: Calibration auto-disable comment disagrees with implementation threshold

The comment states that calibration auto-disables at 99% aggregate SoC, but buildBatterySnapshot turns batteryCalibrationCharge off at site.battery.Soc >= 100. Please align the comment and the threshold so it’s clear exactly when calibration stops.

[sourcery-ai]

issue: The documented minimum taper factor is inconsistent (0.10 vs 25%).

The formula comment says taperFactor is clamped to a minimum of 0.10, while the bullet below defines a "Minimum factor" of 25% of requested power. These look like the same lower bound but use different values. Please clarify which minimum is correct and update the docs to be consistent, or explicitly explain if they refer to different concepts.

[sourcery-ai]

issue (complexity): Consider extracting the meter-guard/target computation from batteryFastTick and splitting fastControl into smaller helpers so each step of the fast loop is handled by a focused, testable function.

You can reduce the cognitive load without changing behavior by carving out a few focused helpers from the monolithic functions. Two low‑risk wins:

---

1. Split batteryFastTick into guard + planning

batteryFastTick currently handles snapshot age, meter guards, direction choice, and calling the controller in one go. Extracting the guard + target computation into a pure helper keeps the main tick readable and easier to test.

// small data carrier for the fast loop
type fastLoopState struct {
	gridPower, battPower         float64
	dischargeTarget, chargeTarget float64
	desired                      batteryPlanDirection
}

// pure-ish helper: all meter guards + target computation
func (site *Site) computeFastLoopState(snap *batterySnapshot) (*fastLoopState, bool) {
	gridPower, err := site.gridMeter.CurrentPower()
	if err != nil {
		batteryLog.DEBUG.Printf("solar power (fast): grid power: %v", err)
		return nil, false
	}

	firstTick := !site.batteryGuardValid
	if !firstTick && gridPower == site.batteryLastGrid {
		batteryLog.TRACE.Printf("solar power (fast): stale grid %.0fW, skip", gridPower)
		return nil, false
	}

	powers := make([]float64, len(snap.batteries))
	errs := make([]error, len(snap.batteries))
	var wg sync.WaitGroup
	for i := range snap.batteries {
		i := i
		wg.Add(1)
		go func() {
			defer wg.Done()
			powers[i], errs[i] = snap.batteries[i].meter.CurrentPower()
		}()
	}
	wg.Wait()

	var battPower float64
	for i, e := range snap.batteries {
		if errs[i] != nil {
			batteryLog.DEBUG.Printf("solar power (fast): %s power: %v", e.name, errs[i])
			return nil, false
		}
		battPower += powers[i]
	}

	dGrid, dBatt := gridPower-site.batteryLastGrid, battPower-site.batteryLastBatt
	site.batteryLastGrid, site.batteryLastBatt, site.batteryGuardValid = gridPower, battPower, true
	if !firstTick && math.Abs(dBatt) > fastLoopSkewThreshold && math.Abs(dGrid+dBatt) > fastLoopSkewThreshold {
		batteryLog.TRACE.Printf("solar power (fast): meters inconsistent (Δgrid %.0fW, Δbattery %.0fW), skip", dGrid, dBatt)
		return nil, false
	}

	dischargeTarget := battPower + gridPower + snap.dischargeOffset - snap.dischargeEvExcluded
	chargeTarget := -battPower - (gridPower + snap.chargeOffset)

	desired := batteryPlanIdle
	switch {
	case dischargeTarget > snap.threshold && dischargeTarget >= chargeTarget:
		desired = batteryPlanDischarge
	case chargeTarget > snap.threshold:
		desired = batteryPlanCharge
	}

	return &fastLoopState{
		gridPower:       gridPower,
		battPower:       battPower,
		dischargeTarget: dischargeTarget,
		chargeTarget:    chargeTarget,
		desired:         desired,
	}, true
}

Then batteryFastTick becomes mostly orchestration, with much less nesting:

func (site *Site) batteryFastTick() {
	site.batteryPlanMu.Lock()
	defer site.batteryPlanMu.Unlock()

	snap := site.batterySnapshot
	if snap == nil || !snap.enabled || len(snap.batteries) == 0 || time.Since(snap.created) > batterySnapshotMaxAge {
		// existing logging here
		return
	}

	if site.batteryStopped == nil {
		site.batteryStopped = make(map[string]int)
	}

	st, ok := site.computeFastLoopState(snap)
	if !ok {
		return
	}

	batteryLog.TRACE.Printf("solar power (fast): grid=%.0fW batt=%.0fW dis=%.0fW chg=%.0fW committed=%s desired=%s",
		st.gridPower, st.battPower, st.dischargeTarget, st.chargeTarget,
		batteryPlanDirectionString(site.batteryFastDirection), batteryPlanDirectionString(st.desired))

	direction := site.batteryArbitrateDirection(st.desired)

	switch direction {
	case batteryPlanCharge:
		site.fastControl(snap, batteryPlanCharge, math.Max(0, st.chargeTarget))
	case batteryPlanDischarge:
		site.fastControl(snap, batteryPlanDischarge, math.Max(0, st.dischargeTarget))
	default:
		site.stopBatteries(fastEntriesAll(snap))
		site.batteryChargeActive, site.batteryDischargeActive = nil, nil
		site.batteryChargeTier, site.batteryDischargeTier = 0, 0
	}
	site.batteryFastDirection = direction
}

This isolates the meter guard / skew logic and target computation into something you can unit‑test independently without touching direction arbitration or dispatch.

---

2. Decompose fastControl into small, composable steps

fastControl currently does: eligibility filtering → tiering/sticky/concentration → scaling/tapering → dispatch/stop. Splitting this into 2–3 focused helpers keeps behavior but flattens nesting.

For example:

func (site *Site) filterEligible(snap *batterySnapshot, dir batteryPlanDirection) (active, ineligible []fastEntry) {
	charge := dir == batteryPlanCharge
	for i := range snap.batteries {
		e := &snap.batteries[i]
		fe := fastEntry{batterySnapEntry: e}
		if charge {
			if !snap.calibration && e.hasSocLimit && e.maxSoc > 0 && e.socOK && e.soc >= e.maxSoc {
				ineligible = append(ineligible, fe)
				continue
			}
		} else {
			if !e.socOK || (e.hasSocLimit && e.soc <= e.minSoc) {
				ineligible = append(ineligible, fe)
				continue
			}
		}
		active = append(active, fe)
	}
	return
}

func (site *Site) applyTieringAndSticky(
	snap *batterySnapshot,
	dir batteryPlanDirection,
	target float64,
	active []fastEntry,
) (selected, toStop []fastEntry) {
	charge := dir == batteryPlanCharge
	if snap.pool {
		return active, nil
	}

	capOf := func(e fastEntry) float64 {
		if charge {
			return e.chargeCap
		}
		return e.dischargeCap
	}

	var maxPerBat float64
	for _, e := range active {
		if c := capOf(e); c > 0 && (maxPerBat == 0 || c < maxPerBat) {
			maxPerBat = c
		}
	}

	selected = active

	if maxPerBat > 0 && snap.tiering {
		tierPerBat := maxPerBat * batteryTierFraction
		tier := &site.batteryChargeTier
		activeNames := &site.batteryChargeActive
		if !charge {
			tier = &site.batteryDischargeTier
			activeNames = &site.batteryDischargeActive
		}
		*tier = computeTier(target, tierPerBat, *tier, len(active))
		needed := *tier

		if needed < len(active) {
			var stop []fastEntry
			selected, stop = site.selectSticky(active, needed, charge, activeNames)
			toStop = append(toStop, stop...)
		} else {
			*activeNames = nil
		}
	} else if maxPerBat == 0 {
		share := target / float64(len(active))
		if share < minEffectiveShare && len(active) > 1 {
			best := 0
			for i, e := range active {
				if charge {
					if e.socOK && (i == 0 || e.soc < active[best].soc) {
						best = i
					}
				} else if e.socOK && (i == 0 || e.soc > active[best].soc) {
					best = i
				}
			}
			for i, e := range active {
				if i != best {
					toStop = append(toStop, e)
				}
			}
			selected = active[best : best+1]
		}
	}
	return
}

Then fastControl becomes a thin pipeline:

func (site *Site) fastControl(snap *batterySnapshot, dir batteryPlanDirection, target float64) {
	active, ineligible := site.filterEligible(snap, dir)

	deferStop := append([]fastEntry{}, ineligible...)
	if len(active) == 0 {
		site.stopBatteries(fastEntriesAll(snap))
		return
	}

	selected, moreStop := site.applyTieringAndSticky(snap, dir, target, active)
	deferStop = append(deferStop, moreStop...)

	charge := dir == batteryPlanCharge
	commands := site.computeCommands(snap, selected, dir, target)
	site.sendBatteries(selected, charge, commands)

	var total float64
	for _, c := range commands {
		total += c
	}
	batteryLog.DEBUG.Printf("solar power (fast): %s %.0fW across %d/%d batteries (grid target %.0fW)",
		batteryPlanDirectionString(dir), total, len(selected), len(snap.batteries), target)

	site.stopBatteries(deferStop)
}

computeCommands can hold the cap + taper logic you already have, but in a small, self‑contained function.

This keeps all existing semantics (tiering, sticky, tapering, stop‑guard), but breaks them into named steps aligned with the reviewer’s suggested responsibilities (filterEligible, applyTieringAndSticky, computeCommands), which reduces branching in each function and makes the module easier to reason about and test.

[sourcery-ai]

issue (complexity): Consider encapsulating the new battery solar/fast-loop configuration and state into a dedicated controller struct that Site delegates to, instead of expanding Site with many loosely related fields and behaviors.

You can reduce the added complexity without changing behavior by encapsulating the new battery‑solar/fast‑loop state into a dedicated struct and letting Site delegate to it.

1. Group the new fields into a controller struct

Instead of adding all battery‑solar/fast‑loop fields directly on Site, move them into a dedicated type:

type BatterySolarController struct {
	// configuration
	solarControl      bool
	calibrationCharge bool
	controlDeadBand   float64
	solarPool         bool
	solarTiering      bool
	solarSticky       bool
	solarTapering     bool

	// tiered / sticky state
	chargeTier      int
	dischargeTier   int
	chargeActive    []string
	dischargeActive []string
	stopped         map[string]int

	// snapshot / fast‑loop contract
	mu        sync.Mutex
	snapshot  *batterySnapshot
	direction batteryPlanDirection

	// fast‑loop metering guard
	lastGrid   float64
	lastBatt   float64
	guardValid bool

	// flip timing / backoff
	flipSince            time.Time
	lastFlipRequest      time.Time
	flipBackoff          time.Duration
}

Attach it to Site:

type Site struct {
	// ...

	prioritySoc             float64
	bufferSoc               float64
	bufferStartSoc          float64
	batteryDischargeControl bool
	batteryGridChargeLimit  *float64

	batterySolar *BatterySolarController

	// cached state ...
}

This keeps the god‑struct surface area down and makes the conceptual boundary of “battery solar control” obvious.

2. Delegate settings load/restore into the controller

Move the solar‑related settings wiring out of Site.restoreSettings into a method on BatterySolarController. The Site owns the controller but doesn’t need to know about all flags:

func (c *BatterySolarController) LoadSettings(settings *util.Settings) error {
	// defaults
	c.solarPool = false
	c.solarTiering = true
	c.solarSticky = true
	c.solarTapering = true

	if v, err := settings.Bool(keys.BatterySolarControl); err == nil {
		c.solarControl = v
	}
	if v, err := settings.Float(keys.BatteryControlDeadBand); err == nil {
		c.controlDeadBand = v
	}
	if v, err := settings.Bool(keys.BatterySolarPool); err == nil {
		c.solarPool = v
	}
	if v, err := settings.Bool(keys.BatterySolarTiering); err == nil {
		c.solarTiering = v
	}
	if v, err := settings.Bool(keys.BatterySolarSticky); err == nil {
		c.solarSticky = v
	}
	if v, err := settings.Bool(keys.BatterySolarTapering); err == nil {
		c.solarTapering = v
	}

	return nil
}

Site.restoreSettings then becomes simpler:

func (site *Site) restoreSettings() error {
	// existing non‑battery settings...

	if err := site.batterySolar.LoadSettings(settings); err != nil {
		return err
	}

	// rest of method...
	return nil
}

3. Delegate publishing of solar‑related state

Similarly, move the publish calls into the controller, so prepare doesn’t need to know every flag:

func (c *BatterySolarController) Publish(publish func(key string, val any)) {
	publish(keys.BatterySolarControl, c.solarControl)
	publish(keys.BatteryCalibrationCharge, c.calibrationCharge)
	publish(keys.BatteryControlDeadBand, c.controlDeadBand)
	publish(keys.BatterySolarPool, c.solarPool)
	publish(keys.BatterySolarTiering, c.solarTiering)
	publish(keys.BatterySolarSticky, c.solarSticky)
	publish(keys.BatterySolarTapering, c.solarTapering)
}

Then in Site.prepare:

func (site *Site) prepare() {
	if err := site.restoreSettings(); err != nil {
		site.log.ERROR.Println(err)
	}

	// other publishes...
	site.batterySolar.Publish(site.publish)
}

4. Encapsulate fast‑loop direction / flip logic

The fast‑loop‑specific fields and logic (batteryFastDirection, batteryLastGrid, batteryLastBatt, batteryGuardValid, batteryFlipSince, lastBatteryFlipRequest, batteryFlipBackoff) can live behind methods on BatterySolarController so Site.updateBatteryMode and batteryFastLoop just call into it:

func (c *BatterySolarController) Plan(
	now time.Time,
	sitePower float64,
	sitePowerValid bool,
	batt types.BatteryState,
) batteryPlan {
	c.mu.Lock()
	defer c.mu.Unlock()

	// use c.direction, c.lastGrid, c.flipSince, c.lastFlipRequest, c.flipBackoff, etc.
	// to compute the next plan, respecting dead band and backoff

	return plan
}

Then adjust Site.updateBatteryMode to delegate:

func (site *Site) updateBatteryMode(
	gridChargeActive bool,
	rate *planner.Rate,
	sitePower float64,
	sitePowerValid bool,
) {
	if !site.batterySolar.solarControl {
		// existing non‑solar logic
		return
	}

	plan := site.batterySolar.Plan(time.Now(), sitePower, sitePowerValid, site.battery)

	// apply plan to underlying batteries / modes (unchanged)
}

This preserves all behavior but:

• keeps Site focused on orchestration and wiring,
• makes the battery solar controller a clear, testable unit,
• groups tightly coupled flags/state so you don’t have to reason about them scattered across Site.

[sourcery-ai]

issue (complexity): Consider extracting small helper functions from buildBatterySnapshot and separating snapshot updates from mode decisions to make the battery control flow easier to scan and maintain without changing behavior.

You can reduce the perceived complexity without changing behavior by factoring out a few narrowly‑scoped helpers and loosening the coupling between mode logic and snapshot building.

1. Split buildBatterySnapshot into small helpers

Right now buildBatterySnapshot handles:

• EV power aggregation
• Calibration charge completion
• Residual/offset calculations
• Snapshot assembly
• Per‑battery capability discovery

You can keep the same behavior but move each concern into a helper with a clear contract. That makes the main function easier to scan and reason about.

Example (shortened for illustration only):

func (site *Site) buildBatterySnapshot(rate api.Rate) {
	evPower, evPowerFast := site.computeEvPowers()

	site.maybeFinishCalibrationCharge()

	chargeOffset, dischargeOffset, dischargeEvExcluded := site.computeBatteryOffsets(rate, evPower, evPowerFast)

	snap := &batterySnapshot{
		enabled:             true,
		pool:                site.batterySolarPool,
		tiering:             site.batterySolarTiering,
		sticky:              site.batterySolarSticky,
		tapering:            site.batterySolarTapering,
		calibration:         site.batteryCalibrationCharge,
		chargeOffset:        chargeOffset,
		dischargeOffset:     dischargeOffset,
		dischargeEvExcluded: dischargeEvExcluded,
		threshold:           standbyPower + site.batteryControlDeadBand,
		created:             time.Now(),
	}

	site.fillBatteryEntries(snap)

	site.batteryPlanMu.Lock()
	site.batterySnapshot = snap
	site.batteryPlanMu.Unlock()

	batteryLog.TRACE.Printf(
		"battery snapshot: %d batteries soc=%.0f%% chargeOff=%.0fW dischargeOff=%.0fW evExcl=%.0fW threshold=%.0fW",
		len(snap.batteries), site.battery.Soc, chargeOffset, dischargeOffset, dischargeEvExcluded, snap.threshold,
	)
}

Helpers (kept short and focused):

func (site *Site) computeEvPowers() (evPower, evPowerFast float64) {
	for _, lp := range site.loadpoints {
		if lp.IsHeating() {
			continue
		}
		p := lp.GetChargePower()
		evPower += p
		if lp.GetStatus() != api.StatusA && lp.IsFastChargingActive() {
			evPowerFast += p
		}
	}
	return
}

func (site *Site) maybeFinishCalibrationCharge() {
	if site.batteryCalibrationCharge && site.battery.Soc >= 100 {
		site.Lock()
		site.batteryCalibrationCharge = false
		site.Unlock()
		site.publish(keys.BatteryCalibrationCharge, false)
		batteryLog.DEBUG.Printf("battery calibration charge complete (soc %.0f%%)", site.battery.Soc)
	}
}

func (site *Site) computeBatteryOffsets(
	rate api.Rate, evPower, evPowerFast float64,
) (chargeOffset, dischargeOffset, dischargeEvExcluded float64) {
	residual := site.GetResidualPower()

	if site.battery.Soc >= site.prioritySoc {
		chargeOffset = residual
	}
	if site.dischargeControlActive(rate) {
		dischargeEvExcluded = evPowerFast
	} else if !(site.bufferSoc > 0 && site.battery.Soc > site.bufferSoc) {
		dischargeEvExcluded = evPower
	}
	return chargeOffset, residual, dischargeEvExcluded
}

func (site *Site) fillBatteryEntries(snap *batterySnapshot) {
	for _, dev := range site.batteryMeters {
		ctrl, ok := api.Cap[api.BatteryPowerController](dev.Instance())
		if !ok {
			continue
		}
		e := batterySnapEntry{
			ctrl:  ctrl,
			meter: dev.Instance(),
			name:  dev.Config().Name,
		}
		if bat, ok := api.Cap[api.Battery](dev.Instance()); ok {
			if soc, err := bat.Soc(); err == nil {
				e.soc, e.socOK = soc, true
			}
		}
		if limiter, ok := api.Cap[api.BatterySocLimiter](dev.Instance()); ok {
			e.hasSocLimit = true
			e.minSoc, e.maxSoc = limiter.GetSocLimits()
		}
		if limiter, ok := api.Cap[api.BatteryPowerLimiter](dev.Instance()); ok {
			e.chargeCap, e.dischargeCap = limiter.GetPowerLimits()
		}
		snap.batteries = append(snap.batteries, e)
	}
}

This keeps all logic in the same place but makes each step self‑documenting and individually testable.

2. Decouple mode decisions from snapshot updates

updateBatteryMode currently decides the mode and also manages the snapshot for the fast loop. To reduce implicit coupling, you can keep the behavior but make the separation explicit: one function decides modes, another updates the snapshot.

Minimal change example:

func (site *Site) updateBatteryMode(batteryGridChargeActive bool, rate api.Rate, sitePower float64, sitePowerValid bool) {
	batteryMode := site.requiredBatteryMode(batteryGridChargeActive, rate, sitePower)

	// existing HEMS dimming + applyBatteryMode logic stays as-is...
	// ...

	site.updateBatterySnapshot(rate)
}

func (site *Site) updateBatterySnapshot(rate api.Rate) {
	if !site.batterySolarControl {
		site.batteryPlanMu.Lock()
		site.batterySnapshot = nil
		site.batteryPlanMu.Unlock()
		return
	}
	site.buildBatterySnapshot(rate)
}

This keeps all current behavior (including when the snapshot is built/cleared) but makes the file’s responsibilities clearer and prepares for moving snapshot/selection concerns into a dedicated controller later if needed.