Reason for strange API design in app_timer?

[zrrr]

As i recently wanted to have a timer as a class member and a naive

class Foo {
    APP_TIMER_DEF(my_timer);
};

did not work I started to look into the source code. What I found where some very strange design decisions.

Let's start with a look at what the APP_TIMER_DEF macro actually does (app_timer.h line 107-109):

#define APP_TIMER_DEF(timer_id)                                  \
    static app_timer_t timer_id##_data = { {0} };                  \
    static const app_timer_id_t timer_id = &timer_id##_data

So this macro declares a variable of type app_timer_t (a struct with an uint32_t array as it's only member) with the name supplied to the macro appended with _data as its name and initialized with 0. It also declares a const variable of type app_timer_id_t and initializes it with a pointer to the variable it just declared. The first variable is essentially hidden from the user of the API.

As it turns out app_timer_id_t is just a typedef for a pointer to app_timer_t (app_timer.h line 100):

typedef app_timer_t * app_timer_id_t;

I would call that already bad practice but it gets even worse (or at least stranger).

Let us look next at the declaration of app_timer_create (app_timer.h line 209-212):

uint32_t app_timer_create(app_timer_id_t const *      p_timer_id,
                          app_timer_mode_t            mode,
                          app_timer_timeout_handler_t timeout_handler);

The first argument looks innocent enough at first but remember app_timer_id_t is just a typedef for a pointer to app_timer_t, so if we resolve the typedef p_time_id is of type app_timer_t* const*. Yes, that is a pointer to a const pointer to app_timer_t.

That seems strange, why would it make sense to have a pointer to a pointer? I assume that the typedef hiding the pointer led the nordic developer to making the mistake of making this double pointer without noticing it (which is one of the reasons why I consider such typedefs bad practice).

To check if there is a good reason for this pointer to pointer thing I also took a look at the definition of the function. The only use of the argument p_timer_id except for a error check to ensure it's !NULL is this (app_timer.c line 1009):

timer_node_t * p_node     = (timer_node_t *)*p_timer_id;

Here the outer pointer gets dereferenced and the inner pointer gets casted to a pointer of a different type. So as far as I can see there is no reason to not just directly use a pointer to app_timer_t instead.

I also took a look at the rest of the implementation of the timer system but still couldn't find a reason for these design decisions.

So my question is: What is the reason for that macro, the typedef and the strange pointer to a pointer?

Because if there is no good reason for this I would propose to simplify the API so that users declare a variable of type app_timer_t directly instead of using the APP_TIMER_DEF macro, that all functions use pointers to app_timer_t as arguments and the app_timer_id_t type gets removed.

For reference I'm using SDK version 10.0.0 but there is no difference in version 11.0.0.

[zrrr]

Nearly 15k views but not a single answer, comment or vote?

[Krzysztof Chruscinski]

Hi,

the reason for that strange macro was API backward compatibility. In legacy SDKs app_timer_create was setting this pointer since it was managing the pool of timers (then you had to predefine number of app timers in application and that was error prone) and in current SDK user is providing memory (allocated by APP_TIMER_DEF macro). Sorry for the confusion but we wanted to keep the API the same and yet change how RAM is allocated and managed.