diff mbox series

[BlueZ] advertising: Fix reporting advertising properties

Message ID 20211215180527.886481-1-claudio.takahasi@gmail.com
State New
Headers show
Series [BlueZ] advertising: Fix reporting advertising properties | expand

Commit Message

Claudio Takahasi Dec. 15, 2021, 6:05 p.m. UTC
InterfacesAdded signal for LEAdvertisingManager1 might be emitted
containing initial/default properties values and property changed is
not emitted after reading advertising features. This patch registers
the interface (LEAdvertisingManager1) after reading advertising features
from kernel.
---
 src/advertising.c | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

Comments

Marijn Suijten Feb. 3, 2022, 11:04 p.m. UTC | #1
Hi Claudio, Luiz,

On 2021-12-15 14:53:04, Luiz Augusto von Dentz wrote:
> Hi Claudio,
> 
> On Wed, Dec 15, 2021 at 12:50 PM <bluez.test.bot@gmail.com> 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=596151
> >
> > ---Test result---
> >
> > Test Summary:
> > CheckPatch                    PASS      1.47 seconds
> > GitLint                       PASS      0.99 seconds
> > Prep - Setup ELL              PASS      49.04 seconds
> > Build - Prep                  PASS      0.61 seconds
> > Build - Configure             PASS      9.40 seconds
> > Build - Make                  PASS      209.53 seconds
> > Make Check                    PASS      9.37 seconds
> > Make Distcheck                PASS      248.72 seconds
> > Build w/ext ELL - Configure   PASS      9.46 seconds
> > Build w/ext ELL - Make        PASS      198.40 seconds
> > Incremental Build with patchesPASS      0.00 seconds
> >
> >
> >
> > ---
> > Regards,
> > Linux Bluetooth
> 
> Applied, thanks.

This seems to at least partially back out of:

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

And as such, reintroduce the bug it fixed:

https://bugzilla.redhat.com/show_bug.cgi?id=1534857
https://bugs.archlinux.org/task/57464

Me and two others in the pulseaudio IRC channel see this connection
failure appear, and I've locally confirmed reverting this patch resolves
the issue yet again.

- Marijn
Luiz Augusto von Dentz Feb. 4, 2022, 1:06 a.m. UTC | #2
Hi Marijn.

On Thu, Feb 3, 2022 at 3:04 PM Marijn Suijten
<marijn.suijten@somainline.org> wrote:
>
> Hi Claudio, Luiz,
>
> On 2021-12-15 14:53:04, Luiz Augusto von Dentz wrote:
> > Hi Claudio,
> >
> > On Wed, Dec 15, 2021 at 12:50 PM <bluez.test.bot@gmail.com> 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=596151
> > >
> > > ---Test result---
> > >
> > > Test Summary:
> > > CheckPatch                    PASS      1.47 seconds
> > > GitLint                       PASS      0.99 seconds
> > > Prep - Setup ELL              PASS      49.04 seconds
> > > Build - Prep                  PASS      0.61 seconds
> > > Build - Configure             PASS      9.40 seconds
> > > Build - Make                  PASS      209.53 seconds
> > > Make Check                    PASS      9.37 seconds
> > > Make Distcheck                PASS      248.72 seconds
> > > Build w/ext ELL - Configure   PASS      9.46 seconds
> > > Build w/ext ELL - Make        PASS      198.40 seconds
> > > Incremental Build with patchesPASS      0.00 seconds
> > >
> > >
> > >
> > > ---
> > > Regards,
> > > Linux Bluetooth
> >
> > Applied, thanks.
>
> This seems to at least partially back out of:
>
> https://git.kernel.org/pub/scm/bluetooth/bluez.git/commit/?id=d36c45c55
>
> And as such, reintroduce the bug it fixed:
>
> https://bugzilla.redhat.com/show_bug.cgi?id=1534857
> https://bugs.archlinux.org/task/57464
>
> Me and two others in the pulseaudio IRC channel see this connection
> failure appear, and I've locally confirmed reverting this patch resolves
> the issue yet again.

