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).

  • Maybe just keeping layers and modules separated?:) My experience with Nordic nRF5x DSKs is actual pretty opposite then yours... Cheers Jan

  • I don't get your point?

    "function paraeters" are used in both cases:

    • Your style uses a large number of individual parameters;
    • Nordic's style gathers the individual parameters into a single struct.
  • There's lots of reasons embedded code looks like this. One of them is that it's much cheaper, faster, less code, to pass the address of a struct in an ARM ABI call than a ton of arguments. After the first couple of arguments, stack needs to be reserved, all the arguments, including default zero ones, have to be copied to it, then the call. Waste of code, waste of processor cycles, waste of RAM.

    If the arguments are static, ie the structure is static, there's no code at all, the struct is static data in flash and is passed directly. I have lots of code like that. If most of the arguments are defaulted, one memzero(), or a memcpy() of a mostly static struct, sets them and you only need code for the ones which aren't, and then a quick pass of the structure.

    The code is also much more self-documenting than un-named arguments, and quite future-proof when new struct fields are added.

  • I guessed that this won't be constructive discussion because we all already have our fixed opinions. Well. At least "unnamed arguments" is from past history, since the IDE's nowadays let you know the arguments after typing the opening bracked "(". And I am not the only one with this kind of opinions.

    If Bluetooth LE "Hello world!" takes hundred of code lines that are always the same from application to application, there MUST be something wrong with the API. You are actually missing the final layer towards application developer.

Related