TimerMgr - support for periodic timers

Hi!

At the moment, Timer class is a simple deadline timer. Using it we can implement something like periodic timer, but in my opinion it is not a performance-optimal solution:

void SomeClass::periodicTask()
{
   /* do some work */
   timer.asyncWait(std::chrono::seconds(1), 
      [this](const auto& errorStatus)
      {
         if(errorStatus) // optional - stop the interval
         {
            /* LOG_ERROR(...) */
            return;
         }
         periodicTask(); 
      });
}

void SomeClass::start()
{
   timer.asyncWait(std::chrono::seconds(1), 
      [this](const auto& errorStatus)
      {
         if(errorStatus) // optional - stop the interval
         {
            /* LOG_ERROR(...) */
            return;
         }
         periodicTask(); 
      });
}

Correct me, if I'm wrong, but I think this code is re-assigning callback to the StaticFunction object every second. It is arleady not needed, because the callback is always the same. Additionally, body of the callback's lambda will be repeated. It would be nice to have solution like this:

void SomeClass::periodicTask()
{
   /* do some work */
}

void SomeClass::start()
{
   interval.start(std::chrono::seconds(1),
      [this](const auto& errorStatus)
      {
         if(errorStatus) // on error, interval is stopped, but it can be restarted
         {
            /* LOG_ERROR(...) */
            interval.restart();
            return;
         }
         periodicTask();
      });
}

Periodic timer would be very useful to implement cyclical tasks using e.g. SysTick. It can be of course implemented using current Timer class, but the problem of reasigning the same callbacks every time is still not solved. On the other hand, I'm not sure, if current TimerMgr driver is suitable to implement this feature... Maybe another Timer driver has to be implemented? What do you think about this?

Hi @akowalew (what's you first name by the way? How should I address you?), Most of the work for this library was inspired by Boost.Asio and I kind of replicated functionality available there. If I'm not mistaken timers in Boost.Asio are not periodic, every wait they report status and it's up to the application to restart the wait. I worked a lot with Boost.Asio over the years and haven't seen any performance problem in these timers. I also want to remind you that "premature optimization is a root of all evil". Do you really think you will gain something tangible in terms of performance by introducing support for periodic timers?

I've taken a look at TimerMgr again and indeed, at this moment it doesn't care about periodic timers, it just uses single Timer device for multiple independent waits. In order to support such periodic waits, every TimerMgr::TimerInfo instance needs to be extended with extra information of the period as well as boolean flag saying whether the wait is periodic. These fields will be unused (and will just occupy space) in non-periodic waits. When the wait is over and handler is about to be invoked, just check the flag and re-insert the wait automatically. Note, that relocation of the TimerInfo object inside the inner queue may still be required. One away or another you may the re-inserting price. The question is whether it is necessary to do it in interrupt mode immediately or do it in event loop.

As I mentioned to you already in #14, I'm spending most of my time on CommsChampion and related projects, and don't have much time to spend on embxx. In fact I haven't really touched it for 5 years or so. Feel free to implement this extra functionality yourself on your own copy of the library. If you have something working, you are welcome to send me pull request.

My name's is Adam. ;)

I know, that in Boost.Asio there are only deadline timers. I also know, that it is not good to optimize too early. As I said, there is a problem of re-calling asyncWait in order to continue interval: it can be simply missed and an interval will not happen. Additionally, invocation of asyncWait will be repeated in the code. I think adding the feature like periodic timer will mostly simplify design of interval tasks. Performance may be also increased or not - it depends on details.

I agree that adding this feature into the TimerMgr is not the best idea. Maybe adding additional classes, like IntervalMgr and Interval will solve this problem? They will be specialized to work only with periodic tasks. Or maybe only Interval class should be written? It can use then the Timer under the hood to implement periods, by re-calling asyncWait every time. In both cases TimerMgr will be not modified deeply. In a free time I will try to implement this functionality on my own. ;)

I understand, that at the moment you are mostly working on CommsChampion. I'm not expecting, that you will implement this functionality in a day or two - I'm just proposing some enhancements to the embxx. ;) By the way, thank you for both projects, good luck!

arobenko / embxx

TimerMgr - support for periodic timers #18