From 1edb43f80f857cba9ee718ebf16f0eaa6da0a437 Mon Sep 17 00:00:00 2001 From: Andreas Linde <42185+DerAndereAndi@users.noreply.github.com> Date: Tue, 10 Dec 2024 13:08:48 +0100 Subject: [PATCH] Fix available uc scenarios on device removal (#145) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- examples/remote/ucs.go | 8 +++- go.mod | 2 +- go.sum | 4 +- service/service_hub.go | 7 ++++ usecases/cem/cevc/events.go | 2 +- usecases/cem/evcc/events.go | 4 +- usecases/cem/evcem/events.go | 2 +- usecases/cem/evsecc/events.go | 4 +- usecases/cem/evsoc/events.go | 2 +- usecases/cem/opev/events.go | 2 +- usecases/cem/vabd/events.go | 2 +- usecases/cem/vapd/events.go | 2 +- usecases/eg/lpc/events.go | 2 +- usecases/eg/lpp/events.go | 2 +- usecases/internal/helper.go | 6 +-- usecases/internal/helper_test.go | 13 +++--- usecases/ma/mgcp/events.go | 2 +- usecases/ma/mpc/events.go | 2 +- usecases/usecase/events.go | 12 +++++- usecases/usecase/events_test.go | 4 ++ usecases/usecase/usecase.go | 68 +++++++++++++++++++++++++------- usecases/usecase/usecase_test.go | 37 +++++++++++++++++ 22 files changed, 146 insertions(+), 43 deletions(-) diff --git a/examples/remote/ucs.go b/examples/remote/ucs.go index d1eead23..457963a2 100644 --- 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 index 39fb7ab7..03d7f085 100644 --- 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 index 20322b35..6d1f5822 100644 --- 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 index 27230d75..90745b96 100644 --- 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 index c47e31fd..7ddb07de 100644 --- 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 index 0c789827..702cb710 100644 --- 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 index e26e6556..3fa5310e 100644 --- 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 index 6b8a1e7d..65342b58 100644 --- 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 index 21d5910d..4d1e35c9 100644 --- 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 index d0ab328a..4b4ae54a 100644 --- 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 index fa8f9ab9..ba1bea46 100644 --- 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 index 30f94a69..535a4e1a 100644 --- 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 index 7dd70c65..ef2a89bb 100644 --- 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 index 8a058d1d..bb91cd9c 100644 --- 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 index 78dd4d9e..a3f0e1ea 100644 --- 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 index e8f65192..11a83b91 100644 --- 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,9 +37,9 @@ 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{ @@ -46,13 +47,13 @@ func (s *InternalSuite) Test_IsEntityConnected() { 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{ @@ -60,6 +61,6 @@ func (s *InternalSuite) Test_IsEntityDisconnected() { 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 index 38a2f81c..7f88ac64 100644 --- 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 index ae13b5b9..05a2da62 100644 --- 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 index 0bab5b49..73c62cbc 100644 --- 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 index de1c35b2..e4b14b1d 100644 --- 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 index 17d58e36..70341dbf 100644 --- 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 index 7a7c9a16..e5c307b0 100644 --- a/usecases/usecase/usecase_test.go +++ b/usecases/usecase/usecase_test.go @@ -43,6 +43,33 @@ func (s *UseCaseSuite) Test() { assert.False(s.T(), result) } +func (s *UseCaseSuite) Test_RemoveDeviceScenarios() { + result := s.uc.RemoteEntitiesScenarios() + assert.Equal(s.T(), 0, len(result)) + + scenarios := s.uc.AvailableScenariosForEntity(s.monitoredEntity) + assert.Equal(s.T(), 0, len(scenarios)) + + ok := s.uc.IsScenarioAvailableAtEntity(s.monitoredEntity, 1) + assert.False(s.T(), ok) + + s.uc.updateRemoteEntityScenarios(s.monitoredEntity, []model.UseCaseScenarioSupportType{1, 2, 3}) + + result = s.uc.RemoteEntitiesScenarios() + assert.Equal(s.T(), 1, len(result)) + + scenarios = s.uc.AvailableScenariosForEntity(s.monitoredEntity) + assert.Equal(s.T(), 3, len(scenarios)) + + s.uc.removeDeviceFromAvailableEntityScenarios(s.monitoredEntity.Device()) + + result = s.uc.RemoteEntitiesScenarios() + assert.Equal(s.T(), 0, len(result)) + + scenarios = s.uc.AvailableScenariosForEntity(s.monitoredEntity) + assert.Equal(s.T(), 0, len(scenarios)) +} + func (s *UseCaseSuite) Test_AvailableScenarios() { result := s.uc.RemoteEntitiesScenarios() assert.Equal(s.T(), 0, len(result)) @@ -76,6 +103,16 @@ func (s *UseCaseSuite) Test_AvailableScenarios() { result = s.uc.RemoteEntitiesScenarios() assert.Equal(s.T(), 0, len(result)) + + s.uc.updateRemoteEntityScenarios(s.monitoredEntity, []model.UseCaseScenarioSupportType{1, 2, 3}) + + result = s.uc.RemoteEntitiesScenarios() + assert.Equal(s.T(), 1, len(result)) + + s.uc.removeDeviceFromAvailableEntityScenarios(s.monitoredEntity.Device()) + + result = s.uc.RemoteEntitiesScenarios() + assert.Equal(s.T(), 0, len(result)) } func (s *UseCaseSuite) Test_RequiredServerFeatures() {