[BlueZ] main.conf: use correct key for BREDR configuration

Commit Message

Ronan Pigott Nov. 24, 2020, 10:46 p.m. UTC

From: Ronan Pigott <[e-mail]>

Signed-off-by: Ronan Pigott <[e-mail]>
---

There is a mismtach between the "BREDR" group name in the config
and the valid group name "BR" accepted in main.c. This changes
main.conf, but I'm not certain if it's main.c that should be changed
instead.

 src/main.conf | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

[e-mail] Nov. 24, 2020, 11:28 p.m. UTC | #1

This is automated email and please do not reply to this email!

Dear submitter,

Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=390539

---Test result---

##############################
Test: CheckPatch - PASS

##############################
Test: CheckGitLint - PASS

##############################
Test: CheckBuild - PASS

##############################
Test: MakeCheck - PASS



---
Regards,
Linux Bluetooth

Luiz Augusto von Dentz Nov. 25, 2020, 1:19 a.m. UTC | #2

Hi Ronan,

On Tue, Nov 24, 2020 at 4:07 PM <[e-mail]> wrote:
>
> This is automated email and please do not reply to this email!
>
> Dear submitter,
>
> Thank you for submitting the patches to the linux bluetooth mailing list.
> This is a CI test results with your patch series:
> PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=390539
>
> ---Test result---
>
> ##############################
> Test: CheckPatch - PASS
>
> ##############################
> Test: CheckGitLint - PASS
>
> ##############################
> Test: CheckBuild - PASS
>
> ##############################
> Test: MakeCheck - PASS
>
>
>
> ---
> Regards,
> Linux Bluetooth

Applied, thanks.

Alain Michaud Dec. 1, 2020, 8:33 p.m. UTC | #3

Hi Luiz/Ronan,

This appears to have been an incorrect fix since
parse_mode_config(config, "BREDR", params, ARRAY_SIZE(params)); will
attempt to read from the BREDR section.  My suggestion would be to
update the group table entry instead:

static const struct group_table {
const char *name;
const char **options;
} valid_groups[] = {
{ "General", supported_options },
{ "BREDR", br_options },               //<------
{ "LE", le_options },
{ "Policy", policy_options },
{ "GATT", gatt_options },
{ "AVDTP", avdtp_options },
{ }
};

Thanks,
Alain


On Tue, Nov 24, 2020 at 8:21 PM Luiz Augusto von Dentz
<[e-mail]> wrote:
>
> Hi Ronan,
>
> On Tue, Nov 24, 2020 at 4:07 PM <[e-mail]> wrote:
> >
> > This is automated email and please do not reply to this email!
> >
> > Dear submitter,
> >
> > Thank you for submitting the patches to the linux bluetooth mailing list.
> > This is a CI test results with your patch series:
> > PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=390539
> >
> > ---Test result---
> >
> > ##############################
> > Test: CheckPatch - PASS
> >
> > ##############################
> > Test: CheckGitLint - PASS
> >
> > ##############################
> > Test: CheckBuild - PASS
> >
> > ##############################
> > Test: MakeCheck - PASS
> >
> >
> >
> > ---
> > Regards,
> > Linux Bluetooth
>
> Applied, thanks.
>
>
> --
> Luiz Augusto von Dentz

Ronan Pigott Dec. 1, 2020, 10:51 p.m. UTC | #4

On Tue Dec 1, 2020 at 8:33 AM MST, Alain Michaud wrote:
> Hi Luiz/Ronan,
>
> This appears to have been an incorrect fix since
> parse_mode_config(config, "BREDR", params, ARRAY_SIZE(params)); will
> attempt to read from the BREDR section. My suggestion would be to
> update the group table entry instead:

Oh, that's right. Whoops.

Updating the group table sounds good to me.

Alain Michaud Dec. 1, 2020, 11:03 p.m. UTC | #5

I likely won't get to it for a little while, but if someone will be
fixing this, we also noticed this issue while reviewing the related
patch:

{ "PageTimeout",
&btd_opts.defaults.br.page_timeout,
sizeof(btd_opts.defaults.br.page_scan_win), //<-- this should also be
page_timeout rather than page_scan_win
0x0001,
0xFFFF},

Thanks!
Alain

