Bluetooth API to access bond data

[rakons]

There currently is no API nor any pre-defined polices what to do with multiple bonds - if new bond is created witch should be removed?

There should be an API to access and identify bonds and delete selected ones. Then some kind of automatic policy configuration may be implemented - but this is nice to have feature. Having an option to access and manually delete bond data is a must.

[jhedberg]

@Vudentz @sjanc @carlescufi @joerchan FYI

[jhedberg]

I can think of two possible ways to do this:

/* addr_count contains max number of returned addresses upon function entry, 
 * and actual number of returned addresses on function return.
 */
void bt_le_get_bonds(bt_addr_le_t *addrs, size_t *addr_count)'

or:

/* Call the given callback for each bonded device */
void bt_le_foreach_bond(void (*func)(const bt_addr_le_t *addr, void *user_data), void *user_data);

I've used bt_le_ instead of just bt_ as a prefix since in principle BR/EDR would need a different API.

Another question is: is just the bdaddr sufficient information for the app, or do we want to expose other properties of the bond (security, LTK, CSRK, IRK availability, etc).

[rakons]

Or maybe:

/*
 * @param[out] addr would be filled with the bond data
 * @param[in] idx contains index of the bond
 *
 * @retval false No bond data with such index
 * @retval true Bond data aviable
 */
bool bt_le_get_bond(bt_addr_le_t *addr, size_t idx);

[carlescufi]

@rakons what's the rationale behind using an index?

[rakons]

For your second question, sometimes it would be great to have some user data that may be added to a bond. For example - Central is pairing with keyboard and mouse. When new device is bonding and it is a mouse, I wish to remove old mouse bond. I think that the easiest way to detect that this bond references to a mouse would be if we would have some possibility to set and get user data together with a bond.

[carlescufi]

sometimes it would be great to have some user data that may be added to a bond.

+1 to that, we have had multiple requests in the past in this direction. That said, this can of course be achieved at the application level, but I think it makes sense to incorporate it in the Bluetooth subsystem.

[rakons]

@carlescufi RAM usage - you do not have to copy all the data into the memory, you may iterate through them, but just simpler than using foreach construction that uses callback, just:

addr;
for(size_t i=0; bt_le_get_bond(&addr, i); ++i)
{
// process
}

[carlescufi]

@rakons right. Not sure if I like the concept of an index, but then maybe have an iterator instead (start(), next()). Thoughts?

[rakons]

@carlescufi Only if it would work safely if multiple threads would like to iterate the bonds - maybe it makes not a big sense to iterate bonds from multiple threads, but is should be safe at least.

[carlescufi]

@rakons good point. Let's hear from @jhedberg and the others as well.

[jhedberg]

A "foreach" helper would at least be consistent with existing practice, e.g.:

void bt_gatt_foreach_attr(u16_t start_handle, u16_t end_handle,                 
                          bt_gatt_attr_func_t func, void *user_data);    

[jhedberg]

Also, it feels a bit weird to expose an internal index to the app since it leaks something about the internal implementation (e.g. makes sense if there's an array internally, but would be inefficient if it's a linked list)

[jhedberg]

Regarding attaching user data to a bond - not opposed to it. The internal representation of a bond is the bt_keys struct, so it'd need to get void *data'; size_t data_len; members, and the stack would then take care of writing/reading to/from flash. And then there'd be associated bt_bond_get_data() and bt_bond_set_data() helpers.

[carlescufi]

@jhedberg +1 to the foreach proposal. I dislike exposing the index as well.

[rakons]

@jhedberg Be careful with saving pointers as it might invalidate. I think it is not the best idea to just serialize whole data, it may be just a pointer to a flash sometimes. It may also be pointer to some structure where some fields have to be saved and the rest may be recreated, or the saved values would not be valid after restart. I rather think about moving the duty of saving and restoring user data to the user. Then there should be some callbacks to support it.

[jhedberg]

@rakons understood - I still think we might be able to do better than leave the storing up to the application. Instead of adding any pointers to bt_keys the get/set APIs could also just map straight to the settings API, i.e. settings_set_value() and settings_get_value(), i.e. the RAM storage would be provided by the app.

[carlescufi]

@rakons ~have you looked at the settings subsystem and how BLE uses it? unfortunately those choices have already been made in order to preserve filesystem independence and for other reasons.~

I believe I misunderstood, disregard.

[rakons]

I am looking into it right now.

[rakons]

Well... the key-value strings is not the best option in embedded world. As far as string as a key seems reasonable, string as a value is an overkill (still in embedded world).

But, without touching the API itself, we may add just simple base64 coder/decoder to process binary data blobs effectively.

[jhedberg]

There is work ongoing to do stream-based reading/writing of settings data, that wouldn't require intermediate buffers in PR #9521.

Regarding the settings APIs I mentioned, it's actually not the get/set that this would map to. We may need the app to provide callbacks for data read from flash since it's only read from there upon settings_load().