From ba02d79e3b4c46aa776736e7dcb4e0e767f49b98 Mon Sep 17 00:00:00 2001 From: Sharad Binjola <31142146+sharadb-amazon@users.noreply.github.com> Date: Thu, 23 Feb 2023 07:09:13 -0800 Subject: [PATCH] On kBindingsChangedViaCluster during UDC, pick target video player's nodeID from BindingTable (#24706) * tv-casting-app: On kBindingsChangedViaCluster during UDC, pick out nodeId of video player from BindingTable * Avoiding passing a default accessing fabricIndex * Setting mAccessingFabricIndex used in NotifyBindingsChanged() as data member of BindingTableAccess --- .../tv-casting-common/src/CastingServer.cpp | 61 ++++++++++--------- src/app/clusters/bindings/bindings.cpp | 11 ++-- src/include/platform/CHIPDeviceEvent.h | 4 ++ 3 files changed, 42 insertions(+), 34 deletions(-) diff --git a/examples/tv-casting-app/tv-casting-common/src/CastingServer.cpp b/examples/tv-casting-app/tv-casting-common/src/CastingServer.cpp index 56c6845eb5bbf7..fa4138deeeba43 100644 --- a/examples/tv-casting-app/tv-casting-common/src/CastingServer.cpp +++ b/examples/tv-casting-app/tv-casting-common/src/CastingServer.cpp @@ -345,7 +345,8 @@ void CastingServer::DeviceEventCallback(const DeviceLayer::ChipDeviceEvent * eve if (event->Type == DeviceLayer::DeviceEventType::kBindingsChangedViaCluster) { ChipLogProgress(AppServer, "CastingServer::DeviceEventCallback kBindingsChangedViaCluster received"); - if (CastingServer::GetInstance()->GetActiveTargetVideoPlayer()->IsInitialized()) + if (CastingServer::GetInstance()->GetActiveTargetVideoPlayer()->IsInitialized() && + CastingServer::GetInstance()->GetActiveTargetVideoPlayer()->GetOperationalDeviceProxy() != nullptr) { ChipLogProgress(AppServer, "CastingServer::DeviceEventCallback already connected to video player, reading server clusters"); @@ -355,40 +356,40 @@ void CastingServer::DeviceEventCallback(const DeviceLayer::ChipDeviceEvent * eve else if (CastingServer::GetInstance()->mUdcInProgress) { ChipLogProgress(AppServer, - "CastingServer::DeviceEventCallback UDC is in progress while handling kBindingsChangedViaCluster"); + "CastingServer::DeviceEventCallback UDC is in progress while handling kBindingsChangedViaCluster with " + "fabricIndex: %d", + event->BindingsChanged.fabricIndex); CastingServer::GetInstance()->mUdcInProgress = false; - if (CastingServer::GetInstance()->mTargetVideoPlayerNumIPs > 0) - { - TargetVideoPlayerInfo * connectableVideoPlayerList = - CastingServer::GetInstance()->ReadCachedTargetVideoPlayerInfos(); - if (connectableVideoPlayerList == nullptr || !connectableVideoPlayerList[0].IsInitialized()) - { - ChipLogError(AppServer, "CastingServer::DeviceEventCallback No cached video players found"); - CastingServer::GetInstance()->mCommissioningCompleteCallback(CHIP_ERROR_INCORRECT_STATE); - return; - } - for (size_t i = 0; i < kMaxCachedVideoPlayers && connectableVideoPlayerList[i].IsInitialized(); i++) + // find targetPeerNodeId from binding table by matching the binding's fabricIndex with the accessing fabricIndex + // received in BindingsChanged event + for (const auto & binding : BindingTable::GetInstance()) + { + ChipLogProgress( + AppServer, + "CastingServer::DeviceEventCallback Read cached binding type=%d fabrixIndex=%d nodeId=0x" ChipLogFormatX64 + " groupId=%d local endpoint=%d remote endpoint=%d cluster=" ChipLogFormatMEI, + binding.type, binding.fabricIndex, ChipLogValueX64(binding.nodeId), binding.groupId, binding.local, + binding.remote, ChipLogValueMEI(binding.clusterId.ValueOr(0))); + if (binding.type == EMBER_UNICAST_BINDING && event->BindingsChanged.fabricIndex == binding.fabricIndex) { - if (connectableVideoPlayerList[i].IsSameAs(CastingServer::GetInstance()->mTargetVideoPlayerDeviceName, - CastingServer::GetInstance()->mTargetVideoPlayerNumIPs, - CastingServer::GetInstance()->mTargetVideoPlayerIpAddress)) - { - ChipLogProgress(AppServer, - "CastingServer::DeviceEventCallback found the video player to initialize/connect to"); - targetPeerNodeId = connectableVideoPlayerList[i].GetNodeId(); - targetFabricIndex = connectableVideoPlayerList[i].GetFabricIndex(); - runPostCommissioning = true; - } + ChipLogProgress( + NotSpecified, + "CastingServer::DeviceEventCallback Matched accessingFabricIndex with nodeId=0x" ChipLogFormatX64, + ChipLogValueX64(binding.nodeId)); + targetPeerNodeId = binding.nodeId; + targetFabricIndex = binding.fabricIndex; + runPostCommissioning = true; + break; } + } - if (targetPeerNodeId == 0 && runPostCommissioning == false) - { - ChipLogError(AppServer, - "CastingServer::DeviceEventCallback did NOT find the video player to initialize/connect to"); - CastingServer::GetInstance()->mCommissioningCompleteCallback(CHIP_ERROR_INCORRECT_STATE); - return; - } + if (targetPeerNodeId == 0 && runPostCommissioning == false) + { + ChipLogError(AppServer, "CastingServer::DeviceEventCallback accessingFabricIndex: %d did not match bindings", + event->BindingsChanged.fabricIndex); + CastingServer::GetInstance()->mCommissioningCompleteCallback(CHIP_ERROR_INCORRECT_STATE); + return; } } } diff --git a/src/app/clusters/bindings/bindings.cpp b/src/app/clusters/bindings/bindings.cpp index 9aece569756e52..0b48cee6f6c4a4 100644 --- a/src/app/clusters/bindings/bindings.cpp +++ b/src/app/clusters/bindings/bindings.cpp @@ -54,6 +54,8 @@ class BindingTableAccess : public AttributeAccessInterface CHIP_ERROR WriteBindingTable(const ConcreteDataAttributePath & path, AttributeValueDecoder & decoder); CHIP_ERROR NotifyBindingsChanged(); + + FabricIndex mAccessingFabricIndex; }; BindingTableAccess gAttrAccess; @@ -196,19 +198,19 @@ void BindingTableAccess::OnListWriteEnd(const app::ConcreteAttributePath & aPath CHIP_ERROR BindingTableAccess::WriteBindingTable(const ConcreteDataAttributePath & path, AttributeValueDecoder & decoder) { - FabricIndex accessingFabricIndex = decoder.AccessingFabricIndex(); + mAccessingFabricIndex = decoder.AccessingFabricIndex(); if (!path.IsListOperation() || path.mListOp == ConcreteDataAttributePath::ListOperation::ReplaceAll) { DecodableBindingListType newBindingList; ReturnErrorOnFailure(decoder.Decode(newBindingList)); - ReturnErrorOnFailure(CheckValidBindingList(path.mEndpointId, newBindingList, accessingFabricIndex)); + ReturnErrorOnFailure(CheckValidBindingList(path.mEndpointId, newBindingList, mAccessingFabricIndex)); // Clear all entries for the current accessing fabric and endpoint auto bindingTableIter = BindingTable::GetInstance().begin(); while (bindingTableIter != BindingTable::GetInstance().end()) { - if (bindingTableIter->local == path.mEndpointId && bindingTableIter->fabricIndex == accessingFabricIndex) + if (bindingTableIter->local == path.mEndpointId && bindingTableIter->fabricIndex == mAccessingFabricIndex) { if (bindingTableIter->type == EMBER_UNICAST_BINDING) { @@ -255,7 +257,8 @@ CHIP_ERROR BindingTableAccess::WriteBindingTable(const ConcreteDataAttributePath CHIP_ERROR BindingTableAccess::NotifyBindingsChanged() { DeviceLayer::ChipDeviceEvent event; - event.Type = DeviceLayer::DeviceEventType::kBindingsChangedViaCluster; + event.Type = DeviceLayer::DeviceEventType::kBindingsChangedViaCluster; + event.BindingsChanged.fabricIndex = mAccessingFabricIndex; return chip::DeviceLayer::PlatformMgr().PostEvent(&event); } diff --git a/src/include/platform/CHIPDeviceEvent.h b/src/include/platform/CHIPDeviceEvent.h index 591794cbc21705..2ca528f2cd0924 100644 --- a/src/include/platform/CHIPDeviceEvent.h +++ b/src/include/platform/CHIPDeviceEvent.h @@ -494,6 +494,10 @@ struct ChipDeviceEvent final uint64_t nodeId; FabricIndex fabricIndex; } CommissioningComplete; + struct + { + FabricIndex fabricIndex; + } BindingsChanged; struct {