From 05b21d80e8f1062ce43932a15ae27a3908f48491 Mon Sep 17 00:00:00 2001 From: Lia Stratopoulos <167905060+lia-viam@users.noreply.github.com> Date: Thu, 1 May 2025 15:26:08 -0400 Subject: [PATCH 1/4] condition var refresh thread --- src/viam/sdk/robot/client.cpp | 23 ++++++++++++++++------- src/viam/sdk/robot/client.hpp | 4 +++- 2 files changed, 19 insertions(+), 8 deletions(-) diff --git a/src/viam/sdk/robot/client.cpp b/src/viam/sdk/robot/client.cpp index 740c50fff..6a353bfd1 100644 --- a/src/viam/sdk/robot/client.cpp +++ b/src/viam/sdk/robot/client.cpp @@ -143,7 +143,11 @@ RobotClient::~RobotClient() { } void RobotClient::close() { - should_refresh_.store(false); + if (should_refresh_) { + std::unique_lock lk{refresh_lock_}; + should_refresh_ = false; + refresh_cv_.notify_one(); + } if (refresh_thread_.joinable()) { refresh_thread_.join(); @@ -220,16 +224,21 @@ void RobotClient::refresh() { } void RobotClient::refresh_every() { - while (should_refresh_.load()) { - try { - std::this_thread::sleep_for(refresh_interval_); - refresh(); + std::unique_lock lk{refresh_lock_}; - } catch (std::exception&) { + while (true) { + if (refresh_cv_.wait_for(lk, refresh_interval_) == std::cv_status::timeout) { + try { + refresh(); + } catch (const std::exception& e) { + VIAM_SDK_LOG(warn) << "Refresh thread terminated with exception: " << e.what(); + break; + } + } else if (should_refresh_ == false) { break; } } -}; +} RobotClient::RobotClient(ViamChannel channel) : viam_channel_(std::move(channel)), diff --git a/src/viam/sdk/robot/client.hpp b/src/viam/sdk/robot/client.hpp index 3c19a9edc..be10c248e 100644 --- a/src/viam/sdk/robot/client.hpp +++ b/src/viam/sdk/robot/client.hpp @@ -186,7 +186,9 @@ class RobotClient { void refresh_every(); std::thread refresh_thread_; - std::atomic should_refresh_; + std::mutex refresh_lock_; + std::condition_variable refresh_cv_; + bool should_refresh_; std::chrono::seconds refresh_interval_; ViamChannel viam_channel_; From a2c8bf2ef0c33269566a4975c7eef5af9139f37f Mon Sep 17 00:00:00 2001 From: Lia Stratopoulos <167905060+lia-viam@users.noreply.github.com> Date: Thu, 1 May 2025 15:45:04 -0400 Subject: [PATCH 2/4] include cond var --- src/viam/sdk/robot/client.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/viam/sdk/robot/client.hpp b/src/viam/sdk/robot/client.hpp index be10c248e..6e6a36112 100644 --- a/src/viam/sdk/robot/client.hpp +++ b/src/viam/sdk/robot/client.hpp @@ -3,7 +3,7 @@ /// @brief gRPC client implementation for a `robot`. #pragma once -#include +#include #include #include From a78291f58bf507c9a767d222f263269bd4c06e85 Mon Sep 17 00:00:00 2001 From: Lia Stratopoulos <167905060+lia-viam@users.noreply.github.com> Date: Thu, 1 May 2025 17:11:21 -0400 Subject: [PATCH 3/4] concise refresh --- src/viam/sdk/robot/client.cpp | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/src/viam/sdk/robot/client.cpp b/src/viam/sdk/robot/client.cpp index 6a353bfd1..5f8a8332b 100644 --- a/src/viam/sdk/robot/client.cpp +++ b/src/viam/sdk/robot/client.cpp @@ -226,18 +226,19 @@ void RobotClient::refresh() { void RobotClient::refresh_every() { std::unique_lock lk{refresh_lock_}; - while (true) { - if (refresh_cv_.wait_for(lk, refresh_interval_) == std::cv_status::timeout) { + refresh_cv_.wait_for(lk, refresh_interval_, [this] { + if (should_refresh_) { try { refresh(); } catch (const std::exception& e) { - VIAM_SDK_LOG(warn) << "Refresh thread terminated with exception: " << e.what(); - break; + VIAM_SDK_LOG(warn) << "Refresh thread got exception " << e.what(); + // TODO: maybe recoverable + return true; } - } else if (should_refresh_ == false) { - break; } - } + + return !should_refresh_; + }); } RobotClient::RobotClient(ViamChannel channel) From 7198e15e28b5ffdf9751b4f29b898f3040c18652 Mon Sep 17 00:00:00 2001 From: lia <167905060+lia-viam@users.noreply.github.com> Date: Fri, 2 May 2025 10:05:35 -0400 Subject: [PATCH 4/4] const lock guard --- src/viam/sdk/robot/client.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/viam/sdk/robot/client.cpp b/src/viam/sdk/robot/client.cpp index 5f8a8332b..c4ad0c37f 100644 --- a/src/viam/sdk/robot/client.cpp +++ b/src/viam/sdk/robot/client.cpp @@ -144,7 +144,7 @@ RobotClient::~RobotClient() { void RobotClient::close() { if (should_refresh_) { - std::unique_lock lk{refresh_lock_}; + const std::unique_lock lk{refresh_lock_}; should_refresh_ = false; refresh_cv_.notify_one(); }