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.

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

Bluetooth Client Characteristic Configuration Descriptor Persistence #160

Open
bsiever opened this issue Jan 19, 2022 · 4 comments
Open

Comments

@bsiever
Copy link

bsiever commented Jan 19, 2022

Description of the problem:

The bluetooth specs require the Client Characteristic Configuration Descriptor (CCCD) be persistent across connections for bonded devices (see here). The underlying Nordic stack manages persistence, but this is not propagated back to the CODAL layer on re-connects.

For example,

  1. A device connects to and pairs with a micro:bit
  2. The device enables notifications
  3. The device disconnects
  4. The device reconnects
  5. The underlying CCCD value in the BLE stack is restored, but there's no update to the MicroBitBLEChar objects' internal states (their cccd's), so notifications from CODAL won't propagate due to internal state checks (like this).

Possible solution

The easiest update may be:

  1. Changing the meaning of the cccd ivar in MicroBitBLEChar to be a boolean that indicates a CCCD exists for the characteristic (currently it is a shadow copy of the expected meaning of the CCCD).
  2. Update the cccdNotify() and cccdIndicate() predicates to retrieve the actual CCCD value from the stack (first check the cccd ivar to see if there is a CCCD for this characteristic).

Concerns

Existing Bluetooth apps/features would need additional testing following this change. It's likely that all current BLE apps/services for the micro:bit are implemented to automatically reset CCCDs of interest on connection rather than rely on the specified behavior, so impact is probably (hopefully) minimal.

Justification

The micro:bit should conform to Bluetooth specifications for compatibility with non-custom services. For example, some Android and Windows devices expect the values to persist for BLE Human Interface Devices. Failure to re-enable them correctly prevents micro:bit services from working correctly on reconnects (workarounds are possible to overcome this).

@bsiever
Copy link
Author

bsiever commented Jan 19, 2022

My "hack" to work around this involves using the Nordic pm_register to monitor events from the peer_manager. Anytime an PM_EVT_PEER_DATA_UPDATE_SUCCEEDED happens, I retrieve all CCCD data from the stack and use the values for setCCCD(). Example here.

That shows what I mean by "retrieving the value from the stack" (using sd_ble_gatts_value_get() on the CCCD).

@bsiever
Copy link
Author

bsiever commented Jan 19, 2022

Quick thought: Checking cccd probably isn't needed. cccd itself could be eliminated and handles' cccd field could be used (if it's 0, no CCCD to check).

@martinwork
Copy link
Collaborator

@bsiever I have experimented with using sd_ble_gatts_value_get() instead of caching the value after a write. On reconnection, the flags read from sd_ble_gatts_value_get() get set before the connection is ready. The member variable CCCD is not quite just a copy of the stack value. It remembers that the client has written to the CCCD since reset.

Solely using the stack value causes a problem in the Bluetooth UART service. It starts sending before the connection is ready, encounters a BLE error it doesn't expect, and gets stuck because it doesn't get the confirmation for the packet it thinks it has sent. This occurs on every reboot so it never works a second time. This is really a bug in the UART service, but confirms you're right to be cautious!

Maybe a safe way to improve the situation is to allow individual services to turn on direct access to the stack values, or perhaps add an optional mode parameter to specify checking the member variable and/or stack values. Should the characteristic base class handle waiting for the connection to be ready, or should that be left to the individual service?

@bsiever
Copy link
Author

bsiever commented Jan 19, 2023

@martinwork It's been a while since I looked into this. I think the part that breaks the standard is that the cccds are not restored following a reset. Being retained between connections is ok.

Isn't nearly any change to the values in the BLE stack mostly dependent on getConnectionHandle() already? Things like if ( connection == BLE_CONN_HANDLE_INVALID) return false; already guard changes changes until the connection is ready. I'd need to think about this a lot more, but maybe it's possible to just modify getConnectionHandle() so it is dependent on both the handle and that other connection initialization is done.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants