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

Parents
  • 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.

Reply
  • 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.

Children
No Data
Related