| Message ID | 1585899591-14623-2-git-send-email-thorsten.klein@bshg.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [BlueZ] ManufacturerData field added to ScanResponse | expand |
Hi Thorsten, On Fri, Apr 3, 2020 at 12:42 AM Klein, Thorsten (BSH) <kleinkastel@googlemail.com> wrote: > > From: "Schachschal, Maximilian (BSH)" <maximilian.schachschal@bshg.com> > > Keys are the Manufacturer ID to associate with the data. > --- > doc/advertising-api.txt | 6 ++++++ > src/advertising.c | 25 +++++++++++++++++++------ > 2 files changed, 25 insertions(+), 6 deletions(-) > > diff --git a/doc/advertising-api.txt b/doc/advertising-api.txt > index b0565ea..14ccae2 100644 > --- a/doc/advertising-api.txt > +++ b/doc/advertising-api.txt > @@ -51,6 +51,12 @@ Properties string Type > the Advertising Data. Keys are the Manufacturer ID > to associate with the data. > > + dict ManufacturerDataScanResponse [Experimental] > + > + Manufactuer Data fields to include in > + the Scan Response. Keys are the Manufacturer ID > + to associate with the data. > + Don't really like to do this, beside it seems to be optional to enter either on AD or on SRD so the scanner is really at fault here if it only able to parse the data if placed on SRD, that said we could have some logic that detects if manufacturer don't fit on the AD push it to SRD if that has more space if the advertisement is discoverable. > array{string} SolicitUUIDs > > Array of UUIDs to include in "Service Solicitation" > diff --git a/src/advertising.c b/src/advertising.c > index 45ff19f..0e1a3f1 100644 > --- a/src/advertising.c > +++ b/src/advertising.c > @@ -328,12 +328,12 @@ fail: > } > > static bool parse_manufacturer_data(DBusMessageIter *iter, > - struct btd_adv_client *client) > + struct btd_ad *ad) > { > DBusMessageIter entries; > > if (!iter) { > - bt_ad_clear_manufacturer_data(client->data); > + bt_ad_clear_manufacturer_data(ad); > return true; > } > > @@ -342,7 +342,7 @@ static bool parse_manufacturer_data(DBusMessageIter *iter, > > dbus_message_iter_recurse(iter, &entries); > > - bt_ad_clear_manufacturer_data(client->data); > + bt_ad_clear_manufacturer_data(ad); > > while (dbus_message_iter_get_arg_type(&entries) > == DBUS_TYPE_DICT_ENTRY) { > @@ -373,7 +373,7 @@ static bool parse_manufacturer_data(DBusMessageIter *iter, > > DBG("Adding ManufacturerData for %04x", manuf_id); > > - if (!bt_ad_add_manufacturer_data(client->data, manuf_id, > + if (!bt_ad_add_manufacturer_data(ad, manuf_id, > manuf_data, len)) > goto fail; > > @@ -383,10 +383,22 @@ static bool parse_manufacturer_data(DBusMessageIter *iter, > return true; > > fail: > - bt_ad_clear_manufacturer_data(client->data); > + bt_ad_clear_manufacturer_data(ad); > return false; > } > > +static bool parse_manufacturer_data_adv(DBusMessageIter *iter, > + struct btd_adv_client *client) > +{ > + return parse_manufacturer_data(iter, client->data); > +} > + > +static bool parse_manufacturer_data_scan(DBusMessageIter *iter, > + struct btd_adv_client *client) > +{ > + return parse_manufacturer_data(iter, client->scan); > +} > + > static bool parse_service_data(DBusMessageIter *iter, > struct btd_adv_client *client) > { > @@ -941,7 +953,8 @@ static struct adv_parser { > { "Type", parse_type }, > { "ServiceUUIDs", parse_service_uuids }, > { "SolicitUUIDs", parse_solicit_uuids }, > - { "ManufacturerData", parse_manufacturer_data }, > + { "ManufacturerData", parse_manufacturer_data_adv }, > + { "ManufacturerDataScanResponse", parse_manufacturer_data_scan }, > { "ServiceData", parse_service_data }, > { "Includes", parse_includes }, > { "LocalName", parse_local_name }, > -- > 2.7.4 >
Hi Luiz, > Don't really like to do this, beside it seems to be optional to enter either on AD or on SRD so the scanner is really at fault here if it only able to parse the data if placed on SRD, that said we could have some logic that detects if manufacturer don't fit on the AD push it to SRD if that has more space if the advertisement is discoverable. I agree on that, that may be the best solution. We already thought about that. However it looked like some major changes in shared part would be necessary for that and we didn't want to change too much here. So this was the easiest (although little dirty) solution for us to start with. We wanted to share it here and start discussion on that. What needs to be considered for changes to shared part? BR Max -----Original Message----- From: Luiz Augusto von Dentz <luiz.dentz@gmail.com> Sent: Friday, April 3, 2020 11:42 PM To: Klein, Thorsten (BSH) <kleinkastel@googlemail.com> Cc: linux-bluetooth@vger.kernel.org; Schachschal, Maximilian (GED-SDD2) <Maximilian.Schachschal@bshg.com> Subject: Re: [PATCH BlueZ] ManufacturerData field added to ScanResponse Hi Thorsten, On Fri, Apr 3, 2020 at 12:42 AM Klein, Thorsten (BSH) <kleinkastel@googlemail.com> wrote: > > From: "Schachschal, Maximilian (BSH)" > <maximilian.schachschal@bshg.com> > > Keys are the Manufacturer ID to associate with the data. > --- > doc/advertising-api.txt | 6 ++++++ > src/advertising.c | 25 +++++++++++++++++++------ > 2 files changed, 25 insertions(+), 6 deletions(-) > > diff --git a/doc/advertising-api.txt b/doc/advertising-api.txt index > b0565ea..14ccae2 100644 > --- a/doc/advertising-api.txt > +++ b/doc/advertising-api.txt > @@ -51,6 +51,12 @@ Properties string Type > the Advertising Data. Keys are the Manufacturer ID > to associate with the data. > > + dict ManufacturerDataScanResponse [Experimental] > + > + Manufactuer Data fields to include in > + the Scan Response. Keys are the Manufacturer ID > + to associate with the data. > + Don't really like to do this, beside it seems to be optional to enter either on AD or on SRD so the scanner is really at fault here if it only able to parse the data if placed on SRD, that said we could have some logic that detects if manufacturer don't fit on the AD push it to SRD if that has more space if the advertisement is discoverable. > array{string} SolicitUUIDs > > Array of UUIDs to include in "Service Solicitation" > diff --git a/src/advertising.c b/src/advertising.c index > 45ff19f..0e1a3f1 100644 > --- a/src/advertising.c > +++ b/src/advertising.c > @@ -328,12 +328,12 @@ fail: > } > > static bool parse_manufacturer_data(DBusMessageIter *iter, > - struct btd_adv_client *client) > + struct btd_ad *ad) > { > DBusMessageIter entries; > > if (!iter) { > - bt_ad_clear_manufacturer_data(client->data); > + bt_ad_clear_manufacturer_data(ad); > return true; > } > > @@ -342,7 +342,7 @@ static bool > parse_manufacturer_data(DBusMessageIter *iter, > > dbus_message_iter_recurse(iter, &entries); > > - bt_ad_clear_manufacturer_data(client->data); > + bt_ad_clear_manufacturer_data(ad); > > while (dbus_message_iter_get_arg_type(&entries) > == > DBUS_TYPE_DICT_ENTRY) { @@ -373,7 +373,7 @@ static bool > parse_manufacturer_data(DBusMessageIter *iter, > > DBG("Adding ManufacturerData for %04x", manuf_id); > > - if (!bt_ad_add_manufacturer_data(client->data, manuf_id, > + if (!bt_ad_add_manufacturer_data(ad, manuf_id, > manuf_data, len)) > goto fail; > > @@ -383,10 +383,22 @@ static bool parse_manufacturer_data(DBusMessageIter *iter, > return true; > > fail: > - bt_ad_clear_manufacturer_data(client->data); > + bt_ad_clear_manufacturer_data(ad); > return false; > } > > +static bool parse_manufacturer_data_adv(DBusMessageIter *iter, > + struct btd_adv_client *client) > +{ > + return parse_manufacturer_data(iter, client->data); } > + > +static bool parse_manufacturer_data_scan(DBusMessageIter *iter, > + struct btd_adv_client *client) > +{ > + return parse_manufacturer_data(iter, client->scan); } > + > static bool parse_service_data(DBusMessageIter *iter, > struct btd_adv_client *client) > { @@ -941,7 +953,8 @@ static struct adv_parser { > { "Type", parse_type }, > { "ServiceUUIDs", parse_service_uuids }, > { "SolicitUUIDs", parse_solicit_uuids }, > - { "ManufacturerData", parse_manufacturer_data }, > + { "ManufacturerData", parse_manufacturer_data_adv }, > + { "ManufacturerDataScanResponse", parse_manufacturer_data_scan > + }, > { "ServiceData", parse_service_data }, > { "Includes", parse_includes }, > { "LocalName", parse_local_name }, > -- > 2.7.4 > -- Luiz Augusto von Dentz
Hi Maximilian, On Mon, Apr 6, 2020 at 2:58 AM Schachschal, Maximilian (GED-SDD2) <Maximilian.Schachschal@bshg.com> wrote: > > Hi Luiz, > > > Don't really like to do this, beside it seems to be optional to enter either on AD or on SRD so the scanner is really at fault here if it only able to parse the data if placed on SRD, that said we could have some logic that detects if manufacturer don't fit on the AD push it to SRD if that has more space if the advertisement is discoverable. > > I agree on that, that may be the best solution. > We already thought about that. However it looked like some major changes in shared part would be necessary for that and we didn't want to change too much here. > So this was the easiest (although little dirty) solution for us to start with. We wanted to share it here and start discussion on that. > What needs to be considered for changes to shared part? Afaik we can just try to append to ad instance if that fails then proceed to add to sd instance after checking if the instance is discoverable, so this might not even need a change to bt_ad itself, but perhaps bt_ad don't actually indicate if a proper error for no space but we should be add a specific retur for that. > BR > Max > > -----Original Message----- > From: Luiz Augusto von Dentz <luiz.dentz@gmail.com> > Sent: Friday, April 3, 2020 11:42 PM > To: Klein, Thorsten (BSH) <kleinkastel@googlemail.com> > Cc: linux-bluetooth@vger.kernel.org; Schachschal, Maximilian (GED-SDD2) <Maximilian.Schachschal@bshg.com> > Subject: Re: [PATCH BlueZ] ManufacturerData field added to ScanResponse > > Hi Thorsten, > > On Fri, Apr 3, 2020 at 12:42 AM Klein, Thorsten (BSH) <kleinkastel@googlemail.com> wrote: > > > > From: "Schachschal, Maximilian (BSH)" > > <maximilian.schachschal@bshg.com> > > > > Keys are the Manufacturer ID to associate with the data. > > --- > > doc/advertising-api.txt | 6 ++++++ > > src/advertising.c | 25 +++++++++++++++++++------ > > 2 files changed, 25 insertions(+), 6 deletions(-) > > > > diff --git a/doc/advertising-api.txt b/doc/advertising-api.txt index > > b0565ea..14ccae2 100644 > > --- a/doc/advertising-api.txt > > +++ b/doc/advertising-api.txt > > @@ -51,6 +51,12 @@ Properties string Type > > the Advertising Data. Keys are the Manufacturer ID > > to associate with the data. > > > > + dict ManufacturerDataScanResponse [Experimental] > > + > > + Manufactuer Data fields to include in > > + the Scan Response. Keys are the Manufacturer ID > > + to associate with the data. > > + > > Don't really like to do this, beside it seems to be optional to enter either on AD or on SRD so the scanner is really at fault here if it only able to parse the data if placed on SRD, that said we could have some logic that detects if manufacturer don't fit on the AD push it to SRD if that has more space if the advertisement is discoverable. > > > array{string} SolicitUUIDs > > > > Array of UUIDs to include in "Service Solicitation" > > diff --git a/src/advertising.c b/src/advertising.c index > > 45ff19f..0e1a3f1 100644 > > --- a/src/advertising.c > > +++ b/src/advertising.c > > @@ -328,12 +328,12 @@ fail: > > } > > > > static bool parse_manufacturer_data(DBusMessageIter *iter, > > - struct btd_adv_client *client) > > + struct btd_ad *ad) > > { > > DBusMessageIter entries; > > > > if (!iter) { > > - bt_ad_clear_manufacturer_data(client->data); > > + bt_ad_clear_manufacturer_data(ad); > > return true; > > } > > > > @@ -342,7 +342,7 @@ static bool > > parse_manufacturer_data(DBusMessageIter *iter, > > > > dbus_message_iter_recurse(iter, &entries); > > > > - bt_ad_clear_manufacturer_data(client->data); > > + bt_ad_clear_manufacturer_data(ad); > > > > while (dbus_message_iter_get_arg_type(&entries) > > == > > DBUS_TYPE_DICT_ENTRY) { @@ -373,7 +373,7 @@ static bool > > parse_manufacturer_data(DBusMessageIter *iter, > > > > DBG("Adding ManufacturerData for %04x", manuf_id); > > > > - if (!bt_ad_add_manufacturer_data(client->data, manuf_id, > > + if (!bt_ad_add_manufacturer_data(ad, manuf_id, > > manuf_data, len)) > > goto fail; > > > > @@ -383,10 +383,22 @@ static bool parse_manufacturer_data(DBusMessageIter *iter, > > return true; > > > > fail: > > - bt_ad_clear_manufacturer_data(client->data); > > + bt_ad_clear_manufacturer_data(ad); > > return false; > > } > > > > +static bool parse_manufacturer_data_adv(DBusMessageIter *iter, > > + struct btd_adv_client *client) > > +{ > > + return parse_manufacturer_data(iter, client->data); } > > + > > +static bool parse_manufacturer_data_scan(DBusMessageIter *iter, > > + struct btd_adv_client *client) > > +{ > > + return parse_manufacturer_data(iter, client->scan); } > > + > > static bool parse_service_data(DBusMessageIter *iter, > > struct btd_adv_client *client) > > { @@ -941,7 +953,8 @@ static struct adv_parser { > > { "Type", parse_type }, > > { "ServiceUUIDs", parse_service_uuids }, > > { "SolicitUUIDs", parse_solicit_uuids }, > > - { "ManufacturerData", parse_manufacturer_data }, > > + { "ManufacturerData", parse_manufacturer_data_adv }, > > + { "ManufacturerDataScanResponse", parse_manufacturer_data_scan > > + }, > > { "ServiceData", parse_service_data }, > > { "Includes", parse_includes }, > > { "LocalName", parse_local_name }, > > -- > > 2.7.4 > > > > > -- > Luiz Augusto von Dentz
Hi Luiz, > Afaik we can just try to append to ad instance if that fails then proceed to > add to sd instance after checking if the instance is discoverable, so this might > not even need a change to bt_ad itself, but perhaps bt_ad don't actually > indicate if a proper error for no space but we should be add a specific retur > for that. But where to store the data then? Is there a way to move it from client->data to client->scan? BR, Max
diff --git a/doc/advertising-api.txt b/doc/advertising-api.txt index b0565ea..14ccae2 100644 --- a/doc/advertising-api.txt +++ b/doc/advertising-api.txt @@ -51,6 +51,12 @@ Properties string Type the Advertising Data. Keys are the Manufacturer ID to associate with the data. + dict ManufacturerDataScanResponse [Experimental] + + Manufactuer Data fields to include in + the Scan Response. Keys are the Manufacturer ID + to associate with the data. + array{string} SolicitUUIDs Array of UUIDs to include in "Service Solicitation" diff --git a/src/advertising.c b/src/advertising.c index 45ff19f..0e1a3f1 100644 --- a/src/advertising.c +++ b/src/advertising.c @@ -328,12 +328,12 @@ fail: } static bool parse_manufacturer_data(DBusMessageIter *iter, - struct btd_adv_client *client) + struct btd_ad *ad) { DBusMessageIter entries; if (!iter) { - bt_ad_clear_manufacturer_data(client->data); + bt_ad_clear_manufacturer_data(ad); return true; } @@ -342,7 +342,7 @@ static bool parse_manufacturer_data(DBusMessageIter *iter, dbus_message_iter_recurse(iter, &entries); - bt_ad_clear_manufacturer_data(client->data); + bt_ad_clear_manufacturer_data(ad); while (dbus_message_iter_get_arg_type(&entries) == DBUS_TYPE_DICT_ENTRY) { @@ -373,7 +373,7 @@ static bool parse_manufacturer_data(DBusMessageIter *iter, DBG("Adding ManufacturerData for %04x", manuf_id); - if (!bt_ad_add_manufacturer_data(client->data, manuf_id, + if (!bt_ad_add_manufacturer_data(ad, manuf_id, manuf_data, len)) goto fail; @@ -383,10 +383,22 @@ static bool parse_manufacturer_data(DBusMessageIter *iter, return true; fail: - bt_ad_clear_manufacturer_data(client->data); + bt_ad_clear_manufacturer_data(ad); return false; } +static bool parse_manufacturer_data_adv(DBusMessageIter *iter, + struct btd_adv_client *client) +{ + return parse_manufacturer_data(iter, client->data); +} + +static bool parse_manufacturer_data_scan(DBusMessageIter *iter, + struct btd_adv_client *client) +{ + return parse_manufacturer_data(iter, client->scan); +} + static bool parse_service_data(DBusMessageIter *iter, struct btd_adv_client *client) { @@ -941,7 +953,8 @@ static struct adv_parser { { "Type", parse_type }, { "ServiceUUIDs", parse_service_uuids }, { "SolicitUUIDs", parse_solicit_uuids }, - { "ManufacturerData", parse_manufacturer_data }, + { "ManufacturerData", parse_manufacturer_data_adv }, + { "ManufacturerDataScanResponse", parse_manufacturer_data_scan }, { "ServiceData", parse_service_data }, { "Includes", parse_includes }, { "LocalName", parse_local_name },
From: "Schachschal, Maximilian (BSH)" <maximilian.schachschal@bshg.com> Keys are the Manufacturer ID to associate with the data. --- doc/advertising-api.txt | 6 ++++++ src/advertising.c | 25 +++++++++++++++++++------ 2 files changed, 25 insertions(+), 6 deletions(-)