From a0c41394d3ce846c2a8b750b1f55e3035c9744e1 Mon Sep 17 00:00:00 2001 From: Andreas Linde Date: Thu, 9 Apr 2026 20:07:53 +0200 Subject: [PATCH 1/3] EEBus: fix "out of sync" warning after disabling charger MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- charger/eebus.go | 201 ++++++++++++++++++++++++++++++++---------- charger/eebus_test.go | 194 +++++++++++++++------------------------- server/eebus/eebus.go | 18 ++-- 3 files changed, 237 insertions(+), 176 deletions(-) diff --git a/charger/eebus.go b/charger/eebus.go index 269cdac8491..178bc9124e7 100644 --- a/charger/eebus.go +++ b/charger/eebus.go @@ -8,10 +8,12 @@ import ( "time" eebusapi "github.com/enbility/eebus-go/api" + "github.com/enbility/eebus-go/features/client" ucapi "github.com/enbility/eebus-go/usecases/api" "github.com/enbility/eebus-go/usecases/cem/evcc" "github.com/enbility/eebus-go/usecases/cem/evcem" spineapi "github.com/enbility/spine-go/api" + "github.com/enbility/spine-go/model" "github.com/evcc-io/evcc/api" "github.com/evcc-io/evcc/core/loadpoint" "github.com/evcc-io/evcc/server/eebus" @@ -295,89 +297,196 @@ func (c *EEBus) Enable(enable bool) error { return err } -// send current charging power limits to the EV +// opevFilter matches the OpEV obligation-max overload-protection limit descriptions. +// Must stay in sync with eebus-go usecases/cem/opev/public.go WriteLoadControlLimits. +var opevFilter = model.LoadControlLimitDescriptionDataType{ + LimitType: lo.ToPtr(model.LoadControlLimitTypeTypeMaxValueLimit), + LimitCategory: lo.ToPtr(model.LoadControlCategoryTypeObligation), + Unit: lo.ToPtr(model.UnitOfMeasurementTypeA), + ScopeType: lo.ToPtr(model.ScopeTypeTypeOverloadProtection), +} + +// oscevFilter matches the OSCEV recommendation-max self-consumption limit descriptions. +// Must stay in sync with eebus-go usecases/cem/oscev/public.go WriteLoadControlLimits. +var oscevFilter = model.LoadControlLimitDescriptionDataType{ + LimitType: lo.ToPtr(model.LoadControlLimitTypeTypeMaxValueLimit), + LimitCategory: lo.ToPtr(model.LoadControlCategoryTypeRecommendation), + Unit: lo.ToPtr(model.UnitOfMeasurementTypeA), + ScopeType: lo.ToPtr(model.ScopeTypeTypeSelfConsumption), +} + +// writeCurrentLimitData writes OpEV obligation and (if available) OSCEV +// recommendation limits to the EV in a single atomic spine-level write. +// +// Background: the currently pinned eebus-go version does not send SPINE +// partial writes — every LoadControl update has to ship the full limit list. +// If we write OpEV and OSCEV separately (as the CemOPEV/CemOSCEV use-case +// wrappers do), the second write effectively deletes the OpEV obligation +// limits the first write just installed, because both writes build their +// "full list" from the same stale feature cache. The symptom on a Porsche +// Taycan disable cycle is a persistent "charger out of sync: expected +// disabled, got enabled" warning (see log.txt on the feature/eebus-fixes +// branch for a captured trace). +// +// We cannot simply drop OSCEV: some wallbox + EV combinations will not +// charge at all unless a self-consumption recommendation is active. The fix +// 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 if !c.cem.OpEV.IsScenarioAvailableAtEntity(evEntity, 1) { return api.ErrNotAvailable } - _, maxLimits, _, err := c.cem.OpEV.CurrentLimits(evEntity) + loadControl, err := client.NewLoadControl(c.cem.LocalEntity, evEntity) if err != nil { - c.log.DEBUG.Println("no limits from the EVSE are provided:", err) + return api.ErrNotAvailable + } + elConn, err := client.NewElectricalConnection(c.cem.LocalEntity, evEntity) + if err != nil { + return api.ErrNotAvailable } - // setup the obligation limit data structure - var limits []ucapi.LoadLimitsPhase - for phase := range len(ucapi.PhaseNameMapping) { - limit := ucapi.LoadLimitsPhase{ - Phase: ucapi.PhaseNameMapping[phase], - IsActive: true, - Value: current, - } + var data []model.LoadControlLimitDataType - // if the limit equals to the max allowed, then the obligation limit is actually inactive - if phase < len(maxLimits) && current >= maxLimits[phase] { - limit.IsActive = false + // OpEV: obligation max — active unless the requested current meets or + // exceeds the phase max (in which case "no obligation" = unlimited). + _, maxLimits, _, cerr := c.cem.OpEV.CurrentLimits(evEntity) + if cerr != nil { + c.log.DEBUG.Println("no limits from the EVSE are provided:", cerr) + } + if entries := buildPhaseLimitData(loadControl, elConn, opevFilter, computeOpevLimits(current, maxLimits)); entries != nil { + data = append(data, entries...) + } + + // OSCEV: recommendation max — optional. Active only when the requested + // current is at least the phase min (a recommendation below min cannot + // drive charging and is equivalent to no recommendation). + if c.cem.OscEV.IsScenarioAvailableAtEntity(evEntity, 1) { + if _, lerr := c.cem.OscEV.LoadControlLimits(evEntity); lerr == nil { + if minLimits, _, _, merr := c.cem.OscEV.CurrentLimits(evEntity); merr == nil { + if entries := buildPhaseLimitData(loadControl, elConn, oscevFilter, computeOscevLimits(current, minLimits)); entries != nil { + data = append(data, entries...) + } + } } + } - limits = append(limits, limit) + if len(data) == 0 { + return api.ErrNotAvailable } - // always set overload protection limits (obligation) - if _, err := c.cem.OpEV.WriteLoadControlLimits(evEntity, limits, nil); err != nil { + if _, err := loadControl.WriteLimitData(data, nil, nil); err != nil { return err } - // additionally set self-consumption recommendation limits if available - c.writeOscevLimits(evEntity, current) - c.mux.Lock() defer c.mux.Unlock() - c.limitUpdated = time.Now() - return nil } -// writeOscevLimits writes OSCEV recommendation limits if the use case is available. -// An active recommendation triggers the EV to charge with surplus energy. -// An inactive recommendation is equivalent to no recommendation existing. -func (c *EEBus) writeOscevLimits(evEntity spineapi.EntityRemoteInterface, current float64) { - if !c.cem.OscEV.IsScenarioAvailableAtEntity(evEntity, 1) { - return - } - - // OSCEV requires recommendation limits to be available - if _, err := c.cem.OscEV.LoadControlLimits(evEntity); err != nil { - return - } - - minLimits, _, _, err := c.cem.OscEV.CurrentLimits(evEntity) - if err != nil { - return +// computeOpevLimits returns the per-phase OpEV obligation-max tuples for the +// given current. IsActive is false when the current equals or exceeds the +// phase max (no obligation = unlimited per EEBus semantics). +func computeOpevLimits(current float64, maxLimits []float64) []ucapi.LoadLimitsPhase { + limits := make([]ucapi.LoadLimitsPhase, 0, len(ucapi.PhaseNameMapping)) + for phase := range len(ucapi.PhaseNameMapping) { + limit := ucapi.LoadLimitsPhase{ + Phase: ucapi.PhaseNameMapping[phase], + IsActive: true, + Value: current, + } + if phase < len(maxLimits) && current >= maxLimits[phase] { + limit.IsActive = false + } + limits = append(limits, limit) } + return limits +} - var limits []ucapi.LoadLimitsPhase +// computeOscevLimits returns the per-phase OSCEV recommendation-max tuples for +// the given current. Unlike OpEV, the recommendation is active only when the +// current is at least the phase min — below min there is nothing useful to +// recommend and the limit must be inactive to be ignored by the EV. +func computeOscevLimits(current float64, minLimits []float64) []ucapi.LoadLimitsPhase { + limits := make([]ucapi.LoadLimitsPhase, 0, len(ucapi.PhaseNameMapping)) for phase := range len(ucapi.PhaseNameMapping) { limit := ucapi.LoadLimitsPhase{ Phase: ucapi.PhaseNameMapping[phase], IsActive: false, Value: current, } - - // below min charging current there is nothing to recommend - // in contrast to OPEV the max value has to be active to trigger the recommendation to have any effect if phase < len(minLimits) { limit.IsActive = current >= minLimits[phase] } - limits = append(limits, limit) } + return limits +} + +// buildPhaseLimitData converts per-phase LoadLimitsPhase tuples into spine +// LoadControlLimitDataType entries for a given description filter. It mirrors +// the per-phase mapping inside eebus-go usecases/internal/loadcontrol.go +// WriteLoadControlPhaseLimits but does not dispatch a write — callers may +// accumulate entries across multiple filters and issue a single +// loadControl.WriteLimitData for atomicity. +// +// Returns nil if the filter does not match any limit description on the +// remote (e.g. the use case is not supported); individual phases are skipped +// if their specific description or parameter data is missing. +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 { + 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) - if _, err := c.cem.OscEV.WriteLoadControlLimits(evEntity, limits, nil); err != nil { - c.log.DEBUG.Println("failed to write OSCEV limits:", err) + data = append(data, model.LoadControlLimitDataType{ + LimitId: limitDesc.LimitId, + IsLimitActive: lo.ToPtr(phaseLimit.IsActive), + Value: model.NewScaledNumberType(value), + }) } + return data } // MaxCurrent implements the api.Charger interface diff --git a/charger/eebus_test.go b/charger/eebus_test.go index 434df0375c9..aa92cadeff0 100644 --- a/charger/eebus_test.go +++ b/charger/eebus_test.go @@ -106,140 +106,86 @@ func TestEEBusNoCurrents(t *testing.T) { assert.Equal(t, 10.4, l3) } -// newTestEEBus creates an EEBus instance with all mocks wired up for limit writing tests. -func newTestEEBus(t *testing.T) (*EEBus, *mocks.CemOPEVInterface, *mocks.CemOSCEVInterface, *spinemocks.EntityRemoteInterface) { - t.Helper() - - opev := mocks.NewCemOPEVInterface(t) - oscev := mocks.NewCemOSCEVInterface(t) - evEntity := spinemocks.NewEntityRemoteInterface(t) - - eebus := &EEBus{ - cem: &eebus.CustomerEnergyManagement{ - OpEV: opev, - OscEV: oscev, - }, - ev: evEntity, - log: util.NewLogger("test"), - } - - return eebus, opev, oscev, evEntity -} - // 3-phase limits helper -func opevLimits3p(min, max, def float64) ([]float64, []float64, []float64, error) { - return []float64{min, min, min}, []float64{max, max, max}, []float64{def, def, def}, nil +func limits3p(v float64) []float64 { + return []float64{v, v, v} } -func TestWriteCurrentLimitData_OpevOnly(t *testing.T) { - eebus, opev, oscev, evEntity := newTestEEBus(t) - _ = eebus - - // OPEV available, OSCEV not available - opev.EXPECT().IsScenarioAvailableAtEntity(evEntity, uint(1)).Return(true) - opev.EXPECT().CurrentLimits(evEntity).Return(opevLimits3p(6, 16, 0)) - opev.EXPECT().WriteLoadControlLimits(evEntity, mock.MatchedBy(func(limits []ucapi.LoadLimitsPhase) bool { - return len(limits) == 3 && limits[0].IsActive && limits[0].Value == 10 - }), mock.Anything).Return(nil, nil) - - oscev.EXPECT().IsScenarioAvailableAtEntity(evEntity, uint(1)).Return(false) - - err := eebus.writeCurrentLimitData(evEntity, 10) - require.NoError(t, err) -} - -func TestWriteCurrentLimitData_OpevAndOscev(t *testing.T) { - eebus, opev, oscev, evEntity := newTestEEBus(t) - _ = eebus - - // Both available, current = 10A (between min and max) - opev.EXPECT().IsScenarioAvailableAtEntity(evEntity, uint(1)).Return(true) - opev.EXPECT().CurrentLimits(evEntity).Return(opevLimits3p(6, 16, 0)) - opev.EXPECT().WriteLoadControlLimits(evEntity, mock.MatchedBy(func(limits []ucapi.LoadLimitsPhase) bool { - // OPEV: active at 10A (below max of 16) - return len(limits) == 3 && limits[0].IsActive && limits[0].Value == 10 - }), mock.Anything).Return(nil, nil) - - oscev.EXPECT().IsScenarioAvailableAtEntity(evEntity, uint(1)).Return(true) - oscev.EXPECT().LoadControlLimits(evEntity).Return([]ucapi.LoadLimitsPhase{}, nil) - oscev.EXPECT().CurrentLimits(evEntity).Return(opevLimits3p(2, 16, 0)) - oscev.EXPECT().WriteLoadControlLimits(evEntity, mock.MatchedBy(func(limits []ucapi.LoadLimitsPhase) bool { - // OSCEV: active at 10A (>= min of 2, recommendation to charge) - return len(limits) == 3 && limits[0].IsActive && limits[0].Value == 10 - }), mock.Anything).Return(nil, nil) - - err := eebus.writeCurrentLimitData(evEntity, 10) - require.NoError(t, err) -} - -func TestWriteCurrentLimitData_AtMax(t *testing.T) { - eebus, opev, oscev, evEntity := newTestEEBus(t) - _ = eebus - - // Current equals max limit - opev.EXPECT().IsScenarioAvailableAtEntity(evEntity, uint(1)).Return(true) - opev.EXPECT().CurrentLimits(evEntity).Return(opevLimits3p(6, 16, 0)) - opev.EXPECT().WriteLoadControlLimits(evEntity, mock.MatchedBy(func(limits []ucapi.LoadLimitsPhase) bool { - // OPEV: inactive at max (no restriction needed) - return len(limits) == 3 && !limits[0].IsActive && limits[0].Value == 16 - }), mock.Anything).Return(nil, nil) - - oscev.EXPECT().IsScenarioAvailableAtEntity(evEntity, uint(1)).Return(true) - oscev.EXPECT().LoadControlLimits(evEntity).Return([]ucapi.LoadLimitsPhase{}, nil) - oscev.EXPECT().CurrentLimits(evEntity).Return(opevLimits3p(2, 16, 0)) - oscev.EXPECT().WriteLoadControlLimits(evEntity, mock.MatchedBy(func(limits []ucapi.LoadLimitsPhase) bool { - // OSCEV: active at 16A (>= min, recommend charging) - return len(limits) == 3 && limits[0].IsActive && limits[0].Value == 16 - }), mock.Anything).Return(nil, nil) - - err := eebus.writeCurrentLimitData(evEntity, 16) - require.NoError(t, err) -} +// TestComputeOpevLimits verifies the per-phase OpEV obligation-max tuples that +// writeCurrentLimitData ships through the combined-write path. OpEV is active +// for the given current unless it meets/exceeds the phase max (in which case +// the obligation becomes inactive, meaning "no cap"). +func TestComputeOpevLimits(t *testing.T) { + tests := []struct { + name string + current float64 + maxLimits []float64 + wantActive bool + wantValue float64 + }{ + {"mid-range", 10, limits3p(16), true, 10}, + {"at max", 16, limits3p(16), false, 16}, + {"above max", 20, limits3p(16), false, 20}, + {"disable", 0, limits3p(16), true, 0}, + {"no max limits", 10, nil, true, 10}, + } -func TestWriteCurrentLimitData_Disable(t *testing.T) { - eebus, opev, oscev, evEntity := newTestEEBus(t) - _ = eebus - - // Current = 0 (disable charging) - opev.EXPECT().IsScenarioAvailableAtEntity(evEntity, uint(1)).Return(true) - opev.EXPECT().CurrentLimits(evEntity).Return(opevLimits3p(6, 16, 0)) - opev.EXPECT().WriteLoadControlLimits(evEntity, mock.MatchedBy(func(limits []ucapi.LoadLimitsPhase) bool { - // OPEV: active at 0A (hard stop) - return len(limits) == 3 && limits[0].IsActive && limits[0].Value == 0 - }), mock.Anything).Return(nil, nil) - - oscev.EXPECT().IsScenarioAvailableAtEntity(evEntity, uint(1)).Return(true) - oscev.EXPECT().LoadControlLimits(evEntity).Return([]ucapi.LoadLimitsPhase{}, nil) - oscev.EXPECT().CurrentLimits(evEntity).Return(opevLimits3p(2, 16, 0)) - oscev.EXPECT().WriteLoadControlLimits(evEntity, mock.MatchedBy(func(limits []ucapi.LoadLimitsPhase) bool { - // OSCEV: inactive at 0A (no recommendation, < min) - return len(limits) == 3 && !limits[0].IsActive && limits[0].Value == 0 - }), mock.Anything).Return(nil, nil) - - err := eebus.writeCurrentLimitData(evEntity, 0) - require.NoError(t, err) + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + got := computeOpevLimits(tc.current, tc.maxLimits) + require.Len(t, got, 3) + for i, limit := range got { + assert.Equalf(t, tc.wantActive, limit.IsActive, "phase %d IsActive", i) + assert.Equalf(t, tc.wantValue, limit.Value, "phase %d Value", i) + } + }) + } } -func TestWriteCurrentLimitData_OscevNoLimitData(t *testing.T) { - eebus, opev, oscev, evEntity := newTestEEBus(t) - _ = eebus - - // OSCEV scenario available but no limit data (e.g. PCMP wallbox) - opev.EXPECT().IsScenarioAvailableAtEntity(evEntity, uint(1)).Return(true) - opev.EXPECT().CurrentLimits(evEntity).Return(opevLimits3p(6, 16, 0)) - opev.EXPECT().WriteLoadControlLimits(evEntity, mock.Anything, mock.Anything).Return(nil, nil) - - oscev.EXPECT().IsScenarioAvailableAtEntity(evEntity, uint(1)).Return(true) - oscev.EXPECT().LoadControlLimits(evEntity).Return(nil, errors.New("data not available")) - // no WriteLoadControlLimits call expected for OSCEV +// TestComputeOscevLimits verifies the per-phase OSCEV recommendation tuples. +// Unlike OpEV, OSCEV is only active when the current is at least the phase +// min — a recommendation below min cannot drive charging and must be inactive. +func TestComputeOscevLimits(t *testing.T) { + tests := []struct { + name string + current float64 + minLimits []float64 + wantActive bool + wantValue float64 + }{ + {"at min", 6, limits3p(6), true, 6}, + {"above min", 10, limits3p(6), true, 10}, + {"below min", 4, limits3p(6), false, 4}, + {"disable", 0, limits3p(6), false, 0}, + {"no min limits", 10, nil, false, 10}, + } - err := eebus.writeCurrentLimitData(evEntity, 10) - require.NoError(t, err) + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + got := computeOscevLimits(tc.current, tc.minLimits) + require.Len(t, got, 3) + for i, limit := range got { + assert.Equalf(t, tc.wantActive, limit.IsActive, "phase %d IsActive", i) + assert.Equalf(t, tc.wantValue, limit.Value, "phase %d Value", i) + } + }) + } } +// TestWriteCurrentLimitData_OpevNotAvailable verifies that writeCurrentLimitData +// returns ErrNotAvailable without touching any clients when OpEV scenario 1 is +// not supported on the target entity. func TestWriteCurrentLimitData_OpevNotAvailable(t *testing.T) { - eebus, opev, _, evEntity := newTestEEBus(t) - _ = eebus + opev := mocks.NewCemOPEVInterface(t) + evEntity := spinemocks.NewEntityRemoteInterface(t) + + eebus := &EEBus{ + cem: &eebus.CustomerEnergyManagement{ + OpEV: opev, + }, + ev: evEntity, + log: util.NewLogger("test"), + } opev.EXPECT().IsScenarioAvailableAtEntity(evEntity, uint(1)).Return(false) diff --git a/server/eebus/eebus.go b/server/eebus/eebus.go index 8cf5696a2c2..8b313e85f14 100644 --- a/server/eebus/eebus.go +++ b/server/eebus/eebus.go @@ -41,6 +41,11 @@ type Device interface { // Customer Energy Management type CustomerEnergyManagement struct { + // LocalEntity is the CEM entity that hosts the EV-facing use cases. It is + // exposed so downstream code can build spine-level clients (e.g. for atomic + // multi-category LoadControl writes that bypass the per-use-case wrappers). + LocalEntity spineapi.EntityLocalInterface + EvseCC ucapi.CemEVSECCInterface EvCC ucapi.CemEVCCInterface EvCem ucapi.CemEVCEMInterface @@ -156,12 +161,13 @@ func NewServer(other Config) (*EEBus, error) { // customer energy management to EVSE c.cem = CustomerEnergyManagement{ - EvseCC: evsecc.NewEVSECC(localEntity, c.ucCallback), - EvCC: evcc.NewEVCC(c.service, localEntity, c.ucCallback), - EvCem: evcem.NewEVCEM(c.service, localEntity, c.ucCallback), - OpEV: opev.NewOPEV(localEntity, c.ucCallback), - OscEV: oscev.NewOSCEV(localEntity, c.ucCallback), - EvSoc: evsoc.NewEVSOC(localEntity, c.ucCallback), + LocalEntity: localEntity, + EvseCC: evsecc.NewEVSECC(localEntity, c.ucCallback), + EvCC: evcc.NewEVCC(c.service, localEntity, c.ucCallback), + EvCem: evcem.NewEVCEM(c.service, localEntity, c.ucCallback), + OpEV: opev.NewOPEV(localEntity, c.ucCallback), + OscEV: oscev.NewOSCEV(localEntity, c.ucCallback), + EvSoc: evsoc.NewEVSOC(localEntity, c.ucCallback), } // monitoring appliance to meters From 6c0f7de2c2e44f42afc9fc3fe3b9ee141ae95f5d Mon Sep 17 00:00:00 2001 From: Andreas Linde Date: Sat, 11 Apr 2026 13:17:11 +0200 Subject: [PATCH 2/3] EEBus: guard against nil ParameterId in buildPhaseLimitData Extend the per-phase skip condition to also check ParameterId, preventing a panic when a remote entity omits it in the parameter description. --- charger/eebus.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/charger/eebus.go b/charger/eebus.go index 178bc9124e7..879d1f4777c 100644 --- a/charger/eebus.go +++ b/charger/eebus.go @@ -453,7 +453,7 @@ func buildPhaseLimitData( AcMeasuredPhases: lo.ToPtr(phaseLimit.Phase), } elParamDesc, err := elConn.GetParameterDescriptionsForFilter(paramFilter) - if err != nil || len(elParamDesc) == 0 || elParamDesc[0].MeasurementId == nil { + if err != nil || len(elParamDesc) == 0 || elParamDesc[0].MeasurementId == nil || elParamDesc[0].ParameterId == nil { continue } From 53fcad9bbd73736c84f589585fd6036b721c3d33 Mon Sep 17 00:00:00 2001 From: Andreas Linde Date: Sat, 11 Apr 2026 13:27:16 +0200 Subject: [PATCH 3/3] EEBus: extract oscevMinLimits and findLimitDescriptionByMeasurementID Flatten the four-level nested OSCEV branch in writeCurrentLimitData and replace the inline description-match loop in buildPhaseLimitData with a single slice-aliasing helper, dropping a pre-Go-1.22 loop-variable dance. Both helpers are covered by new tests. --- charger/eebus.go | 57 +++++++++++++++++++-------- charger/eebus_test.go | 89 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 131 insertions(+), 15 deletions(-) diff --git a/charger/eebus.go b/charger/eebus.go index 879d1f4777c..5758616589d 100644 --- a/charger/eebus.go +++ b/charger/eebus.go @@ -363,13 +363,9 @@ func (c *EEBus) writeCurrentLimitData(evEntity spineapi.EntityRemoteInterface, c // OSCEV: recommendation max — optional. Active only when the requested // current is at least the phase min (a recommendation below min cannot // drive charging and is equivalent to no recommendation). - if c.cem.OscEV.IsScenarioAvailableAtEntity(evEntity, 1) { - if _, lerr := c.cem.OscEV.LoadControlLimits(evEntity); lerr == nil { - if minLimits, _, _, merr := c.cem.OscEV.CurrentLimits(evEntity); merr == nil { - if entries := buildPhaseLimitData(loadControl, elConn, oscevFilter, computeOscevLimits(current, minLimits)); entries != nil { - data = append(data, entries...) - } - } + if minLimits, ok := oscevMinLimits(c.cem.OscEV, evEntity); ok { + if entries := buildPhaseLimitData(loadControl, elConn, oscevFilter, computeOscevLimits(current, minLimits)); entries != nil { + data = append(data, entries...) } } @@ -426,6 +422,44 @@ func computeOscevLimits(current float64, minLimits []float64) []ucapi.LoadLimits return limits } +// oscevMinLimits fetches the OSCEV recommendation-min per-phase limits from +// the remote EV. It returns (nil, false) if the OSCEV scenario is not +// available, the load-control limits are missing, or the current limits +// cannot be read — these are the three skip conditions of the OSCEV branch +// in writeCurrentLimitData. +func oscevMinLimits( + oscEV ucapi.CemOSCEVInterface, + evEntity spineapi.EntityRemoteInterface, +) ([]float64, bool) { + if !oscEV.IsScenarioAvailableAtEntity(evEntity, 1) { + return nil, false + } + if _, err := oscEV.LoadControlLimits(evEntity); err != nil { + return nil, false + } + minLimits, _, _, err := oscEV.CurrentLimits(evEntity) + if err != nil { + return nil, false + } + return minLimits, true +} + +// findLimitDescriptionByMeasurementID returns a pointer to the first limit +// description in the slice whose MeasurementId matches the target, or nil +// if none match. Descriptions with a nil MeasurementId are skipped. The +// returned pointer aliases the caller's slice element. +func findLimitDescriptionByMeasurementID( + descs []model.LoadControlLimitDescriptionDataType, + measurementID model.MeasurementIdType, +) *model.LoadControlLimitDescriptionDataType { + for i := range descs { + if descs[i].MeasurementId != nil && *descs[i].MeasurementId == measurementID { + return &descs[i] + } + } + return nil +} + // buildPhaseLimitData converts per-phase LoadLimitsPhase tuples into spine // LoadControlLimitDataType entries for a given description filter. It mirrors // the per-phase mapping inside eebus-go usecases/internal/loadcontrol.go @@ -457,14 +491,7 @@ func buildPhaseLimitData( continue } - var limitDesc *model.LoadControlLimitDescriptionDataType - for _, desc := range limitDescriptions { - if desc.MeasurementId != nil && *desc.MeasurementId == *elParamDesc[0].MeasurementId { - safe := desc - limitDesc = &safe - break - } - } + limitDesc := findLimitDescriptionByMeasurementID(limitDescriptions, *elParamDesc[0].MeasurementId) if limitDesc == nil || limitDesc.LimitId == nil { continue } diff --git a/charger/eebus_test.go b/charger/eebus_test.go index aa92cadeff0..330aa1b8447 100644 --- a/charger/eebus_test.go +++ b/charger/eebus_test.go @@ -172,6 +172,95 @@ func TestComputeOscevLimits(t *testing.T) { } } +// TestFindLimitDescriptionByMeasurementID pins the per-phase description +// lookup that buildPhaseLimitData uses to map a parameter's MeasurementId to +// the matching LoadControl limit description. Descriptions with a nil +// MeasurementId must be skipped; first match wins; no match returns nil. +func TestFindLimitDescriptionByMeasurementID(t *testing.T) { + id := func(v model.MeasurementIdType) *model.MeasurementIdType { return &v } + limitID := func(v model.LoadControlLimitIdType) *model.LoadControlLimitIdType { return &v } + + descs := []model.LoadControlLimitDescriptionDataType{ + {MeasurementId: nil, LimitId: limitID(1)}, + {MeasurementId: id(7), LimitId: limitID(2)}, + {MeasurementId: id(9), LimitId: limitID(3)}, + {MeasurementId: id(9), LimitId: limitID(4)}, + } + + t.Run("empty slice returns nil", func(t *testing.T) { + assert.Nil(t, findLimitDescriptionByMeasurementID(nil, 9)) + }) + + t.Run("no match returns nil", func(t *testing.T) { + assert.Nil(t, findLimitDescriptionByMeasurementID(descs, 42)) + }) + + t.Run("skips nil MeasurementId and returns first match", func(t *testing.T) { + got := findLimitDescriptionByMeasurementID(descs, 7) + require.NotNil(t, got) + require.NotNil(t, got.LimitId) + assert.Equal(t, model.LoadControlLimitIdType(2), *got.LimitId) + }) + + t.Run("first-match wins on duplicates", func(t *testing.T) { + got := findLimitDescriptionByMeasurementID(descs, 9) + require.NotNil(t, got) + require.NotNil(t, got.LimitId) + assert.Equal(t, model.LoadControlLimitIdType(3), *got.LimitId) + }) +} + +// TestOscevMinLimits pins the three skip conditions and the happy path of +// the OSCEV data-availability check that writeCurrentLimitData performs +// before including recommendation limits in the combined write. +func TestOscevMinLimits(t *testing.T) { + t.Run("scenario not available returns false", func(t *testing.T) { + oscev := mocks.NewCemOSCEVInterface(t) + evEntity := spinemocks.NewEntityRemoteInterface(t) + oscev.EXPECT().IsScenarioAvailableAtEntity(evEntity, uint(1)).Return(false) + + limits, ok := oscevMinLimits(oscev, evEntity) + assert.False(t, ok) + assert.Nil(t, limits) + }) + + t.Run("LoadControlLimits error returns false", func(t *testing.T) { + oscev := mocks.NewCemOSCEVInterface(t) + evEntity := spinemocks.NewEntityRemoteInterface(t) + oscev.EXPECT().IsScenarioAvailableAtEntity(evEntity, uint(1)).Return(true) + oscev.EXPECT().LoadControlLimits(evEntity).Return(nil, errors.New("no data")) + + limits, ok := oscevMinLimits(oscev, evEntity) + assert.False(t, ok) + assert.Nil(t, limits) + }) + + t.Run("CurrentLimits error returns false", func(t *testing.T) { + oscev := mocks.NewCemOSCEVInterface(t) + evEntity := spinemocks.NewEntityRemoteInterface(t) + oscev.EXPECT().IsScenarioAvailableAtEntity(evEntity, uint(1)).Return(true) + oscev.EXPECT().LoadControlLimits(evEntity).Return(nil, nil) + oscev.EXPECT().CurrentLimits(evEntity).Return(nil, nil, nil, errors.New("no limits")) + + limits, ok := oscevMinLimits(oscev, evEntity) + assert.False(t, ok) + assert.Nil(t, limits) + }) + + t.Run("happy path returns min limits", func(t *testing.T) { + oscev := mocks.NewCemOSCEVInterface(t) + evEntity := spinemocks.NewEntityRemoteInterface(t) + want := limits3p(6) + oscev.EXPECT().IsScenarioAvailableAtEntity(evEntity, uint(1)).Return(true) + oscev.EXPECT().LoadControlLimits(evEntity).Return(nil, nil) + oscev.EXPECT().CurrentLimits(evEntity).Return(want, limits3p(16), limits3p(10), nil) + + limits, ok := oscevMinLimits(oscev, evEntity) + assert.True(t, ok) + assert.Equal(t, want, limits) + }) +} + // TestWriteCurrentLimitData_OpevNotAvailable verifies that writeCurrentLimitData // returns ErrNotAvailable without touching any clients when OpEV scenario 1 is // not supported on the target entity.