Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

sd: handle system attibutes on reconnect #86

Draft
wants to merge 1 commit into
base: dev
Choose a base branch
from
Draft

sd: handle system attibutes on reconnect #86

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 23 additions & 10 deletions adapter_nrf51.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,19 @@ func handleEvent() {
currentConnection.Reg = gapEvent.conn_handle
DefaultAdapter.connectHandler(Address{}, true)
case C.BLE_GAP_EVT_DISCONNECTED:
if debug {
println("evt: disconnected")
}
// Store system attributes data
dataLen := uint16(0)
C.sd_ble_gatts_sys_attr_get(gapEvent.conn_handle, nil, &dataLen, 0) // get data length
Comment on lines +57 to +58
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are doing a heap allocation here (dataLen escapes to the heap). My suggestion is to make dataLen a global.

if int(dataLen) <= cap(DefaultAdapter.systemAttributes) { // we can not allocate here, so ensure at least data fits the buffer
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@aykevl I may need your help to solve this. We don't know in advance how large system attributes blob will be and we can't allocate here.
So far, I've seen sizes around 16 bytes, but I assume size depends on the number of characteristics published.
In this PR, I'm pre-allocating 64 bytes array and just skipping the storing logic if the blob is larger. I'm sure this can be handled better, but I don't know how.

DefaultAdapter.systemAttributes = DefaultAdapter.systemAttributes[:dataLen]
C.sd_ble_gatts_sys_attr_get(gapEvent.conn_handle, &DefaultAdapter.systemAttributes[0], &dataLen, 0)
}
// Clean up state for this connection.
currentConnection.Reg = C.BLE_CONN_HANDLE_INVALID
// Auto-restart advertisement if needed.
if defaultAdvertisement.isAdvertising.Get() != 0 {
// The advertisement was running but was automatically stopped
// by the connection event.
Expand All @@ -59,7 +72,6 @@ func handleEvent() {
// necessary.
defaultAdvertisement.start()
}
currentConnection.Reg = C.BLE_CONN_HANDLE_INVALID
DefaultAdapter.connectHandler(Address{}, false)
case C.BLE_GAP_EVT_CONN_PARAM_UPDATE_REQUEST:
// Respond with the default PPCP connection parameters by passing
Expand Down Expand Up @@ -87,15 +99,16 @@ func handleEvent() {
handler.callback(Connection(gattsEvent.conn_handle), int(writeEvent.offset), data)
}
case C.BLE_GATTS_EVT_SYS_ATTR_MISSING:
// This event is generated when reading the Generic Attribute
// service. It appears to be necessary for bonded devices.
// From the docs:
// > If the pointer is NULL, the system attribute info is
// > initialized, assuming that the application does not have any
// > previously saved system attribute data for this device.
// Maybe we should look at the error, but as there's not really a
// way to handle it, ignore it.
C.sd_ble_gatts_sys_attr_set(gattsEvent.conn_handle, nil, 0, 0)
if debug {
println("evt: sys attr missing")
}
// Try and restore system attributes data if we have any (from previous connections)
// Fallback to initialize them from scratch otherwise
if len(DefaultAdapter.systemAttributes) > 0 {
C.sd_ble_gatts_sys_attr_set(gattsEvent.conn_handle, &DefaultAdapter.systemAttributes[0], uint16(len(DefaultAdapter.systemAttributes)), 0)
} else {
C.sd_ble_gatts_sys_attr_set(gattsEvent.conn_handle, nil, 0, 0)
}
default:
if debug {
println("unknown GATTS event:", id, id-C.BLE_GATTS_EVT_BASE)
Expand Down
26 changes: 17 additions & 9 deletions adapter_nrf528xx-full.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,13 @@ func handleEvent() {
if debug {
println("evt: disconnected")
}
// Store system attributes data
dataLen := uint16(0)
C.sd_ble_gatts_sys_attr_get(gapEvent.conn_handle, nil, &dataLen, 0) // get data length
if int(dataLen) <= cap(DefaultAdapter.systemAttributes) { // we can not allocate here, so ensure at least data fits the buffer
DefaultAdapter.systemAttributes = DefaultAdapter.systemAttributes[:dataLen]
C.sd_ble_gatts_sys_attr_get(gapEvent.conn_handle, &DefaultAdapter.systemAttributes[0], &dataLen, 0)
}
Comment on lines +55 to +58
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is no need to call this function twice: you can set dataLen to the length of the buffer and check the NRF_ERROR_DATA_SIZE return value.

// Clean up state for this connection.
for i, cb := range gattcNotificationCallbacks {
if cb.connectionHandle == currentConnection.Reg {
Expand Down Expand Up @@ -116,15 +123,16 @@ func handleEvent() {
handler.callback(Connection(gattsEvent.conn_handle), int(writeEvent.offset), data)
}
case C.BLE_GATTS_EVT_SYS_ATTR_MISSING:
// This event is generated when reading the Generic Attribute
// service. It appears to be necessary for bonded devices.
// From the docs:
// > If the pointer is NULL, the system attribute info is
// > initialized, assuming that the application does not have any
// > previously saved system attribute data for this device.
// Maybe we should look at the error, but as there's not really a
// way to handle it, ignore it.
C.sd_ble_gatts_sys_attr_set(gattsEvent.conn_handle, nil, 0, 0)
if debug {
println("evt: sys attr missing")
}
// Try and restore system attributes data if we have any (from previous connections)
// Fallback to initialize them from scratch otherwise
if len(DefaultAdapter.systemAttributes) > 0 {
C.sd_ble_gatts_sys_attr_set(gattsEvent.conn_handle, &DefaultAdapter.systemAttributes[0], uint16(len(DefaultAdapter.systemAttributes)), 0)
} else {
C.sd_ble_gatts_sys_attr_set(gattsEvent.conn_handle, nil, 0, 0)
}
case C.BLE_GATTS_EVT_EXCHANGE_MTU_REQUEST:
// This event is generated by some devices. While we could support
// larger MTUs, this default MTU is supported everywhere.
Expand Down
27 changes: 18 additions & 9 deletions adapter_nrf528xx-peripheral.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,14 @@ func handleEvent() {
if debug {
println("evt: disconnected")
}
// Store system attributes data
dataLen := uint16(0)
C.sd_ble_gatts_sys_attr_get(gapEvent.conn_handle, nil, &dataLen, 0) // get data length
if int(dataLen) <= cap(DefaultAdapter.systemAttributes) { // we can not allocate here, so ensure at least data fits the buffer
DefaultAdapter.systemAttributes = DefaultAdapter.systemAttributes[:dataLen]
C.sd_ble_gatts_sys_attr_get(gapEvent.conn_handle, &DefaultAdapter.systemAttributes[0], &dataLen, 0)
}
// Clean up state for this connection.
currentConnection.Reg = C.BLE_CONN_HANDLE_INVALID
// Auto-restart advertisement if needed.
if defaultAdvertisement.isAdvertising.Get() != 0 {
Expand Down Expand Up @@ -71,15 +79,16 @@ func handleEvent() {
handler.callback(Connection(gattsEvent.conn_handle), int(writeEvent.offset), data)
}
case C.BLE_GATTS_EVT_SYS_ATTR_MISSING:
// This event is generated when reading the Generic Attribute
// service. It appears to be necessary for bonded devices.
// From the docs:
// > If the pointer is NULL, the system attribute info is
// > initialized, assuming that the application does not have any
// > previously saved system attribute data for this device.
// Maybe we should look at the error, but as there's not really a
// way to handle it, ignore it.
C.sd_ble_gatts_sys_attr_set(gattsEvent.conn_handle, nil, 0, 0)
if debug {
println("evt: sys attr missing")
}
// Try and restore system attributes data if we have any (from previous connections)
// Fallback to initialize them from scratch otherwise
if len(DefaultAdapter.systemAttributes) > 0 {
C.sd_ble_gatts_sys_attr_set(gattsEvent.conn_handle, &DefaultAdapter.systemAttributes[0], uint16(len(DefaultAdapter.systemAttributes)), 0)
} else {
C.sd_ble_gatts_sys_attr_set(gattsEvent.conn_handle, nil, 0, 0)
}
case C.BLE_GATTS_EVT_EXCHANGE_MTU_REQUEST:
// This event is generated by some devices. While we could support
// larger MTUs, this default MTU is supported everywhere.
Expand Down
11 changes: 11 additions & 0 deletions adapter_sd.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,13 +50,24 @@ type Adapter struct {
charWriteHandlers []charWriteHandler

connectHandler func(device Address, connected bool)

// System attributes in context of SoftDevice primarily mean Client Characteristic Configuration Descriptors (CCCD).
// It is mandated by Bluetooth specification that CCCD values for a bonded peer should be stored between connections.
//
// Our bluetooth stack stores system attributes on disconnect
// and provides them back to SoftDevice on BLE_GATTS_EVT_SYS_ATTR_MISSING event.
//
// Note: CCCD values can be altered only by the peer, so you cannot change them from the application.
// Treat this data as a blob of unknown format, i.e. store it as is and provide it back as is, without changing it.
systemAttributes []byte
}

// DefaultAdapter is the default adapter on the current system. On Nordic chips,
// it will return the SoftDevice interface.
//
// Make sure to call Enable() before using it to initialize the adapter.
var DefaultAdapter = &Adapter{isDefault: true,
systemAttributes: make([]byte, 64)[:0], // capacity 64, length 0
connectHandler: func(device Address, connected bool) {
return
}}
Expand Down