EEBus: fix "out of sync" warning after disabling charger

[DerAndereAndi]

OpEV and OSCEV limits were written in two separate calls. Since the pinned eebus-go version does not use partial writes, the second call shipped a stale full limit list and silently clobbered the OpEV values the first call had just installed, leaving the charger reporting enabled after a disable.

Dispatch both categories through a single LoadControl.WriteLimitData call to avoid the clobber. OSCEV cannot be dropped — some wallbox/EV combinations require the recommendation to charge.

[sourcery-ai]

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

• The opevFilter and oscevFilter structs are duplicated from eebus-go and must be kept in sync; consider either referencing shared constants upstream (if possible) or at least centralizing these in a single place in this repo to avoid subtle divergence over time.
• Exposing CustomerEnergyManagement.LocalEntity as a public field broadens the API surface; if it’s only needed for building SPINE clients in a few places, consider providing a small accessor method or a narrower interface to avoid leaking the full entity into downstream callers.

Prompt for AI Agents

Please address the comments from this code review:

## Overall Comments
- The `opevFilter` and `oscevFilter` structs are duplicated from eebus-go and must be kept in sync; consider either referencing shared constants upstream (if possible) or at least centralizing these in a single place in this repo to avoid subtle divergence over time.
- Exposing `CustomerEnergyManagement.LocalEntity` as a public field broadens the API surface; if it’s only needed for building SPINE clients in a few places, consider providing a small accessor method or a narrower interface to avoid leaking the full entity into downstream callers.

## Individual Comments

### Comment 1
<location path="charger/eebus.go" line_range="312" />
<code_context>

