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

What is the reason of making nRF APIs that complicated?

As an example, let's see this function from app_hrm (heart rate monitor):

static void conn_params_init(void)
{
    uint32_t               err_code;
    ble_conn_params_init_t cp_init;

    memset(&cp_init, 0, sizeof(cp_init));

    cp_init.p_conn_params                  = NULL;
    cp_init.first_conn_params_update_delay = FIRST_CONN_PARAMS_UPDATE_DELAY;
    cp_init.next_conn_params_update_delay  = NEXT_CONN_PARAMS_UPDATE_DELAY;
    cp_init.max_conn_params_update_count   = MAX_CONN_PARAMS_UPDATE_COUNT;
    cp_init.start_on_notify_cccd_handle    = m_hrs.hrm_handles.cccd_handle;
    cp_init.disconnect_on_fail             = true;
    cp_init.evt_handler                    = NULL;
    cp_init.error_handler                  = conn_params_error_handler;

    err_code = ble_conn_params_init(&cp_init);
    APP_ERROR_CHECK(err_code);
}

Quickly looking, it seems that there's a lot of code doing lots of things. However, after reading out the lines it turns out that the information is mostly redundant (like "cp_init.first_conn_params_update_delay = FIRST_CONN_PARAMS_UPDATE_DELAY"), and the function just bypasses parameters forward to another function. I used to see this type of coding at the beginning of 1990's, but it was quite soon replaced with much more comfortable way, using function parameters. With that style, the above code would shrink as follows:

ble_conn_params_init(
    NULL, FIRST_CONN_PARAMS_UPDATE_DELAY, NEXT_CONN_PARAMS_UPDATE_DELAY,
    MAX_CONN_PARAMS_UPDATE_COUNT, m_hrs.hrm_handles.cccd_handle, true, NULL,
    conn_params_error_handler);

And actually, the extra function definition conn_params_init() would no more be needed at all. This type of coding would make application much much more readable, yet it would save code lines and probably even flash space.

I have to say that your code looks much more complicated that it would need to be. I know this is a kind of "school of thought" issue, and maybe related to "linux-style" -coding. But still, your coding style doesn't make the APIs too easy for customers. Sorry to say, but it has been a horrendous experience to get familiar with nRF51822 SDK (and the DFU OTA is still not working, but that's another story).

  • Yes, actually I was thinking that too, and kind of waiting for this answer :) On the other hand, it would still be possible to hide those complicated "init structures" behind a more comfortable function API. In my opinion, Bluetooth LE "Hello World" should fit into 10..20 lines, instead of almost a thousand.

  • For the sd_*() calls, this style of data passing a side effect of the minimal number of arguments to via SVC calls.

    If it wasn't for the memset, most of this would compile down to const references to flash memory which would use up way less code space then passing parameters on the stack. The use of memset() is to make the code compatible with future libraries that might add additional parameters.

    If everything is constant you can do something like:

    const ble_blah_blah_init_t blah_init =
    {
         .blah1 = blah
         ....
    };
    

    and it will all happily compile away. The same is often (but not always) true for functions with a large number of const arguments that are only called once (-flto is your friend).

  • Yes, from the "machine perspective" the answer is what it is. However, after working decades in this profession, I have started to prefer "human perspective", because we ourselves have so limited resources. Developer's "cognitive load" shouldn't be increased by purpose. I mean that it is mostly better to write code that is "easy-to-read" than code that is "easy-to-execute", so to say. This paradigm produces less errors, and eventually better code.

  • Yeah, you got the point perfectly :) I was eventually speaking both things you mention because they are a bit interleaved. Prefer many simple functions to a few complex ones. Prefer giving working default values to requiring setting all bits and pieces every time.

  • How do you work with default values in the scheme where you pass ALL the parameters to the function? How would it help filling default values into each call? Or you expect functions which basically cuts the variability and allow to set just half or quarter of parameters while they fill the rest by defaults? Isn't it exactly what is done in my example above (= if you decide that your app is using just some configs you do your applicative layer)? Or you expect SDK offering dozens of such functions just to be able to cover all different scenarios in order to satisfy different "users"? Not convincing (at all).

Related