Add IsDisabled() to shm and simulated timers
This is an important addition especially for checking and testing if the
timers are disabled at the right place. Earlier, we were just assuming
that timers would be disabled when we call disable() or let the timer
expire, but this is just a cherry on top to check the same.
Change-Id: I8b47c48609a085efb83eda99236d60f5c1e8f3aa
Signed-off-by: James Kuszmaul <james.kuszmaul@bluerivertech.com>
diff --git a/aos/events/event_loop.h b/aos/events/event_loop.h
index 68b5b35..471b625 100644
--- a/aos/events/event_loop.h
+++ b/aos/events/event_loop.h
@@ -484,6 +484,9 @@
// Stop future calls to callback().
virtual void Disable() = 0;
+ // Check if the timer is disabled
+ virtual bool IsDisabled() = 0;
+
// Sets and gets the name of the timer. Set this if you want a descriptive
// name in the timing report.
void set_name(std::string_view name) { name_ = std::string(name); }
diff --git a/aos/events/event_loop_param_test.cc b/aos/events/event_loop_param_test.cc
index 990cbe5..0c8e41e 100644
--- a/aos/events/event_loop_param_test.cc
+++ b/aos/events/event_loop_param_test.cc
@@ -524,6 +524,104 @@
EXPECT_THAT(values, ::testing::ElementsAreArray({201}));
}
+// Tests that timer handler is enabled after setup (even if it is in the past)
+// and is disabled after running
+TEST_P(AbstractEventLoopTest, CheckTimerDisabled) {
+ auto loop = MakePrimary("primary");
+
+ auto timer = loop->AddTimer([this]() {
+ LOG(INFO) << "timer called";
+ Exit();
+ });
+
+ loop->OnRun([&loop, timer]() {
+ EXPECT_TRUE(timer->IsDisabled());
+ timer->Setup(loop->monotonic_now() + chrono::milliseconds(100));
+ EXPECT_FALSE(timer->IsDisabled());
+ });
+
+ Run();
+ EXPECT_TRUE(timer->IsDisabled());
+}
+
+// Tests that timer handler is enabled after setup (even if it is in the past)
+// and is disabled after running
+TEST_P(AbstractEventLoopTest, CheckTimerRunInPastDisabled) {
+ auto loop = MakePrimary("primary");
+
+ auto timer2 = loop->AddTimer([this]() {
+ LOG(INFO) << "timer called";
+ Exit();
+ });
+
+ auto timer = loop->AddTimer([&loop, timer2]() {
+ timer2->Setup(loop->monotonic_now() - chrono::nanoseconds(1));
+ });
+
+ loop->OnRun([&loop, timer]() {
+ timer->Setup(loop->monotonic_now() + chrono::seconds(1));
+ EXPECT_FALSE(timer->IsDisabled());
+ });
+
+ Run();
+ EXPECT_TRUE(timer2->IsDisabled());
+}
+
+// Tests that timer handler is not disabled even after calling Exit on the event
+// loop within the timer
+TEST_P(AbstractEventLoopTest, CheckTimerRepeatOnCountDisabled) {
+ auto loop = MakePrimary("primary");
+ int counter = 0;
+
+ auto timer = loop->AddTimer([&counter, this]() {
+ LOG(INFO) << "timer called";
+ counter++;
+ if (counter >= 5) {
+ Exit();
+ }
+ });
+
+ loop->OnRun([&loop, timer]() {
+ timer->Setup(loop->monotonic_now() + chrono::seconds(1),
+ chrono::seconds(1));
+ EXPECT_FALSE(timer->IsDisabled());
+ });
+ Run();
+
+ // Sanity check
+ EXPECT_EQ(counter, 5);
+
+ // if you run the loop again, the timer will start running again
+ EXPECT_FALSE(timer->IsDisabled());
+
+ counter = 0;
+ Run();
+ timer->Disable();
+
+ EXPECT_TRUE(timer->IsDisabled());
+}
+
+// Tests that timer handler is not disabled even after calling Exit on the event
+// loop using an external timer
+TEST_P(AbstractEventLoopTest, CheckTimerRepeatTillEndTimerDisabled) {
+ auto loop = MakePrimary("primary");
+
+ auto timer = loop->AddTimer([]() { LOG(INFO) << "timer called"; });
+
+ loop->OnRun([&loop, timer]() {
+ timer->Setup(loop->monotonic_now() + chrono::seconds(1),
+ chrono::seconds(1));
+ EXPECT_FALSE(timer->IsDisabled());
+ });
+
+ EndEventLoop(loop.get(), chrono::seconds(5));
+ Run();
+ EXPECT_FALSE(timer->IsDisabled());
+
+ timer->Disable();
+ EXPECT_TRUE(timer->IsDisabled());
+}
+
// Tests that Fetch and FetchNext interleave as expected.
TEST_P(AbstractEventLoopTest, FetchAndFetchNextTogether) {
auto loop1 = Make();
diff --git a/aos/events/shm_event_loop.cc b/aos/events/shm_event_loop.cc
index 5be5077..56d2357 100644
--- a/aos/events/shm_event_loop.cc
+++ b/aos/events/shm_event_loop.cc
@@ -599,6 +599,7 @@
if (repeat_offset_ == std::chrono::seconds(0)) {
timerfd_.Disable();
+ disabled_ = true;
} else {
// Compute how many cycles have elapsed and schedule the next iteration
// for the next iteration in the future.
@@ -612,6 +613,7 @@
event_.set_event_time(base_);
shm_event_loop_->AddEvent(&event_);
timerfd_.SetTime(base_, std::chrono::seconds(0));
+ disabled_ = false;
}
}
@@ -627,6 +629,7 @@
repeat_offset_ = repeat_offset;
event_.set_event_time(base_);
shm_event_loop_->AddEvent(&event_);
+ disabled_ = false;
}
void Disable() override {
@@ -636,6 +639,8 @@
disabled_ = true;
}
+ bool IsDisabled() override { return disabled_; }
+
private:
ShmEventLoop *shm_event_loop_;
EventHandler<ShmTimerHandler> event_;
@@ -647,7 +652,7 @@
// Used to track if Disable() was called during the callback, so we know not
// to reschedule.
- bool disabled_ = false;
+ bool disabled_ = true;
};
// Adapter class to the timerfd and PhasedLoop.
diff --git a/aos/events/simulated_event_loop.cc b/aos/events/simulated_event_loop.cc
index 944a22b..22b9603 100644
--- a/aos/events/simulated_event_loop.cc
+++ b/aos/events/simulated_event_loop.cc
@@ -521,6 +521,8 @@
void Disable() override;
+ bool IsDisabled() override;
+
private:
SimulatedEventLoop *simulated_event_loop_;
EventHandler<SimulatedTimerHandler> event_;
@@ -529,6 +531,7 @@
monotonic_clock::time_point base_;
monotonic_clock::duration repeat_offset_;
+ bool disabled_ = true;
};
class SimulatedPhasedLoopHandler : public PhasedLoopHandler,
@@ -1201,6 +1204,7 @@
token_ = scheduler_->Schedule(std::max(base, monotonic_now), this);
event_.set_event_time(base_);
simulated_event_loop_->AddEvent(&event_);
+ disabled_ = false;
}
void SimulatedTimerHandler::Handle() noexcept {
@@ -1232,8 +1236,10 @@
token_ = scheduler_->Schedule(base_, this);
event_.set_event_time(base_);
simulated_event_loop_->AddEvent(&event_);
+ disabled_ = false;
+ } else {
+ disabled_ = true;
}
-
{
ScopedMarkRealtimeRestorer rt(
simulated_event_loop_->runtime_realtime_priority() > 0);
@@ -1251,8 +1257,11 @@
}
token_ = scheduler_->InvalidToken();
}
+ disabled_ = true;
}
+bool SimulatedTimerHandler::IsDisabled() { return disabled_; }
+
SimulatedPhasedLoopHandler::SimulatedPhasedLoopHandler(
EventScheduler *scheduler, SimulatedEventLoop *simulated_event_loop,
::std::function<void(int)> fn, const monotonic_clock::duration interval,