From d9e6711cfbcb588151198b359137a627145bb70e Mon Sep 17 00:00:00 2001 From: Andreas Linde Date: Tue, 6 Aug 2024 15:20:29 +0200 Subject: [PATCH] Fix subscription removal for multiple remotes - When a service has multiple similar remote devices (EVSEs with attached EVs), then removing an EV did remove the subscriptions for all remote EVs and therefor these wallboxes switched into fallback mode. This change fixes this - Add more tests for NodeManagementDetailedDiscovery entity removal --- spine/helper_test.go | 5 +- .../nodemanagement_detaileddiscovery_test.go | 21 +++++++ spine/subscription_manager.go | 7 ++- spine/subscription_manager_test.go | 46 ++++++++++++-- ...aileddiscoverydata_recv_notify_remove.json | 61 +++++++++++++++++++ 5 files changed, 132 insertions(+), 8 deletions(-) create mode 100644 spine/testdata/wallbox_detaileddiscoverydata_recv_notify_remove.json diff --git a/spine/helper_test.go b/spine/helper_test.go index 3a04a75..bbcc56d 100644 --- a/spine/helper_test.go +++ b/spine/helper_test.go @@ -18,8 +18,9 @@ import ( ) const ( - wallbox_detaileddiscoverydata_recv_reply_file_path = "./testdata/wallbox_detaileddiscoverydata_recv_reply.json" - wallbox_detaileddiscoverydata_recv_notify_file_path = "./testdata/wallbox_detaileddiscoverydata_recv_notify.json" + wallbox_detaileddiscoverydata_recv_reply_file_path = "./testdata/wallbox_detaileddiscoverydata_recv_reply.json" + wallbox_detaileddiscoverydata_recv_notify_file_path = "./testdata/wallbox_detaileddiscoverydata_recv_notify.json" + wallbox_detaileddiscoverydata_recv_notify_remove_file_path = "./testdata/wallbox_detaileddiscoverydata_recv_notify_remove.json" ) type WriteMessageHandler struct { diff --git a/spine/nodemanagement_detaileddiscovery_test.go b/spine/nodemanagement_detaileddiscovery_test.go index e0e881d..653a80d 100644 --- a/spine/nodemanagement_detaileddiscovery_test.go +++ b/spine/nodemanagement_detaileddiscovery_test.go @@ -153,6 +153,27 @@ func (s *NodeManagementSuite) TestDetailedDiscovery_RecvNotifyAdded() { assert.Equal(s.T(), 10, len(ev.Features())) } } + + // Act + msgCounter, _ = s.remoteDevice.HandleSpineMesssage(loadFileData(s.T(), wallbox_detaileddiscoverydata_recv_notify_remove_file_path)) + waitForAck(s.T(), msgCounter, s.writeHandler) + + // Assert + rEntities = remoteDevice.Entities() + if assert.Equal(s.T(), 2, len(rEntities)) { + { + di := rEntities[DeviceInformationEntityId] + assert.NotNil(s.T(), di) + assert.Equal(s.T(), model.EntityTypeTypeDeviceInformation, di.EntityType()) + assert.Equal(s.T(), 2, len(di.Features())) + } + { + evse := rEntities[1] + assert.NotNil(s.T(), evse) + assert.Equal(s.T(), model.EntityTypeTypeEVSE, evse.EntityType()) + assert.Equal(s.T(), 3, len(evse.Features())) + } + } } func (s *NodeManagementSuite) TestDetailedDiscovery_SendReplyWithAcknowledge() { diff --git a/spine/subscription_manager.go b/spine/subscription_manager.go index a0d9a9b..9b7951d 100644 --- a/spine/subscription_manager.go +++ b/spine/subscription_manager.go @@ -114,7 +114,9 @@ func (c *SubscriptionManager) RemoveSubscription(data model.SubscriptionManageme for _, item := range c.subscriptionEntries { itemAddress := item.ClientFeature.Address() - if !reflect.DeepEqual(*itemAddress, clientAddress) && + if !reflect.DeepEqual(itemAddress.Device, clientAddress.Device) || + !reflect.DeepEqual(itemAddress.Entity, clientAddress.Entity) || + !reflect.DeepEqual(itemAddress.Feature, clientAddress.Feature) || !reflect.DeepEqual(item.ServerFeature, serverFeature) { newSubscriptionEntries = append(newSubscriptionEntries, item) } @@ -163,7 +165,8 @@ func (c *SubscriptionManager) RemoveSubscriptionsForEntity(remoteEntity api.Enti var newSubscriptionEntries []*api.SubscriptionEntry for _, item := range c.subscriptionEntries { - if !reflect.DeepEqual(item.ClientFeature.Address().Entity, remoteEntity.Address().Entity) { + if !reflect.DeepEqual(item.ClientFeature.Address().Device, remoteEntity.Address().Device) || + !reflect.DeepEqual(item.ClientFeature.Address().Entity, remoteEntity.Address().Entity) { newSubscriptionEntries = append(newSubscriptionEntries, item) continue } diff --git a/spine/subscription_manager_test.go b/spine/subscription_manager_test.go index a3cc455..4e0102e 100644 --- a/spine/subscription_manager_test.go +++ b/spine/subscription_manager_test.go @@ -18,9 +18,10 @@ func TestSubscriptionManagerSuite(t *testing.T) { type SubscriptionManagerSuite struct { suite.Suite - localDevice api.DeviceLocalInterface - remoteDevice api.DeviceRemoteInterface - sut api.SubscriptionManagerInterface + localDevice api.DeviceLocalInterface + remoteDevice, + remoteDevice2 api.DeviceRemoteInterface + sut api.SubscriptionManagerInterface } func (suite *SubscriptionManagerSuite) WriteShipMessageWithPayload([]byte) {} @@ -31,9 +32,12 @@ func (suite *SubscriptionManagerSuite) SetupSuite() { ski := "test" sender := NewSender(suite) suite.remoteDevice = NewDeviceRemote(suite.localDevice, ski, sender) - _ = suite.localDevice.SetupRemoteDevice(ski, suite) + ski2 := "test2" + suite.remoteDevice2 = NewDeviceRemote(suite.localDevice, ski2, sender) + _ = suite.localDevice.SetupRemoteDevice(ski2, suite) + suite.sut = NewSubscriptionManager(suite.localDevice) } @@ -49,6 +53,7 @@ func (suite *SubscriptionManagerSuite) Test_Subscriptions() { remoteFeature := NewFeatureRemote(remoteEntity.NextFeatureId(), remoteEntity, model.FeatureTypeTypeDeviceDiagnosis, model.RoleTypeClient) remoteFeature.Address().Device = util.Ptr(model.AddressDeviceType("remoteDevice")) remoteEntity.AddFeature(remoteFeature) + remoteEntity.Address().Device = util.Ptr(model.AddressDeviceType("remoteDevice")) subscrRequest := model.SubscriptionManagementRequestCallType{ ClientAddress: remoteFeature.Address(), @@ -56,6 +61,20 @@ func (suite *SubscriptionManagerSuite) Test_Subscriptions() { ServerFeatureType: util.Ptr(model.FeatureTypeTypeDeviceDiagnosis), } + remoteEntity2 := NewEntityRemote(suite.remoteDevice2, model.EntityTypeTypeEVSE, []model.AddressEntityType{1}) + suite.remoteDevice2.AddEntity(remoteEntity2) + + remoteFeature2 := NewFeatureRemote(remoteEntity2.NextFeatureId(), remoteEntity2, model.FeatureTypeTypeDeviceDiagnosis, model.RoleTypeClient) + remoteFeature2.Address().Device = util.Ptr(model.AddressDeviceType("remoteDevice2")) + remoteEntity2.AddFeature(remoteFeature2) + remoteEntity2.Address().Device = util.Ptr(model.AddressDeviceType("remoteDevice2")) + + subscrRequest2 := model.SubscriptionManagementRequestCallType{ + ClientAddress: remoteFeature2.Address(), + ServerAddress: localFeature.Address(), + ServerFeatureType: util.Ptr(model.FeatureTypeTypeDeviceDiagnosis), + } + subMgr := suite.localDevice.SubscriptionManager() err := subMgr.AddSubscription(suite.remoteDevice, subscrRequest) assert.Nil(suite.T(), err) @@ -69,6 +88,12 @@ func (suite *SubscriptionManagerSuite) Test_Subscriptions() { subs = subMgr.Subscriptions(suite.remoteDevice) assert.Equal(suite.T(), 1, len(subs)) + err = subMgr.AddSubscription(suite.remoteDevice2, subscrRequest2) + assert.Nil(suite.T(), err) + + subs = subMgr.Subscriptions(suite.remoteDevice2) + assert.Equal(suite.T(), 1, len(subs)) + subscrDelete := model.SubscriptionManagementDeleteCallType{ ClientAddress: remoteFeature.Address(), ServerAddress: localFeature.Address(), @@ -90,8 +115,21 @@ func (suite *SubscriptionManagerSuite) Test_Subscriptions() { subs = subMgr.Subscriptions(suite.remoteDevice) assert.Equal(suite.T(), 1, len(subs)) + subMgr.RemoveSubscriptionsForEntity(nil) + + subs = subMgr.Subscriptions(suite.remoteDevice) + assert.Equal(suite.T(), 1, len(subs)) + + subMgr.RemoveSubscriptionsForDevice(nil) + + subs = subMgr.Subscriptions(suite.remoteDevice) + assert.Equal(suite.T(), 1, len(subs)) + subMgr.RemoveSubscriptionsForDevice(suite.remoteDevice) subs = subMgr.Subscriptions(suite.remoteDevice) assert.Equal(suite.T(), 0, len(subs)) + + subs = subMgr.Subscriptions(suite.remoteDevice2) + assert.Equal(suite.T(), 1, len(subs)) } diff --git a/spine/testdata/wallbox_detaileddiscoverydata_recv_notify_remove.json b/spine/testdata/wallbox_detaileddiscoverydata_recv_notify_remove.json new file mode 100644 index 0000000..b89477f --- /dev/null +++ b/spine/testdata/wallbox_detaileddiscoverydata_recv_notify_remove.json @@ -0,0 +1,61 @@ +{ + "datagram": { + "header": { + "specificationVersion": "1.3.0", + "addressSource": { + "device": "Wallbox", + "entity": [ + 0 + ], + "feature": 0 + }, + "addressDestination": { + "device": "HEMS", + "entity": [ + 0 + ], + "feature": 0 + }, + "msgCounter": 4, + "cmdClassifier": "notify", + "ackRequest":true + }, + "payload": { + "cmd": [ + { + "function": "nodeManagementDetailedDiscoveryData", + "filter": [ + { + "cmdControl": { + "partial": {} + } + } + ], + "nodeManagementDetailedDiscoveryData": { + "deviceInformation": { + "description": { + "deviceAddress": { + "device": "Wallbox" + } + } + }, + "entityInformation": [ + { + "description": { + "entityAddress": { + "entity": [ + 1, + 1 + ] + }, + "entityType": "EV", + "lastStateChange": "removed" + } + } + ] + } + } + ] + } + } +} \ No newline at end of file