Hardfault ble_nus_send_data - SDK 17

[Roei]

Hi,

I've noticed that there was an issue with ble_nus in SDK 15.3 - detailed in this post.

I'm using SDK 17, and in my application, I don't check data_handler != NULL. When BLE_GATTS_EVT_HVN_TX_COMPLETE occurs, I just define ble_nus_evt_t evt and calls p_nus->data_handler. Probably took it from an example, I can't remember.

Is it safe? Should I use something else?

Thanks!

[Einar Thorsrud]

Hi,

This issue is fixed in SDK 17.0.2.

[Roei]

Hi,

Thanks for your response!

Does it fixed event when I don't use on_hvx_tx_complete() function? Also when I just call p_nus->data_handler() without any NULL validation?

Thanks!

[Einar Thorsrud]

Hi,

Perhaps I was a bit too quick My answer was related to the first part of your question, where the issue discussed in this thread is fixed.

Regarding the second question I do not fully understand it, as this is handled by the NUS service implementation (components\ble\ble_services\ble_nus\ble_nus.c) without you having to think about it. Perhaps you can share your code to provide context, and explain in more detail what you are asking about?

[Roei]

Hi,

This is the implementation in my application, under ble_nus.c:

case BLE_GATTS_EVT_HVN_TX_COMPLETE:
{
    //notify with empty data that some tx was completed.
    ble_nus_evt_t evt = {
                 .type = BLE_NUS_EVT_TX_RDY,
                 .p_nus = p_nus
                                     };
p_nus->data_handler(&evt);
}
break;

As it's not exactly as the SDK code, probably because it's from another example, I'm not sure if it's safe enough, because I don't have any validation if p_nus->data_handler != NULL.

In other cases of this switch, I can see other uses of p_nus->data_handler without validation.

What is your opinion?

Thanks!

[Einar Thorsrud]

Hi,

As long as you provided a data handler when you initialized the NSU Service, then this is no issue. (I cannot say more specific then that as I have not seen enough of your code)

[Roei]

Hi,

I've provided an handler during initialization, but events handler takes ble_nut_t pointer from an argument to the function, as following:

void OnBleNusEvent(ble_evt_t const * p_ble_evt, void * p_context)

if ((p_context == NULL) || (p_ble_evt == NULL))
{
   return;
}

ble_nus_t * p_nus = (ble_nus_t *)p_context;

After that, I'm using p_nus->data_handler.

Is it safe to use p_context?

Thanks!

[Einar Thorsrud]

To be frank, I am seeing only pieces of your modified NUS implementation, and without having the full picture I cannot give any guarantees of anything. If you set p_context to a valid value, then it is safe to use it. If not, then don't. You do check if it is NULL, so unless you put in same random address that check will suffice. So I cannot imagine that there will be an issue here, and if it is, that would be something with your code which I do not see, and not related to the earlier mentioned issue which is fixed in the SDK and also in your code.

[Roei]

This function is an observer, I'm using NRF_SDH_BLE_OBSERVER to register it.

Hence - p_context comes from outside, so I can't know what is it

[Einar Thorsrud]

Yes you can. The context is the last parameter you pass to NRF_SDH_BLE_OBSERVER.

[Roei]

So it looks like that:

#define BLE_NUS_DEF(_name) \
static ble_nus_t _name; \
NRF_SDH_BLE_OBSERVER(_name ## _obs, \
BLE_NUS_BLE_OBSERVER_PRIO, \
OnBleNusEvent, &_name)

[Einar Thorsrud]

In that case the pointer to _name is your context pointer