Hi Marcel, Alain, On Fri, May 1, 2020 at 11:04 AM Marcel Holtmann <marcel@xxxxxxxxxxxx> wrote: > > Hi Alain, > > >>>>> The gatt discovery proceedure simplification to discover all > >>>>> characteristics at once has exposed a device side issue where some > >>>>> device implementation reports orphaned characteristics. While this > >>>>> technically shouldn't be allowed, some instances of this were observed > >>>>> (namely on some Android phones). > >>>>> > >>>>> The fix is to simply skip over orphaned characteristics and continue > >>>>> with everything else that is valid. > >>>>> > >>>>> This has been tested as part of the Android CTS tests against an > >>>>> affected platform and was confirmed to have worked around the issue. > >>>>> --- > >>>>> > >>>>> src/shared/gatt-client.c | 5 ++++- > >>>>> 1 file changed, 4 insertions(+), 1 deletion(-) > >>>>> > >>>>> diff --git a/src/shared/gatt-client.c b/src/shared/gatt-client.c > >>>>> index 963ad619f..d604c77a3 100644 > >>>>> --- a/src/shared/gatt-client.c > >>>>> +++ b/src/shared/gatt-client.c > >>>>> @@ -632,7 +632,10 @@ static bool discover_descs(struct discovery_op *op, bool *discovering) > >>>>> util_debug(client->debug_callback, client->debug_data, > >>>>> "Failed to insert characteristic at 0x%04x", > >>>>> chrc_data->value_handle); > >>>>> - goto failed; > >>>>> + > >>>>> + /* Skip over invalid characteristic */ > >>>>> + free(chrc_data); > >>>>> + continue; > >>>>> } > >>>> > >>>> should we add a warning here? > >>> Is there a good way other than the util_debug mechanism to write a warning? > >> > >> you could just use btd_warn() here. If this gets too noisy, we either remove it later or we introduce a btd_warn_ratelimited() or btd_warn_once(). > > This being under src/shared, I wasn't sure if that was acceptable to > > add a btd dependency here, especially with the work to avoid it via > > the util_debug. Happy to do either way, just want to make sure the > > guidance is clear. > > good point. My bad. You better leave the logging out then. There is already some logging a little bit before so I guess we are covered here. -- Luiz Augusto von Dentz
