Fix available uc scenarios on device removal (#145)

DerAndereAndi · web-flow · commit 1edb43f80f85 · 2024-12-10T13:08:48.000+01:00
When a device was disconnected, the cache of available scenarios wasn’t
properly updated and the entities of the disconnected device still
remained being available.

This update fixes the issue. In addition some method names where updated
to better reflect their functionality
diff --git a/examples/remote/ucs.go b/examples/remote/ucs.go
@@ -45,8 +45,12 @@ func (r *Remote) PropagateEvent(
 ) {
 	params := make(map[string]interface{}, 2)
 	params["ski"] = ski
-	params["device"] = device.Address()
-	params["entity"] = entity.Address()
+	if device != nil {
+		params["device"] = device.Address()
+	}
+	if entity != nil {
+		params["entity"] = entity.Address()
+	}
 	for _, conn := range r.connections {
 		_ = conn.Notify(context.Background(), string(event), params)
 	}
diff --git a/go.mod b/go.mod
@@ -4,7 +4,7 @@ go 1.22.0
 
 require (
 	github.com/enbility/ship-go v0.0.0-20241118145930-d68708c5f1c0
-	github.com/enbility/spine-go v0.0.0-20241118145803-0589320ceced
+	github.com/enbility/spine-go v0.0.0-20241209160856-1aed917e83e7
 	github.com/stretchr/testify v1.9.0
 	golang.org/x/exp/jsonrpc2 v0.0.0-20240909161429-701f63a606c0
 )
diff --git a/go.sum b/go.sum
@@ -6,8 +6,8 @@ github.com/enbility/go-avahi v0.0.0-20240909195612-d5de6b280d7a h1:foChWb8lhzqa6
 github.com/enbility/go-avahi v0.0.0-20240909195612-d5de6b280d7a/go.mod h1:H64mhYcAQUGUUnVqMdZQf93kPecH4M79xwH95Lddt3U=
 github.com/enbility/ship-go v0.0.0-20241118145930-d68708c5f1c0 h1:Z8j/N4DgUL8T8mINkHdq0bUbKcWtwDpno0bsKOGahPo=
 github.com/enbility/ship-go v0.0.0-20241118145930-d68708c5f1c0/go.mod h1:JJp8EQcJhUhTpZ2LSEU4rpdaM3E2n08tswWFWtmm/wU=
-github.com/enbility/spine-go v0.0.0-20241118145803-0589320ceced h1:Z2WrJ+ku7lPZqJ+uzqvIqdMpXqvAZRB3J3xW592pDXI=
-github.com/enbility/spine-go v0.0.0-20241118145803-0589320ceced/go.mod h1:ZoI9TaJO/So/677uknrli8sc6iryD7wC5iWhVIre+MI=
+github.com/enbility/spine-go v0.0.0-20241209160856-1aed917e83e7 h1:Pq1L3U/aoSg8qQj4CfSEUCh9fxgB3G/skUNQI32zQeg=
+github.com/enbility/spine-go v0.0.0-20241209160856-1aed917e83e7/go.mod h1:ZoI9TaJO/So/677uknrli8sc6iryD7wC5iWhVIre+MI=
 github.com/enbility/zeroconf/v2 v2.0.0-20240920094356-be1cae74fda6 h1:XOYvxKtT1oxT37w/5oEiRLuPbm9FuJPt3fiYhX0h8Po=
 github.com/enbility/zeroconf/v2 v2.0.0-20240920094356-be1cae74fda6/go.mod h1:BszP9qFV14mPXgyIREbgIdQtWxbAj3OKqvK02HihMoM=
 github.com/godbus/dbus/v5 v5.1.0 h1:4KLkAxT3aOY8Li4FRJe/KvhoNFFxo0m6fNuFUO8QJUk=
diff --git a/service/service_hub.go b/service/service_hub.go
@@ -7,11 +7,18 @@ import (
 var _ shipapi.HubReaderInterface = (*Service)(nil)
 
 // report a connection to a SKI
+//
+// is triggered whenever a SHIP connected was successful completed
 func (s *Service) RemoteSKIConnected(ski string) {
 	s.serviceHandler.RemoteSKIConnected(s, ski)
 }
 
 // report a disconnection to a SKI
+//
+// is triggered whenever a SHIP connect was closed, is also triggered when the SHIP
+// process wasn't successfully completed
+//
+// NOTE: The connection may not have been reported as connected before!
 func (s *Service) RemoteSKIDisconnected(ski string) {
 	if s.spineLocalDevice != nil {
 		s.spineLocalDevice.RemoveRemoteDeviceConnection(ski)
diff --git a/usecases/cem/cevc/events.go b/usecases/cem/cevc/events.go
@@ -17,7 +17,7 @@ func (e *CEVC) HandleEvent(payload spineapi.EventPayload) {
 		return
 	}
 
-	if internal.IsEntityConnected(payload) {
+	if internal.IsEntityAdded(payload) {
 		e.evConnected(payload.Entity)
 		return
 	}
diff --git a/usecases/cem/evcc/events.go b/usecases/cem/evcc/events.go
@@ -17,10 +17,10 @@ func (e *EVCC) HandleEvent(payload spineapi.EventPayload) {
 		return
 	}
 
-	if internal.IsEntityConnected(payload) {
+	if internal.IsEntityAdded(payload) {
 		e.evConnected(payload)
 		return
-	} else if internal.IsEntityDisconnected(payload) {
+	} else if internal.IsEntityRemoved(payload) {
 		e.evDisconnected(payload)
 		return
 	}
diff --git a/usecases/cem/evcem/events.go b/usecases/cem/evcem/events.go
@@ -17,7 +17,7 @@ func (e *EVCEM) HandleEvent(payload spineapi.EventPayload) {
 		return
 	}
 
-	if internal.IsEntityConnected(payload) {
+	if internal.IsEntityAdded(payload) {
 		e.evConnected(payload.Entity)
 		return
 	}
diff --git a/usecases/cem/evsecc/events.go b/usecases/cem/evsecc/events.go
@@ -15,10 +15,10 @@ func (e *EVSECC) HandleEvent(payload spineapi.EventPayload) {
 		return
 	}
 
-	if internal.IsEntityConnected(payload) {
+	if internal.IsEntityAdded(payload) {
 		e.evseConnected(payload)
 		return
-	} else if internal.IsEntityDisconnected(payload) {
+	} else if internal.IsEntityRemoved(payload) {
 		e.evseDisconnected(payload)
 		return
 	}
diff --git a/usecases/cem/evsoc/events.go b/usecases/cem/evsoc/events.go
@@ -16,7 +16,7 @@ func (e *EVSOC) HandleEvent(payload spineapi.EventPayload) {
 		return
 	}
 
-	if internal.IsEntityConnected(payload) {
+	if internal.IsEntityAdded(payload) {
 		e.evConnected(payload.Entity)
 		return
 	}
diff --git a/usecases/cem/opev/events.go b/usecases/cem/opev/events.go
@@ -17,7 +17,7 @@ func (e *OPEV) HandleEvent(payload spineapi.EventPayload) {
 		return
 	}
 
-	if internal.IsEntityConnected(payload) {
+	if internal.IsEntityAdded(payload) {
 		e.evConnected(payload.Entity)
 		return
 	}
diff --git a/usecases/cem/vabd/events.go b/usecases/cem/vabd/events.go
@@ -17,7 +17,7 @@ func (e *VABD) HandleEvent(payload spineapi.EventPayload) {
 		return
 	}
 
-	if internal.IsEntityConnected(payload) {
+	if internal.IsEntityAdded(payload) {
 		e.inverterConnected(payload.Entity)
 		return
 	}
diff --git a/usecases/cem/vapd/events.go b/usecases/cem/vapd/events.go
@@ -17,7 +17,7 @@ func (e *VAPD) HandleEvent(payload spineapi.EventPayload) {
 		return
 	}
 
-	if internal.IsEntityConnected(payload) {
+	if internal.IsEntityAdded(payload) {
 		e.inverterConnected(payload.Entity)
 		return
 	}
diff --git a/usecases/eg/lpc/events.go b/usecases/eg/lpc/events.go
@@ -14,7 +14,7 @@ func (e *LPC) HandleEvent(payload spineapi.EventPayload) {
 	if !e.IsCompatibleEntityType(payload.Entity) {
 		return
 	}
-	if internal.IsEntityConnected(payload) {
+	if internal.IsEntityAdded(payload) {
 		e.connected(payload.Entity)
 		return
 	}
diff --git a/usecases/eg/lpp/events.go b/usecases/eg/lpp/events.go
@@ -15,7 +15,7 @@ func (e *LPP) HandleEvent(payload spineapi.EventPayload) {
 		return
 	}
 
-	if internal.IsEntityConnected(payload) {
+	if internal.IsEntityAdded(payload) {
 		e.connected(payload.Entity)
 		return
 	}
diff --git a/usecases/internal/helper.go b/usecases/internal/helper.go
@@ -11,12 +11,12 @@ func IsDeviceConnected(payload spineapi.EventPayload) bool {
 }
 
 func IsDeviceDisconnected(payload spineapi.EventPayload) bool {
-	return payload.Device != nil &&
+	return len(payload.Ski) > 0 &&
 		payload.EventType == spineapi.EventTypeDeviceChange &&
 		payload.ChangeType == spineapi.ElementChangeRemove
 }
 
-func IsEntityConnected(payload spineapi.EventPayload) bool {
+func IsEntityAdded(payload spineapi.EventPayload) bool {
 	if payload.Entity != nil &&
 		payload.EventType == spineapi.EventTypeEntityChange &&
 		payload.ChangeType == spineapi.ElementChangeAdd {
@@ -26,7 +26,7 @@ func IsEntityConnected(payload spineapi.EventPayload) bool {
 	return false
 }
 
-func IsEntityDisconnected(payload spineapi.EventPayload) bool {
+func IsEntityRemoved(payload spineapi.EventPayload) bool {
 	if payload.Entity != nil &&
 		payload.EventType == spineapi.EventTypeEntityChange &&
 		payload.ChangeType == spineapi.ElementChangeRemove {
diff --git a/usecases/internal/helper_test.go b/usecases/internal/helper_test.go
@@ -28,6 +28,7 @@ func (s *InternalSuite) Test_IsDeviceDisconnected() {
 
 	device := mocks.NewDeviceRemoteInterface(s.T())
 	payload = spineapi.EventPayload{
+		Ski:        "12345",
 		Device:     device,
 		EventType:  spineapi.EventTypeDeviceChange,
 		ChangeType: spineapi.ElementChangeRemove,
@@ -36,30 +37,30 @@ func (s *InternalSuite) Test_IsDeviceDisconnected() {
 	assert.Equal(s.T(), true, result)
 }
 
-func (s *InternalSuite) Test_IsEntityConnected() {
+func (s *InternalSuite) Test_IsEntityAdded() {
 	payload := spineapi.EventPayload{}
-	result := IsEntityConnected(payload)
+	result := IsEntityAdded(payload)
 	assert.Equal(s.T(), false, result)
 
 	payload = spineapi.EventPayload{
 		Entity:     s.evseEntity,
 		EventType:  spineapi.EventTypeEntityChange,
 		ChangeType: spineapi.ElementChangeAdd,
 	}
-	result = IsEntityConnected(payload)
+	result = IsEntityAdded(payload)
 	assert.Equal(s.T(), true, result)
 }
 
-func (s *InternalSuite) Test_IsEntityDisconnected() {
+func (s *InternalSuite) Test_IsEntityRemoved() {
 	payload := spineapi.EventPayload{}
-	result := IsEntityDisconnected(payload)
+	result := IsEntityRemoved(payload)
 	assert.Equal(s.T(), false, result)
 
 	payload = spineapi.EventPayload{
 		Entity:     s.evseEntity,
 		EventType:  spineapi.EventTypeEntityChange,
 		ChangeType: spineapi.ElementChangeRemove,
 	}
-	result = IsEntityDisconnected(payload)
+	result = IsEntityRemoved(payload)
 	assert.Equal(s.T(), true, result)
 }
diff --git a/usecases/ma/mgcp/events.go b/usecases/ma/mgcp/events.go
@@ -17,7 +17,7 @@ func (e *MGCP) HandleEvent(payload spineapi.EventPayload) {
 		return
 	}
 
-	if internal.IsEntityConnected(payload) {
+	if internal.IsEntityAdded(payload) {
 		e.gridConnected(payload.Entity)
 		return
 	}
diff --git a/usecases/ma/mpc/events.go b/usecases/ma/mpc/events.go
@@ -17,7 +17,7 @@ func (e *MPC) HandleEvent(payload spineapi.EventPayload) {
 		return
 	}
 
-	if internal.IsEntityConnected(payload) {
+	if internal.IsEntityAdded(payload) {
 		e.deviceConnected(payload.Entity)
 		return
 	}
diff --git a/usecases/usecase/events.go b/usecases/usecase/events.go
@@ -24,7 +24,17 @@ func (u *UseCaseBase) HandleEvent(payload spineapi.EventPayload) {
 }
 
 func (u *UseCaseBase) deviceOrEntityRemoved(payload spineapi.EventPayload) bool {
-	if internal.IsDeviceDisconnected(payload) || internal.IsEntityDisconnected(payload) {
+	if payload.EventType == spineapi.EventTypeDeviceChange &&
+		payload.ChangeType == spineapi.ElementChangeRemove &&
+		payload.Device != nil &&
+		payload.Entity == nil {
+		// device was disconnected, remove all usecases related to this device
+		u.removeDeviceFromAvailableEntityScenarios(payload.Device)
+		return true
+	}
+
+	if internal.IsEntityRemoved(payload) {
+		// entity was disconnected, remove all usecases related to this entity
 		u.removeEntityFromAvailableEntityScenarios(payload.Entity)
 		return true
 	}
diff --git a/usecases/usecase/events_test.go b/usecases/usecase/events_test.go
@@ -11,12 +11,16 @@ func (s *UseCaseSuite) Test_HandleEvent() {
 	payload := spineapi.EventPayload{}
 	s.uc.HandleEvent(payload)
 
+	payload.Ski = "test"
 	payload.Device = s.remoteDevice
 	payload.EventType = spineapi.EventTypeDeviceChange
 	payload.ChangeType = spineapi.ElementChangeRemove
 	s.uc.HandleEvent(payload)
 
+	payload.EventType = spineapi.EventTypeEntityChange
 	payload.Entity = s.remoteDevice.Entities()[0]
+	s.uc.HandleEvent(payload)
+
 	payload.EventType = spineapi.EventTypeDataChange
 	payload.ChangeType = spineapi.ElementChangeUpdate
 	payload.Data = &model.NodeManagementUseCaseDataType{}
diff --git a/usecases/usecase/usecase.go b/usecases/usecase/usecase.go
@@ -127,7 +127,7 @@ func (u *UseCaseBase) RemoteEntitiesScenarios() []api.RemoteEntityScenarios {
 
 // return the currently available scenarios for the use case for a remote entity
 func (u *UseCaseBase) AvailableScenariosForEntity(entity spineapi.EntityRemoteInterface) []uint {
-	_, scenarios := u.indexAndScenariosOfEntity(entity)
+	_, scenarios := u.entitiyScenarioIndexOfEntity(entity)
 
 	return scenarios
 }
@@ -137,24 +137,45 @@ func (u *UseCaseBase) IsScenarioAvailableAtEntity(
 	entity spineapi.EntityRemoteInterface,
 	scenario uint,
 ) bool {
-	if _, scenarios := u.indexAndScenariosOfEntity(entity); scenarios != nil {
+	if _, scenarios := u.entitiyScenarioIndexOfEntity(entity); scenarios != nil {
 		return slices.Contains(scenarios, scenario)
 	}
 
 	return false
 }
 
+// return the indices of all entities of the device in the available entity scenarios
+func (u *UseCaseBase) entityScenarioIndicesOfDevice(device spineapi.DeviceRemoteInterface) []int {
+	u.mux.Lock()
+	defer u.mux.Unlock()
+
+	indices := []int{}
+
+	for i, remoteEntityScenarios := range u.availableEntityScenarios {
+		if device != nil && device.Address() != nil &&
+			remoteEntityScenarios.Entity != nil &&
+			remoteEntityScenarios.Entity.Device() != nil &&
+			remoteEntityScenarios.Entity.Device().Address() != nil &&
+			reflect.DeepEqual(device.Address(), remoteEntityScenarios.Entity.Device().Address()) {
+			indices = append(indices, i)
+		}
+	}
+
+	return indices
+}
+
 // return the index and the scenarios of the entity in the available entity scenarios
 // and return -1 and nil if not found
-func (u *UseCaseBase) indexAndScenariosOfEntity(entity spineapi.EntityRemoteInterface) (int, []uint) {
+func (u *UseCaseBase) entitiyScenarioIndexOfEntity(entity spineapi.EntityRemoteInterface) (int, []uint) {
 	u.mux.Lock()
 	defer u.mux.Unlock()
 
-	for i, remoteEntity := range u.availableEntityScenarios {
-		if entity != nil && entity.Address() != nil && remoteEntity.Entity.Address() != nil &&
-			reflect.DeepEqual(entity.Address().Device, remoteEntity.Entity.Address().Device) &&
-			reflect.DeepEqual(entity.Address().Entity, remoteEntity.Entity.Address().Entity) {
-			return i, remoteEntity.Scenarios
+	for i, remoteEntityScenarios := range u.availableEntityScenarios {
+		if entity != nil && entity.Address() != nil &&
+			remoteEntityScenarios.Entity != nil && remoteEntityScenarios.Entity.Address() != nil &&
+			reflect.DeepEqual(entity.Address().Device, remoteEntityScenarios.Entity.Address().Device) &&
+			reflect.DeepEqual(entity.Address().Entity, remoteEntityScenarios.Entity.Address().Entity) {
+			return i, remoteEntityScenarios.Scenarios
 		}
 	}
 
@@ -173,7 +194,7 @@ func (u *UseCaseBase) updateRemoteEntityScenarios(
 		scenarioValues = append(scenarioValues, uint(scenario))
 	}
 
-	i, _ := u.indexAndScenariosOfEntity(entity)
+	i, _ := u.entitiyScenarioIndexOfEntity(entity)
 	if i == -1 {
 		newItem := api.RemoteEntityScenarios{
 			Entity:    entity,
@@ -198,19 +219,38 @@ func (u *UseCaseBase) updateRemoteEntityScenarios(
 	}
 }
 
+// remove all remote entities of a device from the use case
+func (u *UseCaseBase) removeDeviceFromAvailableEntityScenarios(device spineapi.DeviceRemoteInterface) {
+	indicies := u.entityScenarioIndicesOfDevice(device)
+
+	for _, i := range indicies {
+		u.removeEntityIndexFromAvailableEntityScenarios(i)
+	}
+
+	if u.EventCB != nil && len(indicies) > 0 {
+		u.EventCB(device.Ski(), device, nil, u.useCaseUpdateEvent)
+	}
+}
+
 // remove a remote entity from the use case
 func (u *UseCaseBase) removeEntityFromAvailableEntityScenarios(entity spineapi.EntityRemoteInterface) {
-	if i, _ := u.indexAndScenariosOfEntity(entity); i >= 0 {
-		u.mux.Lock()
-		u.availableEntityScenarios = append(u.availableEntityScenarios[:i], u.availableEntityScenarios[i+1:]...)
-		u.mux.Unlock()
+	if i, _ := u.entitiyScenarioIndexOfEntity(entity); i >= 0 {
+		u.removeEntityIndexFromAvailableEntityScenarios(i)
 
 		if u.EventCB != nil {
-			u.EventCB(entity.Device().Ski(), entity.Device(), entity, u.useCaseUpdateEvent)
+			remoteDevice := entity.Device()
+			u.EventCB(remoteDevice.Ski(), remoteDevice, entity, u.useCaseUpdateEvent)
 		}
 	}
 }
 
+// do the actual removal of the entity from the available entity scenarios
+func (u *UseCaseBase) removeEntityIndexFromAvailableEntityScenarios(index int) {
+	u.mux.Lock()
+	u.availableEntityScenarios = append(u.availableEntityScenarios[:index], u.availableEntityScenarios[index+1:]...)
+	u.mux.Unlock()
+}
+
 // return the required server features for a use case scenario
 func (u *UseCaseBase) requiredServerFeaturesForScenario(scenario model.UseCaseScenarioSupportType) []model.FeatureTypeType {
 	for _, serverFeatures := range u.useCaseScenarios {
diff --git a/usecases/usecase/usecase_test.go b/usecases/usecase/usecase_test.go

Original file line number	Diff line number	Diff line change
`@@ -4,7 +4,7 @@ go 1.22.0`
`4`	`4`
`5`	`5`	`require (`
`6`	`6`	`github.com/enbility/ship-go v0.0.0-20241118145930-d68708c5f1c0`
`7`		`- github.com/enbility/spine-go v0.0.0-20241118145803-0589320ceced`
	`7`	`+ github.com/enbility/spine-go v0.0.0-20241209160856-1aed917e83e7`
`8`	`8`	`github.com/stretchr/testify v1.9.0`
`9`	`9`	`golang.org/x/exp/jsonrpc2 v0.0.0-20240909161429-701f63a606c0`
`10`	`10`	`)`
Original file line number	Diff line number	Diff line change
`@@ -17,7 +17,7 @@ func (e *CEVC) HandleEvent(payload spineapi.EventPayload) {`
`17`	`17`	`return`
`18`	`18`	`}`
`19`	`19`
`20`		`- if internal.IsEntityConnected(payload) {`
	`20`	`+ if internal.IsEntityAdded(payload) {`
`21`	`21`	`e.evConnected(payload.Entity)`
`22`	`22`	`return`
`23`	`23`	`}`
Original file line number	Diff line number	Diff line change
`@@ -17,10 +17,10 @@ func (e *EVCC) HandleEvent(payload spineapi.EventPayload) {`
`17`	`17`	`return`
`18`	`18`	`}`
`19`	`19`
`20`		`- if internal.IsEntityConnected(payload) {`
	`20`	`+ if internal.IsEntityAdded(payload) {`
`21`	`21`	`e.evConnected(payload)`
`22`	`22`	`return`
`23`		`- } else if internal.IsEntityDisconnected(payload) {`
	`23`	`+ } else if internal.IsEntityRemoved(payload) {`
`24`	`24`	`e.evDisconnected(payload)`
`25`	`25`	`return`
`26`	`26`	`}`
Original file line number	Diff line number	Diff line change
`@@ -17,7 +17,7 @@ func (e *EVCEM) HandleEvent(payload spineapi.EventPayload) {`
`17`	`17`	`return`
`18`	`18`	`}`
`19`	`19`
`20`		`- if internal.IsEntityConnected(payload) {`
	`20`	`+ if internal.IsEntityAdded(payload) {`
`21`	`21`	`e.evConnected(payload.Entity)`
`22`	`22`	`return`
`23`	`23`	`}`
Original file line number	Diff line number	Diff line change
`@@ -15,10 +15,10 @@ func (e *EVSECC) HandleEvent(payload spineapi.EventPayload) {`
`15`	`15`	`return`
`16`	`16`	`}`
`17`	`17`
`18`		`- if internal.IsEntityConnected(payload) {`
	`18`	`+ if internal.IsEntityAdded(payload) {`
`19`	`19`	`e.evseConnected(payload)`
`20`	`20`	`return`
`21`		`- } else if internal.IsEntityDisconnected(payload) {`
	`21`	`+ } else if internal.IsEntityRemoved(payload) {`
`22`	`22`	`e.evseDisconnected(payload)`
`23`	`23`	`return`
`24`	`24`	`}`
Original file line number	Diff line number	Diff line change
`@@ -16,7 +16,7 @@ func (e *EVSOC) HandleEvent(payload spineapi.EventPayload) {`
`16`	`16`	`return`
`17`	`17`	`}`
`18`	`18`
`19`		`- if internal.IsEntityConnected(payload) {`
	`19`	`+ if internal.IsEntityAdded(payload) {`
`20`	`20`	`e.evConnected(payload.Entity)`
`21`	`21`	`return`
`22`	`22`	`}`
Original file line number	Diff line number	Diff line change
`@@ -17,7 +17,7 @@ func (e *OPEV) HandleEvent(payload spineapi.EventPayload) {`
`17`	`17`	`return`
`18`	`18`	`}`
`19`	`19`
`20`		`- if internal.IsEntityConnected(payload) {`
	`20`	`+ if internal.IsEntityAdded(payload) {`
`21`	`21`	`e.evConnected(payload.Entity)`
`22`	`22`	`return`
`23`	`23`	`}`