From 71fb5f314d130fad4898cb7226d4526ae20344ae Mon Sep 17 00:00:00 2001 From: Yufeng Wang Date: Thu, 30 May 2024 10:23:35 -0700 Subject: [PATCH] Address review comments --- .../interactive/InteractiveCommands.cpp | 41 ++++++++++++++++--- examples/fabric-admin/main.cpp | 16 +------- examples/fabric-admin/rpc/RpcClient.cpp | 3 -- examples/fabric-admin/rpc/RpcServer.cpp | 2 +- .../fabric-bridge-app/linux/RpcClient.cpp | 3 -- examples/fabric-bridge-app/linux/main.cpp | 35 +++++++++------- 6 files changed, 58 insertions(+), 42 deletions(-) diff --git a/examples/fabric-admin/commands/interactive/InteractiveCommands.cpp b/examples/fabric-admin/commands/interactive/InteractiveCommands.cpp index aaf3c36461cffb..c5c844256f5351 100644 --- a/examples/fabric-admin/commands/interactive/InteractiveCommands.cpp +++ b/examples/fabric-admin/commands/interactive/InteractiveCommands.cpp @@ -28,11 +28,18 @@ #include #include +#if defined(PW_RPC_ENABLED) +#include +#endif + +using namespace chip; + +namespace { + constexpr char kInteractiveModePrompt[] = ">>> "; constexpr char kInteractiveModeHistoryFileName[] = "chip_tool_history"; constexpr char kInteractiveModeStopCommand[] = "quit()"; - -namespace { +constexpr uint16_t kRetryIntervalS = 5; // File pointer for the log file FILE * sLogFile = nullptr; @@ -67,7 +74,7 @@ void ENFORCE_FORMAT(3, 0) LoggingCallback(const char * module, uint8_t category, return; } - uint64_t timeMs = chip::System::SystemClock().GetMonotonicMilliseconds64().count(); + uint64_t timeMs = System::SystemClock().GetMonotonicMilliseconds64().count(); uint64_t seconds = timeMs / 1000; uint64_t milliseconds = timeMs % 1000; @@ -82,6 +89,26 @@ void ENFORCE_FORMAT(3, 0) LoggingCallback(const char * module, uint8_t category, funlockfile(sLogFile); } +#if defined(PW_RPC_ENABLED) +void AttemptRpcClientConnect(System::Layer * systemLayer, void * appState) +{ + if (InitRpcClient(kFabricBridgeServerPort) == CHIP_NO_ERROR) + { + ChipLogProgress(NotSpecified, "Connected to Fabric-Bridge"); + } + else + { + ChipLogError(NotSpecified, "Failed to connect to Fabric-Bridge, retry in %d seconds....", kRetryIntervalS); + systemLayer->StartTimer(System::Clock::Seconds16(kRetryIntervalS), AttemptRpcClientConnect, nullptr); + } +} + +void ExecuteDeferredConnect(intptr_t ignored) +{ + AttemptRpcClientConnect(&DeviceLayer::SystemLayer(), nullptr); +} +#endif + } // namespace char * InteractiveStartCommand::GetCommand(char * command) @@ -134,9 +161,13 @@ CHIP_ERROR InteractiveStartCommand::RunCommand() OpenLogFile(mLogFilePath.Value()); // Redirect logs to the custom logging callback - chip::Logging::SetLogRedirectCallback(LoggingCallback); + Logging::SetLogRedirectCallback(LoggingCallback); } +#if defined(PW_RPC_ENABLED) + DeviceLayer::PlatformMgr().ScheduleWork(ExecuteDeferredConnect, 0); +#endif + char * command = nullptr; int status; while (true) @@ -167,7 +198,7 @@ bool InteractiveCommand::ParseCommand(char * command, int * status) // If scheduling the cleanup fails, there is not much we can do. // But if something went wrong while the application is leaving it could be because things have // not been cleaned up properly, so it is still useful to log the failure. - LogErrorOnFailure(chip::DeviceLayer::PlatformMgr().ScheduleWork(ExecuteDeferredCleanups, 0)); + LogErrorOnFailure(DeviceLayer::PlatformMgr().ScheduleWork(ExecuteDeferredCleanups, 0)); return false; } diff --git a/examples/fabric-admin/main.cpp b/examples/fabric-admin/main.cpp index 450586cb5f86db..f5f98cc0d960db 100644 --- a/examples/fabric-admin/main.cpp +++ b/examples/fabric-admin/main.cpp @@ -25,33 +25,19 @@ #include #include -#include #include #if defined(PW_RPC_ENABLED) -#include #include #endif -#define RETRY_INTERVAL_S (3) +using namespace chip; void ApplicationInit() { #if defined(PW_RPC_ENABLED) InitRpcServer(kFabricAdminServerPort); ChipLogProgress(NotSpecified, "PW_RPC initialized."); - - while (true) - { - if (InitRpcClient(kFabricBridgeServerPort) == CHIP_NO_ERROR) - { - ChipLogProgress(NotSpecified, "Connected to Fabric-Bridge"); - break; - } - - ChipLogError(NotSpecified, "Failed to connect to Fabric-Bridge, retry in %d seconds....", RETRY_INTERVAL_S); - std::this_thread::sleep_for(std::chrono::seconds(RETRY_INTERVAL_S)); - } #endif } diff --git a/examples/fabric-admin/rpc/RpcClient.cpp b/examples/fabric-admin/rpc/RpcClient.cpp index 0be1668f2daeac..b9d309e08c5730 100644 --- a/examples/fabric-admin/rpc/RpcClient.cpp +++ b/examples/fabric-admin/rpc/RpcClient.cpp @@ -141,12 +141,9 @@ CHIP_ERROR InitRpcClient(uint16_t rpcServerPort) { if (rpcSocketStream.Connect(rpcServerAddress, rpcServerPort) != PW_STATUS_OK) { - ChipLogError(NotSpecified, "Failed to connect the Fabric-Admin"); return CHIP_ERROR_NOT_CONNECTED; } - ChipLogProgress(NotSpecified, "Connectted to the Fabric-Admin\n"); - // Start a thread to process incoming packets std::thread packet_processor(ProcessPackets); packet_processor.detach(); diff --git a/examples/fabric-admin/rpc/RpcServer.cpp b/examples/fabric-admin/rpc/RpcServer.cpp index 769d430786199a..1fb264bd65c53a 100644 --- a/examples/fabric-admin/rpc/RpcServer.cpp +++ b/examples/fabric-admin/rpc/RpcServer.cpp @@ -36,7 +36,7 @@ class FabricAdmin final : public chip::rpc::FabricAdmin pw::Status OpenCommissioningWindow(const chip_rpc_DeviceInfo & request, chip_rpc_OperationStatus & response) override { chip::NodeId nodeId = request.node_id; - printf("Received OpenCommissioningWindow request: 0x%llx\n", nodeId); + printf("Received OpenCommissioningWindow request: 0x%lx\n", nodeId); response.success = false; return pw::OkStatus(); diff --git a/examples/fabric-bridge-app/linux/RpcClient.cpp b/examples/fabric-bridge-app/linux/RpcClient.cpp index 57cfcb23a82e9b..a91f52dbf2bc75 100644 --- a/examples/fabric-bridge-app/linux/RpcClient.cpp +++ b/examples/fabric-bridge-app/linux/RpcClient.cpp @@ -141,12 +141,9 @@ CHIP_ERROR InitRpcClient(uint16_t rpcServerPort) { if (rpcSocketStream.Connect(rpcServerAddress, rpcServerPort) != PW_STATUS_OK) { - ChipLogError(NotSpecified, "Failed to connect the Fabric-Admin"); return CHIP_ERROR_NOT_CONNECTED; } - ChipLogProgress(NotSpecified, "Connectted to the Fabric-Admin\n"); - // Start a thread to process incoming packets std::thread packet_processor(ProcessPackets); packet_processor.detach(); diff --git a/examples/fabric-bridge-app/linux/main.cpp b/examples/fabric-bridge-app/linux/main.cpp index 5a1cefcfbffedd..e19c06aa476bd1 100644 --- a/examples/fabric-bridge-app/linux/main.cpp +++ b/examples/fabric-bridge-app/linux/main.cpp @@ -62,11 +62,11 @@ using namespace chip::Transport; using namespace chip::DeviceLayer; using namespace chip::app::Clusters; -#define POLL_INTERVAL_MS (100) -#define RETRY_INTERVAL_S (3) - namespace { +constexpr uint16_t kPollIntervalMs = 100; +constexpr uint16_t kRetryIntervalS = 3; + EndpointId gCurrentEndpointId; EndpointId gFirstDynamicEndpointId; Device * gDevices[CHIP_DEVICE_CONFIG_DYNAMIC_ENDPOINT_COUNT + 1]; @@ -103,9 +103,24 @@ void BridgePollingThread() } // Sleep to avoid tight loop reading commands - usleep(POLL_INTERVAL_MS * 1000); + usleep(kPollIntervalMs * 1000); + } +} + +#if defined(PW_RPC_FABRIC_BRIDGE_SERVICE) +void AttemptRpcClientConnect(System::Layer * systemLayer, void * appState) +{ + if (InitRpcClient(kFabricAdminServerPort) == CHIP_NO_ERROR) + { + ChipLogProgress(NotSpecified, "Connected to Fabric-Admin"); + } + else + { + ChipLogError(NotSpecified, "Failed to connect to Fabric-Admin, retry in %d seconds....", kRetryIntervalS); + systemLayer->StartTimer(System::Clock::Seconds16(kRetryIntervalS), AttemptRpcClientConnect, nullptr); } } +#endif } // namespace @@ -302,17 +317,7 @@ void ApplicationInit() #if defined(PW_RPC_FABRIC_BRIDGE_SERVICE) InitRpcServer(kFabricBridgeServerPort); - while (true) - { - if (InitRpcClient(kFabricAdminServerPort) == CHIP_NO_ERROR) - { - ChipLogProgress(NotSpecified, "Connected to Fabric-Admin"); - break; - } - - ChipLogError(NotSpecified, "Failed to connect to Fabric-Admin, retry in %d seconds....", RETRY_INTERVAL_S); - std::this_thread::sleep_for(std::chrono::seconds(RETRY_INTERVAL_S)); - } + AttemptRpcClientConnect(&DeviceLayer::SystemLayer(), nullptr); #endif // Start a thread for bridge polling