Hmm so that is the actual problem that PA can't find the adapter, this
has been popping up quite a lot in our github issues, so I guess we
will need to revert it and update the properties after reading the
features, can either of you come with a patch for that?
Luiz Augusto von Dentz Feb. 4, 2022, 1:43 a.m. UTC | #3
Hi,

On Thu, Feb 3, 2022 at 5:06 PM Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
>
> Hi Marijn.
>
> On Thu, Feb 3, 2022 at 3:04 PM Marijn Suijten
> <marijn.suijten@somainline.org> wrote:
> >
> > Hi Claudio, Luiz,
> >
> > On 2021-12-15 14:53:04, Luiz Augusto von Dentz wrote:
> > > Hi Claudio,
> > >
> > > On Wed, Dec 15, 2021 at 12:50 PM <bluez.test.bot@gmail.com> 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=596151
> > > >
> > > > ---Test result---
> > > >
> > > > Test Summary:
> > > > CheckPatch                    PASS      1.47 seconds
> > > > GitLint                       PASS      0.99 seconds
> > > > Prep - Setup ELL              PASS      49.04 seconds
> > > > Build - Prep                  PASS      0.61 seconds
> > > > Build - Configure             PASS      9.40 seconds
> > > > Build - Make                  PASS      209.53 seconds
> > > > Make Check                    PASS      9.37 seconds
> > > > Make Distcheck                PASS      248.72 seconds
> > > > Build w/ext ELL - Configure   PASS      9.46 seconds
> > > > Build w/ext ELL - Make        PASS      198.40 seconds
> > > > Incremental Build with patchesPASS      0.00 seconds
> > > >
> > > >
> > > >
> > > > ---
> > > > Regards,
> > > > Linux Bluetooth
> > >
> > > Applied, thanks.
> >
> > This seems to at least partially back out of:
> >
> > https://git.kernel.org/pub/scm/bluetooth/bluez.git/commit/?id=d36c45c55
> >
> > And as such, reintroduce the bug it fixed:
> >
> > https://bugzilla.redhat.com/show_bug.cgi?id=1534857
> > https://bugs.archlinux.org/task/57464
> >
> > Me and two others in the pulseaudio IRC channel see this connection
> > failure appear, and I've locally confirmed reverting this patch resolves
> > the issue yet again.
>
> Hmm so that is the actual problem that PA can't find the adapter, this
> has been popping up quite a lot in our github issues, so I guess we
> will need to revert it and update the properties after reading the
> features, can either of you come with a patch for that?

I went ahead and send the following patch, hopefully we won't have to
worry about this sort of problem with that:

