Hi Daniel,
On Wed, Mar 3, 2021 at 11:20 AM Daniel Winkler <[email protected]> wrote:
>
> In order for the advertising parameters hci request to indicate that an
> advertising set uses a scannable PDU, we pass a scannable flag along
> with the initial parameters MGMT request.
>
> Without this patch, a broadcast advertisement with a scan response will
> either be rejected by the controller, or will ignore the requested scan
> response. The patch is tested by performing the above and confirming
> that the scan response is retrievable from a peer as expected.
>
> Reviewed-by: Alain Michaud <[email protected]>
> Reviewed-by: Sonny Sasaka <[email protected]>
>
> ---
>
> lib/mgmt.h | 1 +
> src/advertising.c | 4 ++++
> 2 files changed, 5 insertions(+)
>
> diff --git a/lib/mgmt.h b/lib/mgmt.h
> index 76a03c9c2..7b1b9ab54 100644
> --- a/lib/mgmt.h
> +++ b/lib/mgmt.h
> @@ -507,6 +507,7 @@ struct mgmt_rp_add_advertising {
> #define MGMT_ADV_PARAM_TIMEOUT (1 << 13)
> #define MGMT_ADV_PARAM_INTERVALS (1 << 14)
> #define MGMT_ADV_PARAM_TX_POWER (1 << 15)
> +#define MGMT_ADV_PARAM_SCAN_RSP (1 << 16)
>
> #define MGMT_OP_REMOVE_ADVERTISING 0x003F
> struct mgmt_cp_remove_advertising {
> diff --git a/src/advertising.c b/src/advertising.c
> index f3dc357a1..38cef565f 100644
> --- a/src/advertising.c
> +++ b/src/advertising.c
> @@ -945,6 +945,10 @@ static int refresh_extended_adv(struct btd_adv_client *client,
> return -EINVAL;
> }
>
> + /* Indicate that this instance will be configured as scannable */
> + if (client->scan_rsp_len)
> + flags |= MGMT_ADV_PARAM_SCAN_RSP;
> +
Don't we need to check if the flag is actually supported by the kernel?
> cp.flags = htobl(flags);
For new code it is prefered to use the function from src/shared/util.h
(cpu_to_*).
> mgmt_ret = mgmt_send(client->manager->mgmt, MGMT_OP_ADD_EXT_ADV_PARAMS,
> --
> 2.30.1.766.gb4fecdf3b7-goog
>
--
Luiz Augusto von Dentz
Hello Luiz,
Thank you for the catch and suggestion. I have just sent out a v2 to
address your recommendations.
Thanks!
Daniel
On Thu, Mar 4, 2021 at 10:59 AM Luiz Augusto von Dentz
<[email protected]> wrote:
>
> Hi Daniel,
>
> On Wed, Mar 3, 2021 at 11:20 AM Daniel Winkler <[email protected]> wrote:
> >
> > In order for the advertising parameters hci request to indicate that an
> > advertising set uses a scannable PDU, we pass a scannable flag along
> > with the initial parameters MGMT request.
> >
> > Without this patch, a broadcast advertisement with a scan response will
> > either be rejected by the controller, or will ignore the requested scan
> > response. The patch is tested by performing the above and confirming
> > that the scan response is retrievable from a peer as expected.
> >
> > Reviewed-by: Alain Michaud <[email protected]>
> > Reviewed-by: Sonny Sasaka <[email protected]>
> >
> > ---
> >
> > lib/mgmt.h | 1 +
> > src/advertising.c | 4 ++++
> > 2 files changed, 5 insertions(+)
> >
> > diff --git a/lib/mgmt.h b/lib/mgmt.h
> > index 76a03c9c2..7b1b9ab54 100644
> > --- a/lib/mgmt.h
> > +++ b/lib/mgmt.h
> > @@ -507,6 +507,7 @@ struct mgmt_rp_add_advertising {
> > #define MGMT_ADV_PARAM_TIMEOUT (1 << 13)
> > #define MGMT_ADV_PARAM_INTERVALS (1 << 14)
> > #define MGMT_ADV_PARAM_TX_POWER (1 << 15)
> > +#define MGMT_ADV_PARAM_SCAN_RSP (1 << 16)
> >
> > #define MGMT_OP_REMOVE_ADVERTISING 0x003F
> > struct mgmt_cp_remove_advertising {
> > diff --git a/src/advertising.c b/src/advertising.c
> > index f3dc357a1..38cef565f 100644
> > --- a/src/advertising.c
> > +++ b/src/advertising.c
> > @@ -945,6 +945,10 @@ static int refresh_extended_adv(struct btd_adv_client *client,
> > return -EINVAL;
> > }
> >
> > + /* Indicate that this instance will be configured as scannable */
> > + if (client->scan_rsp_len)
> > + flags |= MGMT_ADV_PARAM_SCAN_RSP;
> > +
>
> Don't we need to check if the flag is actually supported by the kernel?
>
> > cp.flags = htobl(flags);
>
> For new code it is prefered to use the function from src/shared/util.h
> (cpu_to_*).
>
> > mgmt_ret = mgmt_send(client->manager->mgmt, MGMT_OP_ADD_EXT_ADV_PARAMS,
> > --
> > 2.30.1.766.gb4fecdf3b7-goog
> >
>
>
> --
> Luiz Augusto von Dentz