Skip to content

Commit

Permalink
Fix subscription removal for multiple remotes (#26)
Browse files Browse the repository at this point in the history 
- 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
  • Loading branch information
@DerAndereAndi
DerAndereAndi authored Aug 6, 2024
2 parents f94715c + d9e6711 commit c994673
Show file tree
Hide file tree
Showing 5 changed files with 132 additions and 8 deletions.
5 changes: 3 additions & 2 deletions spine/helper_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
21 changes: 21 additions & 0 deletions spine/nodemanagement_detaileddiscovery_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
7 changes: 5 additions & 2 deletions spine/subscription_manager.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down Expand Up @@ -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
}
Expand Down
46 changes: 42 additions & 4 deletions spine/subscription_manager_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) {}
Expand All @@ -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)
}

Expand All @@ -49,13 +53,28 @@ 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(),
ServerAddress: localFeature.Address(),
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)
Expand All @@ -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(),
Expand All @@ -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))
}
61 changes: 61 additions & 0 deletions spine/testdata/wallbox_detaileddiscoverydata_recv_notify_remove.json
View file
Original file line number Diff line number Diff line change
@@ -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"
}
}
]
}
}
]
}
}
}

0 comments on commit c994673

Please sign in to comment.