TimerController rework

[Riksu9000]

TimerController is now just a c++ wrapper for a FreeRTOS timer.

What directory and namespace should wrappers like this be in?

[github-actions[bot]]

Build size and comparison to main:	Section	Size	Difference
text	406424B	-32B
data	940B	0B
bss	53544B	-16B

[NeroBurner]

what is the reasoning of the new name xTimer? Hasn't TimerController have been only a wrapper around FreeRTOS timer as well? Please explain as I'm curious

[Riksu9000]

TimerController is currently only usable for the Timer App, and I see it as being a container for the timers for the timer app. In this PR I make the class reusable, so it is a true FreeRTOS timer wrapper. Now the name TimerController doesn't make as much sense to me. The FreeRTOS timer handle is often called xTimer in FreeRTOS sources, so I used that name, but maybe we can come up with a better name.

It could be called just Timer if it's put in FreeRTOS namespace. This is why I want clarification on the namespaces and file locations for wrappers like this.

[Riksu9000]

What about moving these files to src/wrappers/FreeRTOS/Timer.* and namespace Pinetime::FreeRTOS::Timer?

[JF002]

I think that making the Timer class reusable is a good idea. We can probably use it for some other current functionalities, and it might allow to implement multiple timers in the Timer app in the future.

Regarding the naming, I'm not sure I like the name wrappers and FreeRTOS. Those names represent implementation details, and we don't need to know about them when using the class. I know it's not clearly documented , but in my mind, the Timer class fits perfectly as a component/controller : it's a classes that implements a high level concept (a timer, something that calls a callback in a specific amount of time) and that abstracts how it's actually implemented. I'm open to suggestions for a better name for this layer of the project :) When we use such controller, we want the functionality of a timer, we don't care if this timer is implemented as a software timer, FreeRTOS timer or hardware timer. In that context, Timer could be named Pinetime::Controllers::Timer and placed in src/components/timer

Apart from this naming discussion, you'll probably want to implement the destructor of the Timer class to allow creating/deleting timers on the fly.

Other than that, I agree with this PR!

[Riksu9000]

I wasn't sure what the best way to do this currently is, because I was considering that we might eventually need to handle multiple timer implementations and then we'd need to move the FreeRTOS code somewhere else anyway. In one case the FreeRTOS code would already be separated, but an interface would need to be created in Controllers::Timer. In the other case, the FreeRTOS code would need to be moved to make Controllers::Timer an interface. Do you think the latter would be better?

[JF002]

Yes, we'll maybe want other implementations of timers in the future : FreeRTOS based or hardware based, for example. In this case, we can name the generic interface Controllers::Timer and concrete implementations HardwareTimer/HighResolutionTimer and SystemTimer/OsTimer/FreeRTOSTimer (this class would contain all the FreeRTOS specific code).

Since we are talking about generic interface, it would be a nice opportunity to think about the design :

• Use a virtual class Timer and then concrete implementations (FreeRTOSTimer, HardwareTimer) that derive from the Timer.
• Write a template class Timer that accepts multiple timer types as template parameters (Timer, for example).

[Riksu9000]

I was thinking of having a virtual class Timer. I don't know how the second idea with a template class would work.

[JF002]

Here's a quick overview of the idea with the template implementation (with a bonus concept):

#include <iostream>
class SoftwareTimer {
public:    
    void Start() {
        std::cout << "SoftwareTimer::Start()" << std::endl;
    }

    void Stop() {
        std::cout << "SoftwareTimer::Stop()" << std::endl;
    }

    int GetRemainingTime() const {
        std::cout << "SoftwareTimer::GetRemainingTime()" << std::endl;
        return 22;
    }
};

class HardwareTimer {
public:
    void Start() {
        std::cout << "HardwareTimer::Start()" << std::endl;
    }

    void Stop() {
        std::cout << "HardwareTimer::Stop()" << std::endl;
    }

    int GetRemainingTime() const {
        std::cout << "HardwareTimer::GetRemainingTime()" << std::endl;
        return 33;
    }
};

template <typename TimerImpl>
concept IsTimer = requires(TimerImpl timer) {
    { timer.Start() };
    { timer.Stop() };
    { timer.GetRemainingTime() } -> std::same_as<int>;
};

template<class T>
    requires IsTimer<T>
class Timer {
public:
    void Start() {
        timer.Start();
    }

    void Stop() {
        timer.Stop();
    }

    int GetRemainingTime() const {
        return timer.GetRemainingTime();
    }

    int Restart() {
        timer.Stop();
        timer.Start();
    }

private:
    T timer;
};

int main() {
    Timer<SoftwareTimer> softwareTimer;
    Timer<HardwareTimer> hardwareTimer;

    softwareTimer.Start();
    hardwareTimer.Restart();

    return 1;

}

https://godbolt.org/z/fhqoYoc99

So there are 2 implementations of a Timer : SoftwareTimer and HardwareTimer. The actual implementation of those classes would encapsulate FreeRTOS timers for the 1st one and the driver for the hardware timer from the nrf52 for the 2nd one.

Then the template class Timer uses an instance of the Timer concept (which is something your can Start, Stop and which returns the remaining time) and implements the higher level logic needed by the application. This logic can be common to all timers, no matter their actual type (hardware or software). I added the Restart() function as an example, but in the real project, this logic could be notifications to the tasks, logging,...