https://patchwork.kernel.org/project/bluetooth/patch/20220204013620.2465024-1-luiz.dentz@gmail.com/
Marijn Suijten Feb. 4, 2022, 3:41 p.m. UTC | #4
On 2022-02-03 17:43:16, Luiz Augusto von Dentz wrote:
> Hi,
> 
> On Thu, Feb 3, 2022 at 5:06 PM Luiz Augusto von Dentz
> <luiz.dentz@gmail.com> wrote:
> >
> > Hi Marijn.
> >
> > On Thu, Feb 3, 2022 at 3:04 PM Marijn Suijten
> > <marijn.suijten@somainline.org> wrote:
> > >
> > > Hi Claudio, Luiz,
> > >
> > > On 2021-12-15 14:53:04, Luiz Augusto von Dentz wrote:
> > > > Hi Claudio,
> > > >
> > > > On Wed, Dec 15, 2021 at 12:50 PM <bluez.test.bot@gmail.com> 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=596151
> > > > >
> > > > > ---Test result---
> > > > >
> > > > > Test Summary:
> > > > > CheckPatch                    PASS      1.47 seconds
> > > > > GitLint                       PASS      0.99 seconds
> > > > > Prep - Setup ELL              PASS      49.04 seconds
> > > > > Build - Prep                  PASS      0.61 seconds
> > > > > Build - Configure             PASS      9.40 seconds
> > > > > Build - Make                  PASS      209.53 seconds
> > > > > Make Check                    PASS      9.37 seconds
> > > > > Make Distcheck                PASS      248.72 seconds
> > > > > Build w/ext ELL - Configure   PASS      9.46 seconds
> > > > > Build w/ext ELL - Make        PASS      198.40 seconds
> > > > > Incremental Build with patchesPASS      0.00 seconds
> > > > >
> > > > >
> > > > >
> > > > > ---
> > > > > Regards,
> > > > > Linux Bluetooth
> > > >
> > > > Applied, thanks.
> > >
> > > This seems to at least partially back out of:
> > >
> > > https://git.kernel.org/pub/scm/bluetooth/bluez.git/commit/?id=d36c45c55
> > >
> > > And as such, reintroduce the bug it fixed:
> > >
> > > https://bugzilla.redhat.com/show_bug.cgi?id=1534857
> > > https://bugs.archlinux.org/task/57464
> > >
> > > Me and two others in the pulseaudio IRC channel see this connection
> > > failure appear, and I've locally confirmed reverting this patch resolves
> > > the issue yet again.
> >
> > Hmm so that is the actual problem that PA can't find the adapter, this
> > has been popping up quite a lot in our github issues, so I guess we
> > will need to revert it and update the properties after reading the
> > features, can either of you come with a patch for that?

Perhaps yes, I haven't had more time to look into this than seeing the
original patch and spotting this patch to "advertising" recently while
bisecting 5.62 and 5.63.  Looks like you came up with a neat solution,
would have taken me some time to get accustomed to this bit of BlueZ :)

> 
> I went ahead and send the following patch, hopefully we won't have to
> worry about this sort of problem with that:
> 
> https://patchwork.kernel.org/project/bluetooth/patch/20220204013620.2465024-1-luiz.dentz@gmail.com/

Many thanks, that addresses the issue once again!

- Marijn

> -- 
> Luiz Augusto von Dentz
diff mbox series

Patch

diff --git a/src/advertising.c b/src/advertising.c
index 41b818650..2110f17c9 100644
--- a/src/advertising.c
+++ b/src/advertising.c
@@ -1786,6 +1786,13 @@  static void read_adv_features_callback(uint8_t status, uint16_t length,
 	manager->max_ads = feat->max_instances;
 	manager->supported_flags |= feat->supported_flags;
 
+	/* Registering interface after querying properties */
+	if (!g_dbus_register_interface(btd_get_dbus_connection(),
+				       adapter_get_path(manager->adapter),
+				       LE_ADVERTISING_MGR_IFACE, methods,
+				       NULL, properties, manager, NULL))
+		error("Failed to register " LE_ADVERTISING_MGR_IFACE);
+
 	if (manager->max_ads == 0)
 		return;
 
@@ -1861,14 +1868,6 @@  static struct btd_adv_manager *manager_create(struct btd_adapter *adapter,
 	manager->min_tx_power = ADV_TX_POWER_NO_PREFERENCE;
 	manager->max_tx_power = ADV_TX_POWER_NO_PREFERENCE;
 
-	if (!g_dbus_register_interface(btd_get_dbus_connection(),
-					adapter_get_path(manager->adapter),
-					LE_ADVERTISING_MGR_IFACE, methods,
-					NULL, properties, manager, NULL)) {
-		error("Failed to register " LE_ADVERTISING_MGR_IFACE);
-		goto fail;
-	}
-
 	if (!mgmt_send(manager->mgmt, MGMT_OP_READ_ADV_FEATURES,
 				manager->mgmt_index, 0, NULL,
 				read_adv_features_callback, manager, NULL)) {