-	// setup the obligation limit data structure
-	var limits []ucapi.LoadLimitsPhase
-	for phase := range len(ucapi.PhaseNameMapping) {
-		limit := ucapi.LoadLimitsPhase{
-			Phase:    ucapi.PhaseNameMapping[phase],
</code_context>
<issue_to_address>
**issue (bug_risk):** The range clause over `len(ucapi.PhaseNameMapping)` is invalid and will not compile.

`range` works on slices/maps, not on `int`. Iterate over `ucapi.PhaseNameMapping` directly, or use an index loop, e.g.:

```go
for phase := range ucapi.PhaseNameMapping { ... }
// or
for phase := 0; phase < len(ucapi.PhaseNameMapping); phase++ { ... }
```

Apply the same fix in `computeOscevLimits`, where the same pattern appears.
</issue_to_address>

### Comment 2
<location path="charger/eebus.go" line_range="455-464" />
<code_context>
+		paramFilter := model.ElectricalConnectionParameterDescriptionDataType{
+			AcMeasuredPhases: lo.ToPtr(phaseLimit.Phase),
+		}
+		elParamDesc, err := elConn.GetParameterDescriptionsForFilter(paramFilter)
+		if err != nil || len(elParamDesc) == 0 || elParamDesc[0].MeasurementId == nil {
+			continue
+		}
+
+		var limitDesc *model.LoadControlLimitDescriptionDataType
+		for _, desc := range limitDescriptions {
+			if desc.MeasurementId != nil && *desc.MeasurementId == *elParamDesc[0].MeasurementId {
+				safe := desc
+				limitDesc = &safe
+				break
+			}
+		}
+		if limitDesc == nil || limitDesc.LimitId == nil {
+			continue
+		}
+
+		limitIdData, err := loadControl.GetLimitDataForId(*limitDesc.LimitId)
+		if err != nil {
+			continue
+		}
+		if limitIdData.IsLimitChangeable != nil && !*limitIdData.IsLimitChangeable {
+			continue
+		}
+
+		value := elConn.AdjustValueToBeWithinPermittedValuesForParameterId(
+			phaseLimit.Value, *elParamDesc[0].ParameterId)

</code_context>
<issue_to_address>
**issue:** Possible nil dereference for `elParamDesc[0].ParameterId` when adjusting the value.

`MeasurementId` is checked for nil, but `ParameterId` is not before dereferencing in:

```go
value := elConn.AdjustValueToBeWithinPermittedValuesForParameterId(
    phaseLimit.Value, *elParamDesc[0].ParameterId)
```

If the remote omits `ParameterId`, this will panic. Please extend the prior condition to also verify `len(elParamDesc) > 0 && elParamDesc[0].ParameterId != nil` and `continue` if it’s missing.
</issue_to_address>

### Comment 3
<location path="charger/eebus.go" line_range="336" />
<code_context>
+// is therefore to bypass the use-case wrappers and dispatch both categories
+// through a single loadControl.WriteLimitData call — one merge against the
+// cache, one wire message, no clobbering between categories.
 func (c *EEBus) writeCurrentLimitData(evEntity spineapi.EntityRemoteInterface, current float64) error {
-	// check if the EVSE supports overload protection limits
+	// OpEV obligation-max is the required baseline for any write
</code_context>
<issue_to_address>
**issue (complexity):** Consider refactoring the new limit-writing logic into small helper functions and shared utilities so `writeCurrentLimitData` and `buildPhaseLimitData` read as short, linear orchestration code.

```markdown
`writeCurrentLimitData` and `buildPhaseLimitData` did get a fair bit more complex. You can keep all behavior and atomicity while making the flow easier to follow with a couple of small, local helpers.

### 1. Split `writeCurrentLimitData` orchestration

Right now `writeCurrentLimitData` does: scenario checks, client construction, OpEV limit computation, OSCEV limit computation, SPINE data building, and the write.

You can keep the atomic write but push OpEV/OSCEV branches into helpers. That keeps the main function as a short “script”:

```go
func (c *EEBus) writeCurrentLimitData(evEntity spineapi.EntityRemoteInterface, current float64) error {
	if !c.cem.OpEV.IsScenarioAvailableAtEntity(evEntity, 1) {
		return api.ErrNotAvailable
	}

	loadControl, err := client.NewLoadControl(c.cem.LocalEntity, evEntity)
	if err != nil {
		return api.ErrNotAvailable
	}
	elConn, err := client.NewElectricalConnection(c.cem.LocalEntity, evEntity)
	if err != nil {
		return api.ErrNotAvailable
	}

	var data []model.LoadControlLimitDataType

	if entries := c.buildOpevData(loadControl, elConn, evEntity, current); len(entries) > 0 {
		data = append(data, entries...)
	}
	if entries := c.buildOscevData(loadControl, elConn, evEntity, current); len(entries) > 0 {
		data = append(data, entries...)
	}
	if len(data) == 0 {
		return api.ErrNotAvailable
	}

	if _, err := loadControl.WriteLimitData(data, nil, nil); err != nil {
		return err
	}

	c.mux.Lock()
	c.limitUpdated = time.Now()
	c.mux.Unlock()
	return nil
}

func (c *EEBus) buildOpevData(
	loadControl eebusapi.LoadControlCommonInterface,
	elConn eebusapi.ElectricalConnectionCommonInterface,
	evEntity spineapi.EntityRemoteInterface,
	current float64,
) []model.LoadControlLimitDataType {
	_, maxLimits, _, err := c.cem.OpEV.CurrentLimits(evEntity)
	if err != nil {
		c.log.DEBUG.Println("no limits from the EVSE are provided:", err)
	}
	limits := computeOpevLimits(current, maxLimits)
	return buildPhaseLimitData(loadControl, elConn, opevFilter, limits)
}

func (c *EEBus) buildOscevData(
	loadControl eebusapi.LoadControlCommonInterface,
	elConn eebusapi.ElectricalConnectionCommonInterface,
	evEntity spineapi.EntityRemoteInterface,
	current float64,
) []model.LoadControlLimitDataType {
	if !c.cem.OscEV.IsScenarioAvailableAtEntity(evEntity, 1) {
		return nil
	}
	if _, err := c.cem.OscEV.LoadControlLimits(evEntity); err != nil {
		return nil
	}
	minLimits, _, _, err := c.cem.OscEV.CurrentLimits(evEntity)
	if err != nil {
		return nil
	}
	limits := computeOscevLimits(current, minLimits)
	return buildPhaseLimitData(loadControl, elConn, oscevFilter, limits)
}
```

This keeps the atomic write logic and semantics intact, but the high‑level flow is immediately obvious.

### 2. Deduplicate phase iteration

Both `computeOpevLimits` and `computeOscevLimits` iterate the same way over `ucapi.PhaseNameMapping`. A tiny helper keeps them in sync and cuts repetition:

```go
func forEachPhase(fn func(idx int, phase model.ElectricalConnectionPhaseNameType)) {
	for i, name := range ucapi.PhaseNameMapping {
		fn(i, name)
	}
}

func computeOpevLimits(current float64, maxLimits []float64) []ucapi.LoadLimitsPhase {
	limits := make([]ucapi.LoadLimitsPhase, 0, len(ucapi.PhaseNameMapping))
	forEachPhase(func(idx int, phase model.ElectricalConnectionPhaseNameType) {
		limit := ucapi.LoadLimitsPhase{
			Phase:    phase,
			IsActive: true,
			Value:    current,
		}
		if idx < len(maxLimits) && current >= maxLimits[idx] {
			limit.IsActive = false
		}
		limits = append(limits, limit)
	})
	return limits
}

func computeOscevLimits(current float64, minLimits []float64) []ucapi.LoadLimitsPhase {
	limits := make([]ucapi.LoadLimitsPhase, 0, len(ucapi.PhaseNameMapping))
	forEachPhase(func(idx int, phase model.ElectricalConnectionPhaseNameType) {
		limit := ucapi.LoadLimitsPhase{
			Phase:    phase,
			IsActive: false,
			Value:    current,
		}
		if idx < len(minLimits) {
			limit.IsActive = current >= minLimits[idx]
		}
		limits = append(limits, limit)
	})
	return limits
}
```

### 3. Flatten `buildPhaseLimitData`’s inner loop

The current loop mixes description lookup, changeability checks, and value adjustment inline. Small, local helpers make the happy path clearer without changing behavior.

```go
func buildPhaseLimitData(
	loadControl eebusapi.LoadControlCommonInterface,
	elConn eebusapi.ElectricalConnectionCommonInterface,
	filter model.LoadControlLimitDescriptionDataType,
	limits []ucapi.LoadLimitsPhase,
) []model.LoadControlLimitDataType {
	limitDescriptions, err := loadControl.GetLimitDescriptionsForFilter(filter)
	if err != nil || len(limitDescriptions) == 0 {
		return nil
	}

	var data []model.LoadControlLimitDataType
	for _, phaseLimit := range limits {
		desc := findLimitDescriptionForPhase(loadControl, elConn, limitDescriptions, phaseLimit)
		if desc == nil || desc.LimitId == nil {
			continue
		}

		entry, ok := buildLimitDataEntry(loadControl, elConn, phaseLimit, desc)
		if !ok {
			continue
		}
		data = append(data, entry)
	}
	return data
}

func findLimitDescriptionForPhase(
	loadControl eebusapi.LoadControlCommonInterface,
	elConn eebusapi.ElectricalConnectionCommonInterface,
	limitDescriptions []model.LoadControlLimitDescriptionDataType,
	phaseLimit ucapi.LoadLimitsPhase,
) *model.LoadControlLimitDescriptionDataType {
	paramFilter := model.ElectricalConnectionParameterDescriptionDataType{
		AcMeasuredPhases: lo.ToPtr(phaseLimit.Phase),
	}
	elParamDesc, err := elConn.GetParameterDescriptionsForFilter(paramFilter)
	if err != nil || len(elParamDesc) == 0 || elParamDesc[0].MeasurementId == nil {
		return nil
	}
	measurementID := *elParamDesc[0].MeasurementId

	for _, desc := range limitDescriptions {
		if desc.MeasurementId != nil && *desc.MeasurementId == measurementID {
			d := desc
			return &d
		}
	}
	return nil
}

func buildLimitDataEntry(
	loadControl eebusapi.LoadControlCommonInterface,
	elConn eebusapi.ElectricalConnectionCommonInterface,
	phaseLimit ucapi.LoadLimitsPhase,
	limitDesc *model.LoadControlLimitDescriptionDataType,
) (model.LoadControlLimitDataType, bool) {
	limitIdData, err := loadControl.GetLimitDataForId(*limitDesc.LimitId)
	if err != nil {
		return model.LoadControlLimitDataType{}, false
	}
	if limitIdData.IsLimitChangeable != nil && !*limitIdData.IsLimitChangeable {
		return model.LoadControlLimitDataType{}, false
	}

	value := elConn.AdjustValueToBeWithinPermittedValuesForParameterId(
		phaseLimit.Value, *limitIdData.ParameterId,
	)

	return model.LoadControlLimitDataType{
		LimitId:       limitDesc.LimitId,
		IsLimitActive: lo.ToPtr(phaseLimit.IsActive),
		Value:         model.NewScaledNumberType(value),
	}, true
}
```

This keeps all SPINE mapping and policy the same, but the main function reads as “get descriptions → per phase: resolve description → build entry” with the nested checks pushed into well-named helpers.

### 4. Group filter configuration

To visually separate configuration from behavior, you can group the two filters in a small struct or dedicated helper file. Even something minimal like:

```go
type limitFilters struct {
	opev  model.LoadControlLimitDescriptionDataType
	oscev model.LoadControlLimitDescriptionDataType
}

var filters = limitFilters{
	opev:  opevFilter,
	oscev: oscevFilter,
}
```

or moving the existing `opevFilter` / `oscevFilter` into an `eebus_limits.go` file keeps `charger/eebus.go` more focused on business logic while retaining the explicit filter definitions you need to keep in sync with `eebus-go`.
```
</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.}

[DerAndereAndi]

This is tested locally, working and fixing the mentioned issues.

[andig]

Afaiu this is not obvious to library consumers. Has this always been the case or was it introduced as an effect of #28761? Then- rather than adding more code to patch this- we could revert that PR and reapply when we can upgrade the library?

[DerAndereAndi]

This has always been the case in the version evcc is using it. Partial support is a major PITA to implement and wasn't done at that time.

This is still better than before the patch that combined it. I would recommend to incorporate this for now.