On Tue, Dec 1, 2020 at 5:53 PM Ronan Pigott <[e-mail]> wrote:
>
> On Tue Dec 1, 2020 at 8:33 AM MST, Alain Michaud wrote:
> > Hi Luiz/Ronan,
> >
> > This appears to have been an incorrect fix since
> > parse_mode_config(config, "BREDR", params, ARRAY_SIZE(params)); will
> > attempt to read from the BREDR section. My suggestion would be to
> > update the group table entry instead:
>
> Oh, that's right. Whoops.
>
> Updating the group table sounds good to me.

Luiz Augusto von Dentz Dec. 1, 2020, 11:13 p.m. UTC | #6

Hi Alain,

On Tue, Dec 1, 2020 at 3:03 PM Alain Michaud <[e-mail]> wrote:
>
> I likely won't get to it for a little while, but if someone will be
> fixing this, we also noticed this issue while reviewing the related
> patch:
>
> { "PageTimeout",
> &btd_opts.defaults.br.page_timeout,
> sizeof(btd_opts.defaults.br.page_scan_win), //<-- this should also be
> page_timeout rather than page_scan_win
> 0x0001,
> 0xFFFF},

Nice catch:

https://git.kernel.org/pub/scm/bluetooth/bluez.git/commit/?id=a37d53db9ae7d21a8f812925303d767d3f03e597

> Thanks!
> Alain
>
> On Tue, Dec 1, 2020 at 5:53 PM Ronan Pigott <[e-mail]> wrote:
> >
> > On Tue Dec 1, 2020 at 8:33 AM MST, Alain Michaud wrote:
> > > Hi Luiz/Ronan,
> > >
> > > This appears to have been an incorrect fix since
> > > parse_mode_config(config, "BREDR", params, ARRAY_SIZE(params)); will
> > > attempt to read from the BREDR section. My suggestion would be to
> > > update the group table entry instead:
> >
> > Oh, that's right. Whoops.
> >
> > Updating the group table sounds good to me.

https://git.kernel.org/pub/scm/bluetooth/bluez.git/commit/?id=e2863c003c8e65b386a981ef6037518beb605795

So now everything should be using BR as group name.

Alain Michaud Dec. 1, 2020, 11:16 p.m. UTC | #7

Thanks for the quick turnaround Luiz.  Converging onto BR instead of
BREDR also works.

On Tue, Dec 1, 2020 at 6:13 PM Luiz Augusto von Dentz
<[e-mail]> wrote:
>
> Hi Alain,
>
> On Tue, Dec 1, 2020 at 3:03 PM Alain Michaud <[e-mail]> wrote:
> >
> > I likely won't get to it for a little while, but if someone will be
> > fixing this, we also noticed this issue while reviewing the related
> > patch:
> >
> > { "PageTimeout",
> > &btd_opts.defaults.br.page_timeout,
> > sizeof(btd_opts.defaults.br.page_scan_win), //<-- this should also be
> > page_timeout rather than page_scan_win
> > 0x0001,
> > 0xFFFF},
>
> Nice catch:
>
> https://git.kernel.org/pub/scm/bluetooth/bluez.git/commit/?id=a37d53db9ae7d21a8f812925303d767d3f03e597
>
> > Thanks!
> > Alain
> >
> > On Tue, Dec 1, 2020 at 5:53 PM Ronan Pigott <[e-mail]> wrote:
> > >
> > > On Tue Dec 1, 2020 at 8:33 AM MST, Alain Michaud wrote:
> > > > Hi Luiz/Ronan,
> > > >
> > > > This appears to have been an incorrect fix since
> > > > parse_mode_config(config, "BREDR", params, ARRAY_SIZE(params)); will
> > > > attempt to read from the BREDR section. My suggestion would be to
> > > > update the group table entry instead:
> > >
> > > Oh, that's right. Whoops.
> > >
> > > Updating the group table sounds good to me.
>
> https://git.kernel.org/pub/scm/bluetooth/bluez.git/commit/?id=e2863c003c8e65b386a981ef6037518beb605795
>
> So now everything should be using BR as group name.
>
>
> --
> Luiz Augusto von Dentz

Patch

diff --git a/src/main.conf b/src/main.conf
index d3bc61441..ad36638b7 100644
--- a/src/main.conf
+++ b/src/main.conf
@@ -86,7 +86,7 @@ 
 # profile is connected. Defaults to true.
 #RefreshDiscovery = true
 
-[BREDR]
+[BR]
 # The following values are used to load default adapter parameters for BR/EDR.
 # BlueZ loads the values into the kernel before the adapter is powered if the
 # kernel supports the MGMT_LOAD_DEFAULT_PARAMETERS command. If a value isn't