fixed some potential bug related to timer getting stopped, but signaling that it expired

This commit is contained in:
Francisco Paisana 2019-10-21 12:03:30 +01:00 committed by Francisco Paisana
parent 551192705e
commit 5953d5ef96
2 changed files with 206 additions and 135 deletions

View File

@ -44,8 +44,8 @@ namespace srslte {
class timer_callback class timer_callback
{ {
public: public:
virtual void timer_expired(uint32_t timer_id) = 0; virtual void timer_expired(uint32_t timer_id) = 0;
}; };
class timers class timers
@ -54,28 +54,34 @@ public:
class timer class timer
{ {
public: public:
timer(uint32_t id_=0) {id = id_; counter = 0; timeout = 0; running = false; callback = NULL; } timer(uint32_t id_ = 0)
void set(timer_callback *callback_, uint32_t timeout_) { {
id = id_;
counter = 0;
timeout = 0;
running = false;
callback = nullptr;
}
void set(timer_callback* callback_, uint32_t timeout_)
{
callback = callback_; callback = callback_;
timeout = timeout_; timeout = timeout_;
reset(); reset();
} }
bool is_running() {
return (counter < timeout) && running; bool is_running() { return (counter < timeout) && running; }
}
bool is_expired() { bool is_expired() { return (timeout > 0) && (counter >= timeout); }
return (timeout > 0) && (counter >= timeout);
} uint32_t get_timeout() { return timeout; }
uint32_t get_timeout() {
return timeout; void reset() { counter = 0; }
}
void reset() { uint32_t value() { return counter; }
counter = 0;
} void step()
uint32_t value() { {
return counter;
}
void step() {
if (running) { if (running) {
counter++; counter++;
if (is_expired()) { if (is_expired()) {
@ -86,18 +92,18 @@ public:
} }
} }
} }
void stop() {
running = false; void stop() { running = false; }
}
void run() { void run() { running = true; }
running = true;
}
uint32_t id; uint32_t id;
private: private:
timer_callback *callback; timer_callback* callback;
uint32_t timeout; uint32_t timeout;
uint32_t counter; uint32_t counter;
bool running; bool running;
}; };
timers(uint32_t nof_timers_) : timer_list(nof_timers_), used_timers(nof_timers_) timers(uint32_t nof_timers_) : timer_list(nof_timers_), used_timers(nof_timers_)
@ -107,31 +113,40 @@ public:
nof_used_timers = 0; nof_used_timers = 0;
for (uint32_t i = 0; i < nof_timers; i++) { for (uint32_t i = 0; i < nof_timers; i++) {
timer_list[i].id = i; timer_list[i].id = i;
used_timers[i] = false; used_timers[i] = false;
} }
} }
void step_all() { void step_all()
for (uint32_t i=0;i<nof_timers;i++) { {
for (uint32_t i = 0; i < nof_timers; i++) {
get(i)->step(); get(i)->step();
} }
} }
void stop_all() {
for (uint32_t i=0;i<nof_timers;i++) { void stop_all()
{
for (uint32_t i = 0; i < nof_timers; i++) {
get(i)->stop(); get(i)->stop();
} }
} }
void run_all() {
for (uint32_t i=0;i<nof_timers;i++) { void run_all()
{
for (uint32_t i = 0; i < nof_timers; i++) {
get(i)->run(); get(i)->run();
} }
} }
void reset_all() {
for (uint32_t i=0;i<nof_timers;i++) { void reset_all()
{
for (uint32_t i = 0; i < nof_timers; i++) {
get(i)->reset(); get(i)->reset();
} }
} }
timer *get(uint32_t i) {
timer* get(uint32_t i)
{
if (i < nof_timers) { if (i < nof_timers) {
return &timer_list[i]; return &timer_list[i];
} else { } else {
@ -139,7 +154,9 @@ public:
return NULL; return NULL;
} }
} }
void release_id(uint32_t i) {
void release_id(uint32_t i)
{
if (nof_used_timers > 0 && i < nof_timers) { if (nof_used_timers > 0 && i < nof_timers) {
used_timers[i] = false; used_timers[i] = false;
nof_used_timers--; nof_used_timers--;
@ -147,12 +164,14 @@ public:
ERROR("Error releasing timer id=%d: nof_used_timers=%d, nof_timers=%d\n", i, nof_used_timers, nof_timers); ERROR("Error releasing timer id=%d: nof_used_timers=%d, nof_timers=%d\n", i, nof_used_timers, nof_timers);
} }
} }
uint32_t get_unique_id() {
uint32_t get_unique_id()
{
if (nof_used_timers >= nof_timers) { if (nof_used_timers >= nof_timers) {
ERROR("Error getting unique timer id: no more timers available\n"); ERROR("Error getting unique timer id: no more timers available\n");
return 0; return 0;
} else { } else {
for (uint32_t i=0;i<nof_timers;i++) { for (uint32_t i = 0; i < nof_timers; i++) {
if (!used_timers[i]) { if (!used_timers[i]) {
used_timers[i] = true; used_timers[i] = true;
nof_used_timers++; nof_used_timers++;
@ -165,32 +184,39 @@ public:
return 0; return 0;
} }
} }
private: private:
uint32_t next_timer; uint32_t next_timer;
uint32_t nof_used_timers; uint32_t nof_used_timers;
uint32_t nof_timers; uint32_t nof_timers;
std::vector<timer> timer_list; std::vector<timer> timer_list;
std::vector<bool> used_timers; std::vector<bool> used_timers;
}; };
class timers2 class timer_handler
{ {
constexpr static uint32_t MAX_TIMER_DURATION = std::numeric_limits<uint32_t>::max() / 4;
constexpr static uint32_t MAX_TIMER_VALUE = std::numeric_limits<uint32_t>::max() / 2;
struct timer_impl { struct timer_impl {
timers2* parent; timer_handler* parent;
uint32_t duration = 0, timeout = 0; uint32_t duration = 0, timeout = 0;
bool running = false; bool running = false;
bool active = false; bool active = false;
std::function<void(uint32_t)> callback; std::function<void(uint32_t)> callback;
explicit timer_impl(timers2* parent_) : parent(parent_) {} explicit timer_impl(timer_handler* parent_) : parent(parent_) {}
uint32_t id() const { return std::distance((const timers2::timer_impl*)&parent->timer_list[0], this); }
bool is_running() const { return active and running and timeout > 0; } uint32_t id() const { return std::distance((const timer_handler::timer_impl*)&parent->timer_list[0], this); }
bool is_expired() const { return active and not running and timeout > 0; }
bool is_running() const { return active and running and timeout > 0; }
bool is_expired() const { return active and not running and timeout > 0; }
bool set(uint32_t duration_) bool set(uint32_t duration_)
{ {
if (duration_ > std::numeric_limits<uint32_t>::max() / 4) { if (duration_ > MAX_TIMER_DURATION) {
ERROR("Error: timer durations above %u are not supported\n", std::numeric_limits<uint32_t>::max() / 4); ERROR("Error: timer durations above %u are not supported\n", MAX_TIMER_DURATION);
return false; return false;
} }
if (not active) { if (not active) {
@ -199,6 +225,7 @@ class timers2
} }
duration = duration_; duration = duration_;
running = false; // invalidates any on-going run running = false; // invalidates any on-going run
timeout = 0;
return true; return true;
} }
@ -218,10 +245,16 @@ class timers2
return; return;
} }
timeout = parent->cur_time + duration; timeout = parent->cur_time + duration;
parent->running_timers.emplace(parent, id(), timeout); parent->running_timers.emplace(id(), timeout);
running = true; running = true;
} }
void stop()
{
running = false; // invalidates trigger
timeout = 0; // makes is_expired() == false && is_running() == false
}
void clear() void clear()
{ {
timeout = 0; timeout = 0;
@ -229,13 +262,15 @@ class timers2
running = false; running = false;
active = false; active = false;
callback = std::function<void(uint32_t)>(); callback = std::function<void(uint32_t)>();
// leave run_id unchanged; // leave run_id unchanged. Since the timeout was changed, we shall not get spurious triggering
} }
void trigger() void trigger()
{ {
if (is_running()) { if (is_running()) {
callback(id()); if (callback) {
callback(id());
}
running = false; running = false;
} }
} }
@ -245,12 +280,15 @@ public:
class unique_timer class unique_timer
{ {
public: public:
explicit unique_timer(timers2* parent_, uint32_t timer_id_) : parent(parent_), timer_id(timer_id_) {} explicit unique_timer(timer_handler* parent_, uint32_t timer_id_) : parent(parent_), timer_id(timer_id_) {}
unique_timer(const unique_timer&) = delete; unique_timer(const unique_timer&) = delete;
unique_timer(unique_timer&& other) noexcept : parent(other.parent), timer_id(other.timer_id) unique_timer(unique_timer&& other) noexcept : parent(other.parent), timer_id(other.timer_id)
{ {
other.parent = nullptr; other.parent = nullptr;
} }
~unique_timer() ~unique_timer()
{ {
if (parent) { if (parent) {
@ -258,8 +296,10 @@ public:
impl()->clear(); impl()->clear();
} }
} }
unique_timer& operator=(const unique_timer&) = delete; unique_timer& operator=(const unique_timer&) = delete;
unique_timer& operator =(unique_timer&& other) noexcept
unique_timer& operator=(unique_timer&& other) noexcept
{ {
if (this != &other) { if (this != &other) {
timer_id = other.timer_id; timer_id = other.timer_id;
@ -269,23 +309,30 @@ public:
return *this; return *this;
} }
void set(uint32_t duration_, const std::function<void(int)>& callback_) { impl()->set(duration_, callback_); } void set(uint32_t duration_, const std::function<void(int)>& callback_) { impl()->set(duration_, callback_); }
void set(uint32_t duration_) { impl()->set(duration_); }
bool is_running() const { return impl()->is_running(); } void set(uint32_t duration_) { impl()->set(duration_); }
bool is_expired() const { return impl()->is_expired(); }
void run() { impl()->run(); } bool is_running() const { return impl()->is_running(); }
void stop() { impl()->running = false; }
bool is_expired() const { return impl()->is_expired(); }
void run() { impl()->run(); }
void stop() { impl()->stop(); }
uint32_t id() const { return timer_id; } uint32_t id() const { return timer_id; }
private: private:
timer_impl* impl() { return &parent->timer_list[timer_id]; } timer_impl* impl() { return &parent->timer_list[timer_id]; }
const timer_impl* impl() const { return &parent->timer_list[timer_id]; } const timer_impl* impl() const { return &parent->timer_list[timer_id]; }
timers2* parent; timer_handler* parent;
uint32_t timer_id; uint32_t timer_id;
}; };
explicit timers2(uint32_t capacity = 64) explicit timer_handler(uint32_t capacity = 64)
{ {
timer_list.reserve(capacity); timer_list.reserve(capacity);
// reserve a priority queue using a vector // reserve a priority queue using a vector
@ -315,8 +362,8 @@ public:
while (not running_timers.empty()) { while (not running_timers.empty()) {
running_timers.pop(); running_timers.pop();
} }
for (uint32_t i = 0; i < timer_list.size(); ++i) { for (auto& i : timer_list) {
timer_list[i].running = false; i.running = false;
} }
} }
@ -336,6 +383,7 @@ public:
} }
uint32_t get_cur_time() const { return cur_time; } uint32_t get_cur_time() const { return cur_time; }
uint32_t nof_timers() const uint32_t nof_timers() const
{ {
return std::count_if(timer_list.begin(), timer_list.end(), [](const timer_impl& t) { return t.active; }); return std::count_if(timer_list.begin(), timer_list.end(), [](const timer_impl& t) { return t.active; });
@ -343,23 +391,18 @@ public:
private: private:
struct timer_run { struct timer_run {
timers2* parent;
uint32_t timer_id; uint32_t timer_id;
uint32_t timeout; uint32_t timeout;
timer_run(timers2* parent_, uint32_t timer_id_, uint32_t timeout_) :
parent(parent_), timer_run(uint32_t timer_id_, uint32_t timeout_) : timer_id(timer_id_), timeout(timeout_) {}
timer_id(timer_id_),
timeout(timeout_)
{
}
bool operator<(const timer_run& other) const bool operator<(const timer_run& other) const
{ {
// returns true, if greater // returns true, if other.timeout is lower than timeout, accounting for wrap around
uint32_t lim = std::numeric_limits<uint32_t>::max();
if (timeout > other.timeout) { if (timeout > other.timeout) {
return (timeout - other.timeout) < lim / 2; return (timeout - other.timeout) < MAX_TIMER_VALUE / 2;
} }
return (other.timeout - timeout) > lim / 2; return (other.timeout - timeout) > MAX_TIMER_VALUE / 2;
} }
}; };

View File

@ -23,78 +23,78 @@
#include <iostream> #include <iostream>
#define TESTASSERT(cond) \ #define TESTASSERT(cond) \
{ \ do { \
if (!(cond)) { \ if (!(cond)) { \
std::cout << "[" << __FUNCTION__ << "][Line " << __LINE__ << "]: FAIL at " << (#cond) << std::endl; \ std::cout << "[" << __FUNCTION__ << "][Line " << __LINE__ << "]: FAIL at " << (#cond) << std::endl; \
return -1; \ return -1; \
} \ } \
} } while (0)
using namespace srslte; using namespace srslte;
int timers2_test() int timers2_test()
{ {
timers2 timers; timer_handler timers;
uint32_t dur = 5; uint32_t dur = 5;
{ {
timers2::unique_timer t = timers.get_unique_timer(); timer_handler::unique_timer t = timers.get_unique_timer();
TESTASSERT(not t.is_running() and not t.is_expired()) TESTASSERT(not t.is_running() and not t.is_expired());
TESTASSERT(t.id() == 0) TESTASSERT(t.id() == 0);
timers2::unique_timer t2 = timers.get_unique_timer(); timer_handler::unique_timer t2 = timers.get_unique_timer();
TESTASSERT(not t2.is_running() and not t2.is_expired()) TESTASSERT(not t2.is_running() and not t2.is_expired());
TESTASSERT(t2.id() == 1) TESTASSERT(t2.id() == 1);
TESTASSERT(timers.nof_timers() == 2) TESTASSERT(timers.nof_timers() == 2);
// TEST: Run multiple times with the same duration // TEST: Run multiple times with the same duration
bool callback_called = false; bool callback_called = false;
t.set(dur, [&callback_called](int) { callback_called = true; }); t.set(dur, [&callback_called](int) { callback_called = true; });
TESTASSERT(timers.get_cur_time() == 0) TESTASSERT(timers.get_cur_time() == 0);
for (uint32_t runs = 0; runs < 3; ++runs) { for (uint32_t runs = 0; runs < 3; ++runs) {
callback_called = false; callback_called = false;
TESTASSERT(not t.is_running()) TESTASSERT(not t.is_running());
t.run(); t.run();
TESTASSERT(t.is_running() and not t.is_expired()) TESTASSERT(t.is_running() and not t.is_expired());
for (uint32_t i = 0; i < dur; ++i) { for (uint32_t i = 0; i < dur; ++i) {
timers.step_all(); timers.step_all();
TESTASSERT(t.is_running() and not t.is_expired()) TESTASSERT(t.is_running() and not t.is_expired());
} }
timers.step_all(); timers.step_all();
TESTASSERT(not t.is_running() and t.is_expired()) TESTASSERT(not t.is_running() and t.is_expired());
TESTASSERT(callback_called) TESTASSERT(callback_called);
} }
TESTASSERT(timers.get_cur_time() == 3 * (1 + dur)) TESTASSERT(timers.get_cur_time() == 3 * (1 + dur));
// TEST: interrupt a timer. check if callback was called // TEST: interrupt a timer. check if callback was called
callback_called = false; callback_called = false;
t.run(); t.run();
timers.step_all(); timers.step_all();
TESTASSERT(t.is_running()) TESTASSERT(t.is_running());
t.stop(); t.stop();
TESTASSERT(not t.is_running()) TESTASSERT(not t.is_running());
for (uint32_t i = 0; i < dur; ++i) { for (uint32_t i = 0; i < dur; ++i) {
timers.step_all(); timers.step_all();
TESTASSERT(not t.is_running()) TESTASSERT(not t.is_running());
} }
TESTASSERT(not callback_called) TESTASSERT(not callback_called);
// TEST: call timer::run() when it is already running. Check if duration gets extended. // TEST: call timer::run() when it is already running. Check if duration gets extended.
callback_called = false; callback_called = false;
t.run(); t.run();
timers.step_all(); timers.step_all();
TESTASSERT(t.is_running()) TESTASSERT(t.is_running());
t.run(); // re-run t.run(); // re-run
for (uint32_t i = 0; i < dur; ++i) { for (uint32_t i = 0; i < dur; ++i) {
timers.step_all(); timers.step_all();
TESTASSERT(t.is_running()) TESTASSERT(t.is_running());
} }
timers.step_all(); timers.step_all();
TESTASSERT(not t.is_running()) TESTASSERT(not t.is_running());
TESTASSERT(callback_called) TESTASSERT(callback_called);
// TEST: ordering of timers is respected // TEST: ordering of timers is respected
timers2::unique_timer t3 = timers.get_unique_timer(); timer_handler::unique_timer t3 = timers.get_unique_timer();
TESTASSERT(t3.id() == 2) TESTASSERT(t3.id() == 2);
int first_id = -1, last_id = -1; int first_id = -1, last_id = -1;
auto callback = [&first_id, &last_id](int id) { auto callback = [&first_id, &last_id](int id) {
if (first_id < 0) { if (first_id < 0) {
@ -111,27 +111,55 @@ int timers2_test()
t3.run(); t3.run();
for (uint32_t i = 0; i < 6; ++i) { for (uint32_t i = 0; i < 6; ++i) {
timers.step_all(); timers.step_all();
TESTASSERT(i >= 4 or t.is_running()) TESTASSERT(i >= 4 or t.is_running());
TESTASSERT(i >= 2 or t2.is_running()) TESTASSERT(i >= 2 or t2.is_running());
TESTASSERT(t3.is_running()) TESTASSERT(t3.is_running());
} }
timers.step_all(); timers.step_all();
TESTASSERT(t.is_expired() and t2.is_expired() and t3.is_expired()) TESTASSERT(t.is_expired() and t2.is_expired() and t3.is_expired());
TESTASSERT(first_id == 1) TESTASSERT(first_id == 1);
printf("Last timer id=%d\n", last_id); printf("Last timer id=%d\n", last_id);
TESTASSERT(last_id == 2) TESTASSERT(last_id == 2);
} }
// TEST: timer dtor is called and removes "timer" from "timers" // TEST: timer dtor is called and removes "timer" from "timers"
TESTASSERT(timers.nof_timers() == 0) TESTASSERT(timers.nof_timers() == 0);
printf("Success\n"); return SRSLTE_SUCCESS;
}
int timers2_test2()
{
/**
* Description:
* - check if we call stop(), the timer does not get into expired state
*/
timer_handler timers;
uint32_t duration = 2;
auto utimer = timers.get_unique_timer();
auto utimer2 = timers.get_unique_timer();
utimer.set(duration);
utimer.run();
utimer2.set(duration);
utimer2.run();
TESTASSERT(utimer.is_running() and not utimer.is_expired());
utimer.stop();
TESTASSERT(not utimer.is_running() and not utimer.is_expired());
for (uint32_t i = 0; i < 5; ++i) {
timers.step_all();
}
TESTASSERT(not utimer.is_expired());
TESTASSERT(utimer2.is_expired());
return SRSLTE_SUCCESS; return SRSLTE_SUCCESS;
} }
int main() int main()
{ {
TESTASSERT(timers2_test() == SRSLTE_SUCCESS) TESTASSERT(timers2_test() == SRSLTE_SUCCESS);
TESTASSERT(timers2_test2() == SRSLTE_SUCCESS);
printf("Success\n");
return 0; return 0;
} }