From 44db0f660e20ed996add613b2a7ffb4f63955316 Mon Sep 17 00:00:00 2001 From: Terence Hampson Date: Mon, 21 Oct 2024 22:38:19 -0400 Subject: [PATCH] Call DeviceReachableChanged when subscription fails (#36175) * Call DeviceReachableChanged when subscription fails * Restyled by clang-format --------- Co-authored-by: Restyled.io --- .../device_manager/DeviceSubscription.cpp | 21 +++++++++++++++++- .../device_manager/DeviceSynchronization.cpp | 6 ++++- examples/fabric-admin/rpc/RpcClient.cpp | 22 +++++++++++++++++++ examples/fabric-admin/rpc/RpcClient.h | 11 ++++++++++ 4 files changed, 58 insertions(+), 2 deletions(-) diff --git a/examples/fabric-admin/device_manager/DeviceSubscription.cpp b/examples/fabric-admin/device_manager/DeviceSubscription.cpp index 0da2bdddd4fbf5..434d349e8c5113 100644 --- a/examples/fabric-admin/device_manager/DeviceSubscription.cpp +++ b/examples/fabric-admin/device_manager/DeviceSubscription.cpp @@ -125,6 +125,16 @@ void DeviceSubscription::OnDone(ReadClient * apReadClient) void DeviceSubscription::OnError(CHIP_ERROR error) { +#if defined(PW_RPC_ENABLED) + if (error == CHIP_ERROR_TIMEOUT && mState == State::SubscriptionStarted) + { + chip_rpc_ReachabilityChanged reachabilityChanged; + reachabilityChanged.has_id = true; + reachabilityChanged.id = mCurrentAdministratorCommissioningAttributes.id; + reachabilityChanged.reachability = false; + DeviceReachableChanged(reachabilityChanged); + } +#endif ChipLogProgress(NotSpecified, "Error subscribing: %" CHIP_ERROR_FORMAT, error.Format()); } @@ -198,7 +208,16 @@ void DeviceSubscription::OnDeviceConnectionFailure(const ScopedNodeId & peerId, { VerifyOrDie(mState == State::Connecting || mState == State::Stopping); ChipLogError(NotSpecified, "DeviceSubscription failed to connect to " ChipLogFormatX64, ChipLogValueX64(peerId.GetNodeId())); - // TODO(#35333) Figure out how we should recover if we fail to connect and mState == State::Connecting. +#if defined(PW_RPC_ENABLED) + if (mState == State::Connecting) + { + chip_rpc_ReachabilityChanged reachabilityChanged; + reachabilityChanged.has_id = true; + reachabilityChanged.id = mCurrentAdministratorCommissioningAttributes.id; + reachabilityChanged.reachability = false; + DeviceReachableChanged(reachabilityChanged); + } +#endif // After calling mOnDoneCallback we are indicating that `this` is deleted and we shouldn't do anything else with // DeviceSubscription. diff --git a/examples/fabric-admin/device_manager/DeviceSynchronization.cpp b/examples/fabric-admin/device_manager/DeviceSynchronization.cpp index 6c2284018e0d19..1e8728ee59b567 100644 --- a/examples/fabric-admin/device_manager/DeviceSynchronization.cpp +++ b/examples/fabric-admin/device_manager/DeviceSynchronization.cpp @@ -277,12 +277,16 @@ void DeviceSynchronizer::SynchronizationCompleteAddDevice() if (!mCurrentDeviceData.is_icd) { VerifyOrDie(mController); - // TODO(#35333) Figure out how we should recover in this circumstance. ScopedNodeId scopedNodeId(mNodeId, mController->GetFabricIndex()); CHIP_ERROR err = DeviceSubscriptionManager::Instance().StartSubscription(*mController, scopedNodeId); if (err != CHIP_NO_ERROR) { ChipLogError(NotSpecified, "Failed start subscription to NodeId:" ChipLogFormatX64, ChipLogValueX64(mNodeId)); + chip_rpc_ReachabilityChanged reachabilityChanged; + reachabilityChanged.has_id = true; + reachabilityChanged.id = mCurrentDeviceData.id; + reachabilityChanged.reachability = false; + DeviceReachableChanged(reachabilityChanged); } } #endif diff --git a/examples/fabric-admin/rpc/RpcClient.cpp b/examples/fabric-admin/rpc/RpcClient.cpp index e33dd0472e6fe2..9964dc6767b67e 100644 --- a/examples/fabric-admin/rpc/RpcClient.cpp +++ b/examples/fabric-admin/rpc/RpcClient.cpp @@ -205,3 +205,25 @@ CHIP_ERROR AdminCommissioningAttributeChanged(const chip_rpc_AdministratorCommis return WaitForResponse(call); } + +CHIP_ERROR DeviceReachableChanged(const chip_rpc_ReachabilityChanged & data) +{ + ChipLogProgress(NotSpecified, "DeviceReachableChanged"); + // TODO(#35333): When there is some sort of device manager in fabric-admin that handles all the devices we + // are currently connected to (and not just device on the remote bridge), we should notify that manager + // so that it can properly handle any sort of reconnection logic. This can either be done here when + // `data.reachability == false`, or more control can be given wherever DeviceReachableChanged is currently + // called + + // The RPC call is kept alive until it completes. When a response is received, it will be logged by the handler + // function and the call will complete. + auto call = fabricBridgeClient.DeviceReachableChanged(data, RpcCompletedWithEmptyResponse); + + if (!call.active()) + { + // The RPC call was not sent. This could occur due to, for example, an invalid channel ID. Handle if necessary. + return CHIP_ERROR_INTERNAL; + } + + return WaitForResponse(call); +} diff --git a/examples/fabric-admin/rpc/RpcClient.h b/examples/fabric-admin/rpc/RpcClient.h index da6a82115a3cd2..a4cefe8bd8e076 100644 --- a/examples/fabric-admin/rpc/RpcClient.h +++ b/examples/fabric-admin/rpc/RpcClient.h @@ -88,3 +88,14 @@ CHIP_ERROR ActiveChanged(chip::ScopedNodeId scopedNodeId, uint32_t promisedActiv * - CHIP_ERROR_INTERNAL: An internal error occurred while activating the RPC call. */ CHIP_ERROR AdminCommissioningAttributeChanged(const chip_rpc_AdministratorCommissioningChanged & data); + +/** + * @brief Notify that reachachability of the bridged device has changed + * + * @param data information regarding change in reachability of the bridged device. + * @return CHIP_ERROR An error code indicating the success or failure of the operation. + * - CHIP_NO_ERROR: The RPC command was successfully processed. + * - CHIP_ERROR_BUSY: Another operation is currently in progress. + * - CHIP_ERROR_INTERNAL: An internal error occurred while activating the RPC call. + */ +CHIP_ERROR DeviceReachableChanged(const chip_rpc_ReachabilityChanged & data);