Configured advertising duration and interval dont match up observed values for ble_app_eddystone example. Is this a bug?

[Prasad]

I am using sdk15.2.0 with PCA10040 dev board.

In ble_app_eddystone example, the configured advertising interval and duration dont match the observed values.

I think I have found the reason why they dont match... Just want a confirmation from Nordic engineers weather I am correct or not.

Below code snippet is from es_app_config.h file 

#define APP_CFG_NON_CONN_ADV_TIMEOUT                        0                           //!< Time for which the device must be advertising in non-connectable mode (in seconds). 0 disables the time-out.
#define APP_CFG_NON_CONN_ADV_INTERVAL_MS                    1000                        //!< The advertising interval for non-connectable advertisement (in milliseconds). This value can vary between 100 ms and 10.24 s.
#define APP_CFG_CONNECTABLE_ADV_TIMEOUT                     6000                        //!< Time for which the device must be advertising in connectable mode (in milliseconds). 0 disables the time-out.
#define APP_CFG_CONNECTABLE_ADV_INTERVAL_MS                 100                         //!< The advertising interval for connectable advertisement (in milliseconds). This value can vary between 20 ms and 10.24 s.

As per the comment in the code, the values are in milli seconds.

In es_adv.c in get_adv_params() values from es_app_config.h are assigned to ble_gap_adv_params_t structure. Below code is from es_adv.c

/**@brief  Given state of Eddystone beacon, get advertisement parameters. */
static void get_adv_params(ble_gap_adv_params_t * p_adv_params,
                           bool                   non_connectable,
                           bool                   remain_connectable)
{
    // Initialize advertising parameters (used when starting advertising).
    memset(p_adv_params, 0, sizeof(ble_gap_adv_params_t));

    // Non-connectable
    p_adv_params->properties.type = non_connectable
                                  ? BLE_GAP_ADV_TYPE_NONCONNECTABLE_NONSCANNABLE_UNDIRECTED
                                  : BLE_GAP_ADV_TYPE_CONNECTABLE_SCANNABLE_UNDIRECTED;
    p_adv_params->p_peer_addr     = NULL; // Undirected advertisement.
    p_adv_params->filter_policy   = BLE_GAP_ADV_FP_ANY;
    p_adv_params->interval        = non_connectable ? m_adv_interval : APP_CFG_CONNECTABLE_ADV_INTERVAL_MS;
    p_adv_params->duration        = non_connectable
                                  ? APP_CFG_NON_CONN_ADV_TIMEOUT
                                  : (remain_connectable ? 0 : APP_CFG_CONNECTABLE_ADV_TIMEOUT);
    p_adv_params->primary_phy     = BLE_GAP_PHY_1MBPS;
}

Now if you check ble_gap_adv_params_t structure definition in ble_gap.h, the interval and duration values are not in milli seconds(read the comments).

  uint32_t                 interval;                /**< Advertising interval in 625 us units. @sa BLE_GAP_ADV_INTERVALS.
                                                         @note If @ref ble_gap_adv_properties_t::type is set to
                                                               @ref BLE_GAP_ADV_TYPE_CONNECTABLE_NONSCANNABLE_DIRECTED_HIGH_DUTY_CYCLE
                                                               advertising, this parameter is ignored. */
  uint16_t                 duration;                /**< Advertising duration in 10 ms units. When timeout is reached,
                                                         an event of type @ref BLE_GAP_EVT_ADV_SET_TERMINATED is raised.
                                                         @sa BLE_GAP_ADV_TIMEOUT_VALUES.
                                                         @note The SoftDevice will always complete at least one advertising

Milli second macros from es_config.h should have been converted to respective count before use. This is the reason why configured and observed values don't match up.

Please confirm my understanding...

[Sigurd]

Hi,

Yes, I can confirm that this is a bug.

A fix could be to convert the interval from ms units to 625 us units in adv_start() in es_adv.c 

Snippet:

static void adv_start(ble_gap_adv_params_t * p_adv_params)
{
    ret_code_t err_code = NRF_SUCCESS;

    es_tlm_adv_cnt_inc();

    uint32_t interval_0625 = MSEC_TO_UNITS(p_adv_params->interval,UNIT_0_625_MS);

    p_adv_params->interval = interval_0625;

    err_code = sd_ble_gap_adv_set_configure(mp_adv_handle, &m_adv_data, p_adv_params);
    APP_ERROR_CHECK(err_code);

    err_code = sd_ble_gap_adv_start(*mp_adv_handle, BLE_CONN_CFG_TAG_DEFAULT);

    if (err_code != NRF_ERROR_BUSY && err_code != NRF_SUCCESS)
    {
        APP_ERROR_CHECK(err_code);
    }
}

OR maybe better, in the get_adv_params() function.

Snippet:

/**@brief  Given state of Eddystone beacon, get advertisement parameters. */
static void get_adv_params(ble_gap_adv_params_t * p_adv_params,
                           bool                   non_connectable,
                           bool                   remain_connectable)
{
    // Initialize advertising parameters (used when starting advertising).
    memset(p_adv_params, 0, sizeof(ble_gap_adv_params_t));

    // Non-connectable
    p_adv_params->type =
        non_connectable ? BLE_GAP_ADV_TYPE_ADV_NONCONN_IND : BLE_GAP_ADV_TYPE_ADV_IND;
    p_adv_params->p_peer_addr = NULL; // Undirected advertisement.
    p_adv_params->fp          = BLE_GAP_ADV_FP_ANY;

    p_adv_params->interval    = non_connectable ? BLE_GAP_ADV_INTERVAL_MAX : 1000;

    uint32_t interval_0625 = MSEC_TO_UNITS(p_adv_params->interval,UNIT_0_625_MS);
    p_adv_params->interval = interval_0625;


    p_adv_params->timeout     = non_connectable
                                ? APP_CFG_NON_CONN_ADV_TIMEOUT
                                : (remain_connectable ? 0 : APP_CFG_CONNECTABLE_ADV_TIMEOUT);
}

