This post is older than 2 years and might not be relevant anymore
More Info: Consider searching for newer posts

Reason for strange API design in app_timer?

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.

Related