ARMmbed / ble-nrf51822

Nordic stack and drivers for the mbed BLE_API
Other
46 stars 51 forks source link

rgrover patch fixed #47

Closed fabiencomte closed 8 years ago

fabiencomte commented 8 years ago

your patch fixed (works with gapConnectionHandle)

sd_ble_gatts_hvx still wrong sometimes on triangle test exemple but it looks like a s130 issue.

fabiencomte commented 8 years ago

gapConnectionHandle works (tested) but connectionHandle not works (tested too). It's normal for me because it's a handle on a "kind of value" and connections can have multiple values for the same gatt server so you need to know the one that you work with, right ?

rgrover commented 8 years ago

Except for the CCCD, all gattserver attributes should be connection independent. So in the default case, I expect write() to be called without a connection handle. In some corner cases, I can see that the peripheral app might want to update CCCD using the connection handle, and then it should be allowed to do so.

Please correct me if I'm wrong, but I feel we should be using the connectionHandle that is passed in.

fabiencomte commented 8 years ago

You loose me, I'm not as expert as you with bluetooth :-) But in the special case of S130, you can only have one connection as device (and 3 as central) so the is always 1 connection for gattserver, it's trivial. But imagine that if you have multiple connections as device, i can understand the need of having multiple instances of gattserver.

fabiencomte commented 8 years ago

A concrete exemple is one of the peer can allow notifications and the other one not (or maybe later) so gattserver canno't act the same wait for both connections.

fabiencomte commented 8 years ago

Stupid question, can BLE exchange user packets without any GATT ?

rgrover commented 8 years ago

I'll look into your comments carefully next week, but regarding your question about exchanging user-packets without GATT: there's always the advertisement packet (and scan response to add to that). That's what beacons are for.

Will reconnect next week. Have a good weekend.

fabiencomte commented 8 years ago

I wish you a good week end.

Envoyé depuis Yahoo Mail pour Android

De:"Rohit Grover" notifications@github.com Date:ven. j août PM à 18:22 Objet:Re: [ble-nrf51822] rgrover patch fixed (#47)

I'll look into your comments carefully next week, but regarding your question about exchanging user-packets without GATT: there's always the advertisement packet (and scan response to add to that). That's what beacons are for.

Will reconnect next week. Have a good weekend.

— Reply to this email directly or view it on GitHub.

rgrover commented 8 years ago

Fabien, according to the bluetooth spec, there is a single gatt server per device; it maintains a single database of attributes. Only the CCCD attribute of a characteristic is special in the sense that is has per-connection state.

In the default case, I expect GattServer::write() to be called without a connection handle, in which case it is OK to translate the call using CONN_HANDLE_INVALID as in https://github.com/ARMmbed/ble-nrf51822/blob/master/source/nRF5xGattServer.cpp#L202. I hesitate to use nRF5xGap::getInstance().getConnectionHandle() because that API is going to be removed at some point not too far into the future--it was added at a time when multiple concurrent connections weren't possible.

For the other write(), https://github.com/ARMmbed/ble-nrf51822/blob/master/source/nRF5xGattServer.cpp#L205, if the user passes in a connectionHandle, it should be used instead of the handle returned by nRF5xGap::getInstance().getConnectionHandle(). I see that you don't do that. Can you explain why, or can you try using the passed-in connectionHandle?

fabiencomte commented 8 years ago

Hello,

It s just because without, function doesnt work and i reverted to previous version that use it.

Envoyé depuis Yahoo Mail pour Android

De:"Rohit Grover" notifications@github.com Date:mar. j sept. PM à 14:08 Objet:Re: [ble-nrf51822] rgrover patch fixed (#47)

Fabien, according to the bluetooth spec, there is a single gatt server per device; it maintains a single database of attributes. Only the CCCD attribute of a characteristic is special in the sense that is has per-connection state.

In the default case, I expect GattServer::write() to be called without a connection handle, in which case it is OK to translate the call using CONN_HANDLE_INVALID as in https://github.com/ARMmbed/ble-nrf51822/blob/master/source/nRF5xGattServer.cpp#L202. I hesitate to use nRF5xGap::getInstance().getConnectionHandle() because that API is going to be removed at some point not too far into the future--it was added at a time when multiple concurrent connections weren't possible.

For the other write(), https://github.com/ARMmbed/ble-nrf51822/blob/master/source/nRF5xGattServer.cpp#L205, if the user passes in a connectionHandle, it should be used instead of the handle returned by nRF5xGap::getInstance().getConnectionHandle(). I see that you don't do that. Can you explain why, or can you try using the passed-in connectionHandle?

— Reply to this email directly or view it on GitHub.

rgrover commented 8 years ago

@fabiencomte: could you please retest with the latest on develop? thanks.