[Prasad]

Sigurd

I would prefer doing it in get_adv_params(). Also, the same issue applies to p_adv_params->duration.

I guess the snippet you posted is not from sdk15.2.0, because you have written p_adv_params->timeout and not p_adv_params->duration!

[Sigurd]

Yes, I agree, get_adv_params() would be the preferred place.

Prasad said:

I guess the snippet you posted is not from sdk15.2.0, because you have written p_adv_params->timeout and not p_adv_params->duration!

Ah, that is correct, that snippet was based on SDK 14.2

[Prasad]

Sigurd

There is another issue now.

The spacing between two advertising intervals is not constant.

These are my configurations in es_app_config.h...

#define APP_CFG_NON_CONN_ADV_TIMEOUT                        2000                       //!< Time for which the device must be advertising in non-connectable mode (in seconds). 0 disables the time-out.
#define APP_CFG_NON_CONN_ADV_INTERVAL_MS                    1000                        //!< The advertising interval for non-connectable advertisement (in milliseconds). This value can vary between 100 ms and 10.24 s.
#define APP_CFG_CONNECTABLE_ADV_TIMEOUT                     1000                        //!< Time for which the device must be advertising in connectable mode (in milliseconds). 0 disables the time-out.
#define APP_CFG_CONNECTABLE_ADV_INTERVAL_MS                 100                         //!< The advertising interval for connectable advertisement (in milliseconds). This value can vary between 20 ms and 10.24 s.

And this is what I observe with the changes that you suggested.

This issue occurs in unmodified sdk15.2.0 as well. I checked that to be sure that my changes have not caused it.

Any idea why this should happen?

Also, APP_CFG_NON_CONN_ADV_TIMEOUT has no effect, I expected that non-connectable advertising should stop after the timeout but that didn't happen.

Should I raise another question for this?

[Prasad]

Hi Sigurd

There are too many timing related issues in ble_app_eddystone. Looks like the developer completely forgot about converting milli second values. es_app_config.h is confusing, the developer who wrote this should take a relook at this file. Here are a few that I can list...

1. The macro APP_CONFIG_ADV_INTERVAL_MS_MAX has "_MS_" in it. I assumed this would be in milliseconds. But the value assigned to this macro is 16384. As per standard the max advertising interval is 10.24 sec. And all the while I was guessing if the standards have changed, it took me some time to realize that 16384 * 0.625 = 10240ms! 

2. Apparently there is one more macro, MAX_ADV_INTERVAL, in the same file for the same purpose but not used anywhere! Again there is contradiction between the comment and macro name.

#define MAX_ADV_INTERVAL                                    (10240)                     //!< Maximum connection interval (in ms).

3. If you change advertising interval from nRF Beacon for Eddystone app then again the advertising intervals go haywire. Looks like the milli sec values are not converted to interval counts.

There are many such issue in this example, I may not be able to list them all. Can you request for a proper fix from the developer? Because if I do it I would need to spend more time and efforts in understanding the sdk code rather than writing my application.

Usually your sdk code and examples are pretty awesome and rarely I find such issues with it but I wonder how this work got approved to be included in the release!