Hi,
On Mon, Jul 1, 2024 at 7:03 AM Prathibha Madugonde
<quic_prathm@xxxxxxxxxxx> wrote:
>
>
>
> On 6/28/2024 7:08 PM, Luiz Augusto von Dentz wrote:
> > Hi,
> >
> > On Fri, Jun 28, 2024 at 3:24 AM <quic_prathm@xxxxxxxxxxx> wrote:
> >>
> >> From: Prathibha Madugonde <quic_prathm@xxxxxxxxxxx>
> >>
> >> Fix for GAP/DISC/NONM/BV-02-C
> >> As per GAP.TS.p44 test spec
> >> IUT does not contain General Discoverable mode and Limited Discoverable
> >> mode in the AD Type Flag. IUT shall send AD Type Flag to PASS the test
> >> case, thus added BR/EDR not supported bit in the AD Type Flag when
> >> discoverable is off.
> >>
> >> Signed-off-by: Prathibha Madugonde <quic_prathm@xxxxxxxxxxx>
> >> ---
> >> src/advertising.c | 16 ++++++++++++++++
> >> 1 file changed, 16 insertions(+)
> >>
> >> diff --git a/src/advertising.c b/src/advertising.c
> >> index 5d373e088..9857ceceb 100644
> >> --- a/src/advertising.c
> >> +++ b/src/advertising.c
> >> @@ -1444,6 +1444,7 @@ static DBusMessage *parse_advertisement(struct btd_adv_client *client)
> >> {
> >> struct adv_parser *parser;
> >> int err;
> >> + uint8_t flags;
> >>
> >> for (parser = parsers; parser && parser->name; parser++) {
> >> DBusMessageIter iter;
> >> @@ -1499,6 +1500,21 @@ static DBusMessage *parse_advertisement(struct btd_adv_client *client)
> >> goto fail;
> >> }
> >>
> >> + if (!btd_adapter_get_discoverable(client->manager->adapter)) {
> >> + /* GAP.TS.p44 Test Spec GAP/DISC/NONM/BV-02-C
> >> + * page 158:
> >> + * IUT does not contain
> >> + * ‘LE General Discoverable Mode’ flag or the
> >> + * ‘LE Limited Discoverable Mode’ flag in the Flags AD Type
> >> + * But AD Flag Type should be there for the test case to
> >> + * PASS. Thus BR/EDR Not Supported BIT shall be included
> >> + * in the AD Type flag.
> >> + */
> >> + flags = bt_ad_get_flags(client->data);
> >> + flags |= BT_AD_FLAG_NO_BREDR;
> >> + bt_ad_add_flags(client->data, &flags, 1);
> >> + }
> >
> > I think we would be much better off using broadcaster role for such a
> > test case or does it require to be connectable? Anyway I don't think
> > there is a requirement to disable BR/EDR when not discoverable, so if
> > we really need to pass specific flags then perhaps it would be better
> > to create a Flags property so clients can set themselves.
> >
> Hi,
> This particular test case require IUT to be in connectable. There is
> already code snippet to disable BR/EDR when adapter is not discoverable
> in the set_flags() like below.
> /* Set BR/EDR Not Supported if adapter is not discoverable but
> * the instance is.
> */
> if ((flags & (BT_AD_FLAG_GENERAL | BT_AD_FLAG_LIMITED)) &&
> !btd_adapter_get_discoverable(client->manager->adapter))
> flags |= BT_AD_FLAG_NO_BREDR;
>
> Hence using the same logic. Currently AD flags(BT_AD_FLAG_LIMITED,
> BT_AD_FLAG_GENERAL & BT_AD_FLAG_NO_BREDR) is managed based on properties
> discoverable, discoverable timeout and adapter discoverable.
Oh, in that case why didn't you change that statement? Anyway, the PTS
requiring the use of flags is rather unconventional here but I think
it should be fine not marking BR/EDR support if it is not
discoverable.
> --
> Prathibha Madugonde
>
>
> >> err = refresh_advertisement(client, add_adv_callback);
> >>
> >> if (!err)
> >> --
> >> 2.17.1
> >>
> >
> >
--
Luiz Augusto von Dentz
