bluekitchen / btstack

Dual-mode Bluetooth stack, with small memory footprint.
http://bluekitchen-gmbh.com
Other
1.71k stars 607 forks source link

ENABLE_GATT_FIND_INFORMATION_FOR_CCC_DISCOVERY codepath uses incorrect gatt_client state variable #632

Open shermp opened 2 weeks ago

shermp commented 2 weeks ago

Describe the bug

When defining ENABLE_GATT_FIND_INFORMATION_FOR_CCC_DISCOVERY, btstack does not compile due to gatt_client_t not having the gatt_client_state struct member.

To Reproduce

  1. Define ENABLE_GATT_FIND_INFORMATION_FOR_CCC_DISCOVERY in btstack_config.h
  2. Attempt to compile

Expected behavior btstack compiles.

Environment: (please complete the following information):

Additional context I did a quick fixup in https://github.com/shermp/btstack/commit/4b82482df9ee5adee540ae826d2efb90e06ba5b7 to test with a Resound hearing aid. I don't know if I missed anything, or if it was the correct approach.

Unfortunately, this option is required for the resound hearing aid. It has an issue where it fails to read the CCCD for the required characteristic (it also happens on Android), and so this alternative method is required to be able to subscribe to notifications.

Is there any possibility this could become a runtime option, or even be available as an automatic fallback?

mringwal commented 1 week ago

Thanks for reporting. I've fixed the compile error on develop.

Could you post a log of the failed CCCD? In which setup does it fail with Android? If I remember correctly, I found our default logic to be better/more efficient, but if that causes issues, it would make more sense to either switch or at least implement a fallback (interesting idea btw, thanks!).

thewierdnut commented 1 week ago

Do you need a full log, or just the relevant packets?

mringwal commented 1 week ago

In this case, the packets since the connection complete would be fine (to get some context).

thewierdnut commented 1 week ago

Here are logs from both the android and pico captures.

resound_status_ccc_find_info_logs.zip

mringwal commented 1 week ago

Thanks for the logs. Is "resound android status ccc.pcap" the one taken on the Android device for comparison? The BTstack one "resound pico statuc ccc patched" seems to have ENABLE_GATT_FIND_INFORMATION_FOR_CCC_DISCOVERY enabled. I was interested in to see the error without that, i.e. using Read By Type Request (CCCD). Could you remove ENABLE_LOG_DEBUG (by default) and send a failing trace?

shermp commented 1 week ago

Hi @mringwal

I've trimmed the pico log (where we discovered the issue) to remove debug statements, and advertisement packets.

resound_before_cccd_no_ad.zip