From 3244a8543898fd1e686d9a0fd52a186e49715f29 Mon Sep 17 00:00:00 2001 From: justagist Date: Thu, 15 Oct 2026 10:34:48 +0000 Subject: [PATCH 1/6] fix health timeout units and make it configurable per state --- .../piper_cpp/interface/timed_freq_state.h | 79 +++++++++++-------- .../src/interface/piper_interface_v2.cpp | 14 ++++ piper_cpp/test/CMakeLists.txt | 1 + piper_cpp/test/timed_freq_state_test.cpp | 64 +++++++++++++++ 4 files changed, 126 insertions(+), 32 deletions(-) create mode 100644 piper_cpp/test/timed_freq_state_test.cpp diff --git a/piper_cpp/include/piper_cpp/interface/timed_freq_state.h b/piper_cpp/include/piper_cpp/interface/timed_freq_state.h index 3b43fe7..bb7ead6 100644 --- a/piper_cpp/include/piper_cpp/interface/timed_freq_state.h +++ b/piper_cpp/include/piper_cpp/interface/timed_freq_state.h @@ -5,18 +5,21 @@ #include #include #include +#include namespace piper_cpp { +/// Pass to `TimedFreqState::setHealthTimeout()` for one-shot responses, which stay healthy +/// once received. +inline constexpr std::chrono::steady_clock::duration kNoHealthTimeout = std::chrono::steady_clock::duration::max(); + +/// A value updated from the CAN read thread, with per-part arrival tracking so callers can tell +/// a live stream from a stale one. Multipart messages register one caller per part. template class TimedFreqState { - using clock = std::chrono::steady_clock; - // if a set() call is not received within this time, the data stream is considered unhealthy. - const uint HEALTH_TIMEOUT_MS = 1000; - public: TimedFreqState() { @@ -26,7 +29,7 @@ template class TimedFreqState void setNumCallers(size_t num_callers) { std::lock_guard lk(mutex_); - last_timestamp_.resize(num_callers, 0.0); + last_received_.resize(num_callers); initialized_.resize(num_callers, false); hz_.resize(num_callers, 0.0); } @@ -34,54 +37,45 @@ template class TimedFreqState size_t getNumCallers() const { std::lock_guard lk(mutex_); - return static_cast(last_timestamp_.size()); + return last_received_.size(); + } + + /// How long after the last `set()` of any part the state stops being healthy. Defaults to + /// one second, which suits the periodic feedback streams. + void setHealthTimeout(clock::duration timeout) + { + std::lock_guard lk(mutex_); + health_timeout_ = timeout; } /// Atomically replace the contained value, and update timestamp/hz. void set(const T& newValue, double timestamp, int caller_id = 0) { std::lock_guard lk(mutex_); - double now_sec = - std::chrono::duration_cast>(clock::now().time_since_epoch()).count(); + const auto now = clock::now(); // compute hz only if we've been initialized once before if (initialized_.at(caller_id)) { - double dt = now_sec - last_timestamp_.at(caller_id); + const double dt = std::chrono::duration(now - last_received_.at(caller_id)).count(); hz_.at(caller_id) = (dt > 0.0) ? (1.0 / dt) : 0.0; } value_ = newValue; - timestamp_ = timestamp; // Use provided timestamp, not now_sec - last_timestamp_.at(caller_id) = now_sec; + timestamp_ = timestamp; // Use provided timestamp, not now + last_received_.at(caller_id) = now; initialized_.at(caller_id) = true; } bool isValid() const { std::lock_guard lk(mutex_); - return initialized_.size() > 0 && - std::all_of(initialized_.begin(), initialized_.end(), [](bool init) { return init; }); + return isValidLocked(); } bool isHealthy() const { - if (!isValid()) - { - return false; - } - { - std::lock_guard lk(mutex_); - double now_sec = - std::chrono::duration_cast>(clock::now().time_since_epoch()).count(); - for (size_t i = 0; i < last_timestamp_.size(); ++i) - { - if (last_timestamp_.at(i) + HEALTH_TIMEOUT_MS < now_sec) - { - return false; - } - } - return true; - } + std::lock_guard lk(mutex_); + return isHealthyLocked(clock::now()); } /// Atomically read out the payload @@ -100,15 +94,36 @@ template class TimedFreqState double getHz() const { std::lock_guard lk(mutex_); - return std::accumulate(hz_.begin(), hz_.end(), 0.0) / hz_.size(); + return getHzLocked(); } private: + bool isValidLocked() const + { + return initialized_.size() > 0 && + std::all_of(initialized_.begin(), initialized_.end(), [](bool init) { return init; }); + } + + bool isHealthyLocked(clock::time_point now) const + { + if (!isValidLocked()) + return false; + for (const auto& received : last_received_) + { + if (now - received > health_timeout_) + return false; + } + return true; + } + + double getHzLocked() const { return std::accumulate(hz_.begin(), hz_.end(), 0.0) / hz_.size(); } + mutable std::mutex mutex_; T value_{}; double timestamp_{0.0}; + clock::duration health_timeout_{std::chrono::seconds(1)}; std::vector hz_; // Instantaneous rate - std::vector last_timestamp_{}; + std::vector last_received_; std::vector initialized_; }; diff --git a/piper_cpp/src/interface/piper_interface_v2.cpp b/piper_cpp/src/interface/piper_interface_v2.cpp index 538ba8c..66fe49f 100644 --- a/piper_cpp/src/interface/piper_interface_v2.cpp +++ b/piper_cpp/src/interface/piper_interface_v2.cpp @@ -40,6 +40,20 @@ PiperInterfaceV2::PiperInterfaceV2( arm_motion_ctrl_code_151_.setNumCallers(1); arm_instruction_response_.setNumCallers(1); + // Query responses and command echoes arrive once, not periodically, so their age says + // nothing about the link; only the periodic feedback streams keep the default timeout. + arm_current_end_vel_acc_.setHealthTimeout(kNoHealthTimeout); + arm_crash_protection_rating_fb_.setHealthTimeout(kNoHealthTimeout); + arm_gripper_teaching_pendant_fb_.setHealthTimeout(kNoHealthTimeout); + arm_current_motor_angle_limit_max_spd_.setHealthTimeout(kNoHealthTimeout); + arm_current_motor_max_acc_limit_.setHealthTimeout(kNoHealthTimeout); + arm_all_motor_angle_limit_max_spd_.setHealthTimeout(kNoHealthTimeout); + arm_all_motor_max_acc_limit_.setHealthTimeout(kNoHealthTimeout); + arm_joint_ctrl_msgs_.setHealthTimeout(kNoHealthTimeout); + arm_gripper_ctrl_.setHealthTimeout(kNoHealthTimeout); + arm_motion_ctrl_code_151_.setHealthTimeout(kNoHealthTimeout); + arm_instruction_response_.setHealthTimeout(kNoHealthTimeout); + /// TODO: initialise other members as in python } diff --git a/piper_cpp/test/CMakeLists.txt b/piper_cpp/test/CMakeLists.txt index 3786ecf..129628e 100644 --- a/piper_cpp/test/CMakeLists.txt +++ b/piper_cpp/test/CMakeLists.txt @@ -13,3 +13,4 @@ endfunction() add_piper_test(std_can_interface_test) add_piper_test(piper_interface_v2_test) add_piper_test(piper_parser_v2_test) +add_piper_test(timed_freq_state_test) diff --git a/piper_cpp/test/timed_freq_state_test.cpp b/piper_cpp/test/timed_freq_state_test.cpp new file mode 100644 index 0000000..7d203ae --- /dev/null +++ b/piper_cpp/test/timed_freq_state_test.cpp @@ -0,0 +1,64 @@ +#include "piper_cpp/interface/timed_freq_state.h" + +#include + +#include +#include +#include +#include + +using namespace std::chrono_literals; +using piper_cpp::StateSnapshot; +using piper_cpp::TimedFreqState; + +namespace +{ + +struct Reading +{ + int n{0}; + std::string toString() const { return std::to_string(n); } +}; + +} // namespace + +TEST(TimedFreqState, HealthExpiresAfterTheTimeout) +{ + TimedFreqState state; + state.setHealthTimeout(50ms); + state.set(Reading{1}, 0.0); + EXPECT_TRUE(state.isHealthy()); + + std::this_thread::sleep_for(80ms); + EXPECT_FALSE(state.isHealthy()) << "no update for longer than the timeout"; + EXPECT_TRUE(state.isValid()) << "validity is about having received, not freshness"; +} + +TEST(TimedFreqState, NoTimeoutStaysHealthyOnceReceived) +{ + TimedFreqState state; + state.setHealthTimeout(1ms); + state.set(Reading{1}, 0.0); + std::this_thread::sleep_for(5ms); + ASSERT_FALSE(state.isHealthy()); + + state.setHealthTimeout(piper_cpp::kNoHealthTimeout); + EXPECT_TRUE(state.isHealthy()) << "a one-shot response does not go stale"; +} + +TEST(TimedFreqState, MultipartHealthNeedsEveryPartFresh) +{ + TimedFreqState state; + state.setNumCallers(3); + state.setHealthTimeout(50ms); + for (int part = 0; part < 3; ++part) + state.set(Reading{part}, 0.0, part); + ASSERT_TRUE(state.isHealthy()); + + std::this_thread::sleep_for(80ms); + state.set(Reading{0}, 0.0, 0); + EXPECT_FALSE(state.isHealthy()) << "one fresh part cannot hide two stale ones"; + state.set(Reading{1}, 0.0, 1); + state.set(Reading{2}, 0.0, 2); + EXPECT_TRUE(state.isHealthy()); +} From ebb412856dd5bec494e15001249453ef27e970df Mon Sep 17 00:00:00 2001 From: justagist Date: Thu, 15 Oct 2026 10:34:48 +0000 Subject: [PATCH 2/6] take state snapshots under one lock --- .../piper_cpp/interface/timed_freq_state.h | 19 +++++++++++--- piper_cpp/test/timed_freq_state_test.cpp | 26 +++++++++++++++++++ 2 files changed, 42 insertions(+), 3 deletions(-) diff --git a/piper_cpp/include/piper_cpp/interface/timed_freq_state.h b/piper_cpp/include/piper_cpp/interface/timed_freq_state.h index bb7ead6..839ea31 100644 --- a/piper_cpp/include/piper_cpp/interface/timed_freq_state.h +++ b/piper_cpp/include/piper_cpp/interface/timed_freq_state.h @@ -10,6 +10,8 @@ namespace piper_cpp { +template struct StateSnapshot; + /// Pass to `TimedFreqState::setHealthTimeout()` for one-shot responses, which stay healthy /// once received. inline constexpr std::chrono::steady_clock::duration kNoHealthTimeout = std::chrono::steady_clock::duration::max(); @@ -97,6 +99,9 @@ template class TimedFreqState return getHzLocked(); } + /// Value and metadata taken under one lock, so they describe the same update. + StateSnapshot snapshot() const; + private: bool isValidLocked() const { @@ -135,9 +140,10 @@ template struct StateSnapshot bool is_valid; bool is_healthy; - StateSnapshot(const TimedFreqState& state) - : value(state.getValue()), timestamp(state.getTimestamp()), hz(state.getHz()), is_valid(state.isValid()), - is_healthy(state.isHealthy()) + StateSnapshot(const TimedFreqState& state) : StateSnapshot(state.snapshot()) {} + + StateSnapshot(T value_, double timestamp_, double hz_, bool is_valid_, bool is_healthy_) + : value(std::move(value_)), timestamp(timestamp_), hz(hz_), is_valid(is_valid_), is_healthy(is_healthy_) { } @@ -150,4 +156,11 @@ template struct StateSnapshot } }; +template StateSnapshot TimedFreqState::snapshot() const +{ + std::lock_guard lk(mutex_); + const auto now = clock::now(); + return StateSnapshot(value_, timestamp_, getHzLocked(), isValidLocked(), isHealthyLocked(now)); +} + } // namespace piper_cpp diff --git a/piper_cpp/test/timed_freq_state_test.cpp b/piper_cpp/test/timed_freq_state_test.cpp index 7d203ae..01e8c81 100644 --- a/piper_cpp/test/timed_freq_state_test.cpp +++ b/piper_cpp/test/timed_freq_state_test.cpp @@ -62,3 +62,29 @@ TEST(TimedFreqState, MultipartHealthNeedsEveryPartFresh) state.set(Reading{2}, 0.0, 2); EXPECT_TRUE(state.isHealthy()); } + +TEST(TimedFreqState, SnapshotPairsValueWithItsOwnMetadata) +{ + // The writer keeps value and timestamp equal, so any snapshot where they differ mixed + // two updates. Separately locked getters did exactly that. + TimedFreqState state; + std::atomic stop{false}; + std::thread writer( + [&] + { + for (int n = 1; !stop.load(); ++n) + state.set(Reading{n}, static_cast(n)); + } + ); + + int mismatches = 0; + for (int i = 0; i < 20000; ++i) + { + const StateSnapshot snap(state); + if (snap.is_valid && static_cast(snap.value.n) != snap.timestamp) + ++mismatches; + } + stop.store(true); + writer.join(); + EXPECT_EQ(mismatches, 0); +} From df36dc1247da3c8ce6e240b446562e3df3aa67b1 Mon Sep 17 00:00:00 2001 From: justagist Date: Thu, 15 Oct 2026 10:34:48 +0000 Subject: [PATCH 3/6] clear cached state on disconnect --- .../piper_cpp/interface/piper_interface_v2.h | 19 +++++++++------ .../piper_cpp/interface/timed_freq_state.h | 10 ++++++++ .../src/interface/piper_interface_v2.cpp | 24 +++++++++++++++++++ piper_cpp/test/piper_interface_v2_test.cpp | 16 +++++++++++++ piper_cpp/test/timed_freq_state_test.cpp | 16 +++++++++++++ 5 files changed, 78 insertions(+), 7 deletions(-) diff --git a/piper_cpp/include/piper_cpp/interface/piper_interface_v2.h b/piper_cpp/include/piper_cpp/interface/piper_interface_v2.h index 240e8cc..ff4c3d2 100644 --- a/piper_cpp/include/piper_cpp/interface/piper_interface_v2.h +++ b/piper_cpp/include/piper_cpp/interface/piper_interface_v2.h @@ -74,9 +74,10 @@ class PiperInterfaceV2 /// @return true if the port opened (or was already open); false on initialisation failure. bool connectPort(bool can_init = false, bool piper_init = true, bool start_threads = true); - /// Stop the background read thread (if running) and close the CAN port. Wakes the read - /// thread out of its blocking receive and joins it, so this returns promptly even on a - /// silent bus. `timeout` is currently unused and kept for API compatibility. Idempotent. + /// Stop the background read thread (if running), close the CAN port and clear every cached + /// state, so a later session cannot read this one's feedback. Wakes the read thread out of + /// its blocking receive and joins it, so this returns promptly even on a silent bus. + /// `timeout` is currently unused and kept for API compatibility. Idempotent. void disconnectPort(std::chrono::milliseconds timeout = std::chrono::milliseconds{100}); /// Fire the one-shot startup queries: per-motor angle/speed limits, per-motor acceleration @@ -93,11 +94,13 @@ class PiperInterfaceV2 // - `value` : the payload itself // - `timestamp` : Unix-epoch timestamp (in seconds, double) of the last update // - `hz` : measured update rate based on inter-arrival time - // - `is_valid` : sticky -- true once at least one frame has been received - // - `is_healthy` : true if a frame has arrived in the last ~1 second + // - `is_valid` : true once every part of the message has been received this session + // - `is_healthy` : for periodic feedback, true while every part arrived within the last + // second; query responses and command echoes stay healthy once received // - // `is_valid` is only false during the brief window between `connectPort` returning and the - // first matching CAN frame arriving. Use `is_healthy` to detect a stalled or dropped feed. + // All five fields describe the same update. `is_valid` is false between `connectPort` + // returning and the first matching CAN frame arriving, and again after `disconnectPort`, + // which clears every cached state. Use `is_healthy` to detect a stalled or dropped feed. /// Latest arm status: control mode, motion status, error codes, teaching state, etc. /// Updates at ~200 Hz when the arm is connected. Drives `getArmHealth()`'s status field. @@ -673,6 +676,8 @@ class PiperInterfaceV2 ArmMsgMotionCtrl_2 ctrl_151_tx_; bool sendCommandedMotionCtrlLocked(); + void resetStates(); + void updateFeedbackFK(); void updateCtrlFK(); diff --git a/piper_cpp/include/piper_cpp/interface/timed_freq_state.h b/piper_cpp/include/piper_cpp/interface/timed_freq_state.h index 839ea31..2c76010 100644 --- a/piper_cpp/include/piper_cpp/interface/timed_freq_state.h +++ b/piper_cpp/include/piper_cpp/interface/timed_freq_state.h @@ -68,6 +68,16 @@ template class TimedFreqState initialized_.at(caller_id) = true; } + /// Forget the value and every part's arrival, as at construction. + void reset() + { + std::lock_guard lk(mutex_); + value_ = T{}; + timestamp_ = 0.0; + std::fill(initialized_.begin(), initialized_.end(), false); + std::fill(hz_.begin(), hz_.end(), 0.0); + } + bool isValid() const { std::lock_guard lk(mutex_); diff --git a/piper_cpp/src/interface/piper_interface_v2.cpp b/piper_cpp/src/interface/piper_interface_v2.cpp index 66fe49f..fcb9e84 100644 --- a/piper_cpp/src/interface/piper_interface_v2.cpp +++ b/piper_cpp/src/interface/piper_interface_v2.cpp @@ -131,6 +131,30 @@ void PiperInterfaceV2::disconnectPort(std::chrono::milliseconds timeout) } arm_can_->close(); connected_.store(false, std::memory_order_relaxed); + resetStates(); +} + +void PiperInterfaceV2::resetStates() +{ + // Called with the read thread stopped, so nothing races these. + arm_status_.reset(); + arm_end_pose_.reset(); + arm_joint_states_.reset(); + arm_gripper_msgs_.reset(); + arm_high_spd_fb_.reset(); + arm_low_spd_fb_.reset(); + arm_current_end_vel_acc_.reset(); + arm_crash_protection_rating_fb_.reset(); + arm_gripper_teaching_pendant_fb_.reset(); + arm_current_motor_angle_limit_max_spd_.reset(); + arm_current_motor_max_acc_limit_.reset(); + arm_all_motor_angle_limit_max_spd_.reset(); + arm_all_motor_max_acc_limit_.reset(); + arm_joint_ctrl_msgs_.reset(); + arm_gripper_ctrl_.reset(); + arm_motion_ctrl_code_151_.reset(); + arm_instruction_response_.reset(); + arm_health_.reset(); } void PiperInterfaceV2::parseCANFrame(const struct can_frame &frame, double timestamp) diff --git a/piper_cpp/test/piper_interface_v2_test.cpp b/piper_cpp/test/piper_interface_v2_test.cpp index a60aa66..b88922d 100644 --- a/piper_cpp/test/piper_interface_v2_test.cpp +++ b/piper_cpp/test/piper_interface_v2_test.cpp @@ -186,3 +186,19 @@ TEST(PiperInterfaceV2, AllMotorAccLimitsValidOnlyAfterEveryMotorReplies) EXPECT_EQ(all.motor[i].max_joint_acc, i + 1); } } + +TEST(PiperInterfaceV2, DisconnectClearsCachedState) +{ + auto bus = makeBus(); + ASSERT_TRUE(bus.piper->connectPort(/*can_init=*/false, /*piper_init=*/false, /*start_threads=*/true)); + const auto deadline = steady_clock::now() + 5s; + sendFrame(bus.peer, jointFeedbackFrame(0x2A5, 1, 2), deadline); + sendFrame(bus.peer, jointFeedbackFrame(0x2A6, 3, 4), deadline); + sendFrame(bus.peer, jointFeedbackFrame(0x2A7, 5, 6), deadline); + ASSERT_TRUE(waitFor([&] { return bus.piper->getArmJointStates().is_valid; })); + + bus.piper->disconnectPort(); + const auto js = bus.piper->getArmJointStates(); + EXPECT_FALSE(js.is_valid) << "feedback from a closed session must not look current"; + EXPECT_EQ(js.value.joint_6, 0); +} diff --git a/piper_cpp/test/timed_freq_state_test.cpp b/piper_cpp/test/timed_freq_state_test.cpp index 01e8c81..0ed6c10 100644 --- a/piper_cpp/test/timed_freq_state_test.cpp +++ b/piper_cpp/test/timed_freq_state_test.cpp @@ -88,3 +88,19 @@ TEST(TimedFreqState, SnapshotPairsValueWithItsOwnMetadata) writer.join(); EXPECT_EQ(mismatches, 0); } + +TEST(TimedFreqState, ResetForgetsValueAndArrivals) +{ + TimedFreqState state; + state.set(Reading{7}, 3.0); + ASSERT_TRUE(state.isValid()); + + state.reset(); + EXPECT_FALSE(state.isValid()); + EXPECT_FALSE(state.isHealthy()); + EXPECT_EQ(state.getValue().n, 0); + EXPECT_EQ(state.getTimestamp(), 0.0); + + state.set(Reading{8}, 4.0); + EXPECT_TRUE(state.isValid()); +} From 95425945dd08cdd85310310d6ce7539b5d06fb8b Mon Sep 17 00:00:00 2001 From: justagist Date: Thu, 15 Oct 2026 10:57:20 +0000 Subject: [PATCH 4/6] reset session state before reporting disconnected and clear firmware data --- piper_cpp/src/interface/piper_interface_v2.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/piper_cpp/src/interface/piper_interface_v2.cpp b/piper_cpp/src/interface/piper_interface_v2.cpp index fcb9e84..6b8a9b2 100644 --- a/piper_cpp/src/interface/piper_interface_v2.cpp +++ b/piper_cpp/src/interface/piper_interface_v2.cpp @@ -130,8 +130,8 @@ void PiperInterfaceV2::disconnectPort(std::chrono::milliseconds timeout) read_thread_.join(); } arm_can_->close(); - connected_.store(false, std::memory_order_relaxed); resetStates(); + connected_.store(false, std::memory_order_relaxed); } void PiperInterfaceV2::resetStates() @@ -155,6 +155,8 @@ void PiperInterfaceV2::resetStates() arm_motion_ctrl_code_151_.reset(); arm_instruction_response_.reset(); arm_health_.reset(); + std::lock_guard lk(firmware_data_mutex_); + firmware_data_.clear(); } void PiperInterfaceV2::parseCANFrame(const struct can_frame &frame, double timestamp) From 93d0f2ea42d04f3ad0921504048e41bf24b9ab62 Mon Sep 17 00:00:00 2001 From: justagist Date: Thu, 15 Oct 2026 10:57:20 +0000 Subject: [PATCH 5/6] keep the health timeout on end velocity feedback since it can be streamed --- piper_cpp/include/piper_cpp/interface/piper_interface_v2.h | 3 ++- piper_cpp/src/interface/piper_interface_v2.cpp | 1 - 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/piper_cpp/include/piper_cpp/interface/piper_interface_v2.h b/piper_cpp/include/piper_cpp/interface/piper_interface_v2.h index ff4c3d2..78a5a9b 100644 --- a/piper_cpp/include/piper_cpp/interface/piper_interface_v2.h +++ b/piper_cpp/include/piper_cpp/interface/piper_interface_v2.h @@ -133,7 +133,8 @@ class PiperInterfaceV2 /// End-effector velocity/acceleration limits the arm currently has set (0.001 m/s, /// 0.001 rad/s, etc.). Response message; populated only after a periodic-feedback request - /// is enabled or after `sendArmParamEnquiryAndConfig(EndVelAcc)` is called. + /// is enabled or after `sendArmParamEnquiryAndConfig(EndVelAcc)` is called. Because it can + /// be streamed, it keeps the 1 s health timeout; after a one-shot enquiry rely on `is_valid`. StateSnapshot getArmCurrentEndVelAcc() const { return StateSnapshot(arm_current_end_vel_acc_); diff --git a/piper_cpp/src/interface/piper_interface_v2.cpp b/piper_cpp/src/interface/piper_interface_v2.cpp index 6b8a9b2..6a9218a 100644 --- a/piper_cpp/src/interface/piper_interface_v2.cpp +++ b/piper_cpp/src/interface/piper_interface_v2.cpp @@ -42,7 +42,6 @@ PiperInterfaceV2::PiperInterfaceV2( // Query responses and command echoes arrive once, not periodically, so their age says // nothing about the link; only the periodic feedback streams keep the default timeout. - arm_current_end_vel_acc_.setHealthTimeout(kNoHealthTimeout); arm_crash_protection_rating_fb_.setHealthTimeout(kNoHealthTimeout); arm_gripper_teaching_pendant_fb_.setHealthTimeout(kNoHealthTimeout); arm_current_motor_angle_limit_max_spd_.setHealthTimeout(kNoHealthTimeout); From 1053f83053e9bc22fc3211aa7b3b0c266f7edaca Mon Sep 17 00:00:00 2001 From: justagist Date: Thu, 15 Oct 2026 10:57:20 +0000 Subject: [PATCH 6/6] harden the freshness tests against scheduling delays --- .../piper_cpp/interface/timed_freq_state.h | 1 + piper_cpp/test/timed_freq_state_test.cpp | 17 ++++++++++++----- 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/piper_cpp/include/piper_cpp/interface/timed_freq_state.h b/piper_cpp/include/piper_cpp/interface/timed_freq_state.h index 2c76010..a24d763 100644 --- a/piper_cpp/include/piper_cpp/interface/timed_freq_state.h +++ b/piper_cpp/include/piper_cpp/interface/timed_freq_state.h @@ -5,6 +5,7 @@ #include #include #include +#include #include namespace piper_cpp diff --git a/piper_cpp/test/timed_freq_state_test.cpp b/piper_cpp/test/timed_freq_state_test.cpp index 0ed6c10..cc6d61a 100644 --- a/piper_cpp/test/timed_freq_state_test.cpp +++ b/piper_cpp/test/timed_freq_state_test.cpp @@ -22,14 +22,16 @@ struct Reading } // namespace +// Timeouts are generous relative to the sleeps so a descheduled test thread cannot expire a +// state before the "still healthy" assertions run. TEST(TimedFreqState, HealthExpiresAfterTheTimeout) { TimedFreqState state; - state.setHealthTimeout(50ms); + state.setHealthTimeout(200ms); state.set(Reading{1}, 0.0); EXPECT_TRUE(state.isHealthy()); - std::this_thread::sleep_for(80ms); + std::this_thread::sleep_for(400ms); EXPECT_FALSE(state.isHealthy()) << "no update for longer than the timeout"; EXPECT_TRUE(state.isValid()) << "validity is about having received, not freshness"; } @@ -50,12 +52,12 @@ TEST(TimedFreqState, MultipartHealthNeedsEveryPartFresh) { TimedFreqState state; state.setNumCallers(3); - state.setHealthTimeout(50ms); + state.setHealthTimeout(200ms); for (int part = 0; part < 3; ++part) state.set(Reading{part}, 0.0, part); ASSERT_TRUE(state.isHealthy()); - std::this_thread::sleep_for(80ms); + std::this_thread::sleep_for(400ms); state.set(Reading{0}, 0.0, 0); EXPECT_FALSE(state.isHealthy()) << "one fresh part cannot hide two stale ones"; state.set(Reading{1}, 0.0, 1); @@ -77,15 +79,20 @@ TEST(TimedFreqState, SnapshotPairsValueWithItsOwnMetadata) } ); + int valid = 0; int mismatches = 0; for (int i = 0; i < 20000; ++i) { const StateSnapshot snap(state); - if (snap.is_valid && static_cast(snap.value.n) != snap.timestamp) + if (!snap.is_valid) + continue; + ++valid; + if (static_cast(snap.value.n) != snap.timestamp) ++mismatches; } stop.store(true); writer.join(); + EXPECT_GT(valid, 0) << "the check needs snapshots that saw the writer"; EXPECT_EQ(mismatches, 0); }