[PATCH BlueZ 0/3] Fix wake_allowed reported error and not being set after pairing

[PATCH BlueZ 0/3] Fix wake_allowed reported error and not being set after pairing

[Ludovico de Nittis]

When a new HID or HOG device is in range, Bluez reports the following error:
```
src/device.c:set_wake_allowed_complete() Set device flags return status:
Invalid Parameters
```

Also, a side effect of that issue, when pairing a HID or HOG device, the
wake_allowed property doesn't get enabled by default, as expected.
It requires a Bluez restart in order to get that property correctly set.

This patch series addresses those issues.

Ludovico de Nittis (3):
  adapter: Preserve pending flags when setting the Device Privacy Mode
  device: Preserve pending flags when setting the wake allowed
  device: Try to set the wake_allowed property only for bonded devices

 src/adapter.c |  5 ++++-
 src/device.c  | 19 +++++++++++++++----
 2 files changed, 19 insertions(+), 5 deletions(-)

-- 
2.49.0

[PATCH BlueZ 1/3] adapter: Preserve pending flags when setting the Device Privacy Mode

[Ludovico de Nittis]

If there are already flags that are pending to be applied, we should
keep them to avoid overwriting them.
At that point we only want to add the Device Privacy Mode on top of the
existing flags.
---
 src/adapter.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/src/adapter.c b/src/adapter.c
index 5d4117a49..d4e42eed8 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -5630,8 +5630,11 @@ static void add_device_complete(uint8_t status, uint16_t length,
 	if (btd_opts.device_privacy) {
 		uint32_t flags = btd_device_get_current_flags(dev);
 
-		/* Set Device Privacy Mode has not set the flag yet. */
+		/* Set Device Privacy Mode if it has not set the flag yet. */
 		if (!(flags & DEVICE_FLAG_DEVICE_PRIVACY)) {
+			/* Include the pending flags, or they may get overwritten. */
+			flags |= btd_device_get_pending_flags(dev);
+
 			adapter_set_device_flags(adapter, dev, flags |
 						DEVICE_FLAG_DEVICE_PRIVACY,
 						set_device_privacy_complete,
-- 
2.49.0

RE: Fix wake_allowed reported error and not being set after pairing

[bluez.test.bot]

[-- Attachment #1: Type: text/plain, Size: 1260 bytes --]

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=947245

---Test result---

Test Summary:
CheckPatch                    PENDING   0.24 seconds
GitLint                       PENDING   0.22 seconds
BuildEll                      PASS      20.76 seconds
BluezMake                     PASS      1503.05 seconds
MakeCheck                     PASS      13.63 seconds
MakeDistcheck                 PASS      171.29 seconds
CheckValgrind                 PASS      225.50 seconds
CheckSmatch                   PASS      307.78 seconds
bluezmakeextell               PASS      98.94 seconds
IncrementalBuild              PENDING   0.32 seconds
ScanBuild                     PASS      876.53 seconds

Details
##############################
Test: CheckPatch - PENDING
Desc: Run checkpatch.pl script
Output:

##############################
Test: GitLint - PENDING
Desc: Run gitlint
Output:

##############################
Test: IncrementalBuild - PENDING
Desc: Incremental build with the patches in the series
Output:



---
Regards,
Linux Bluetooth

[PATCH BlueZ 2/3] device: Preserve pending flags when setting the wake allowed

[Ludovico de Nittis]

If there are already flags that are pending to be applied, we should
keep them to avoid overwriting them.
In device_set_wake_allowed() we only want to either add or remove the
remote wakeup flag, while keeping the existing flags as-is.
---
 src/device.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/src/device.c b/src/device.c
index e8bff718c..474ec5763 100644
--- a/src/device.c
+++ b/src/device.c
@@ -1605,6 +1605,10 @@ void device_set_wake_allowed(struct btd_device *device, bool wake_allowed,
 	device->pending_wake_allowed = wake_allowed;
 
 	flags = device->current_flags;
+
+	/* Include the pending flags, or they may get overwritten. */
+	flags |= device->pending_flags;
+
 	if (wake_allowed)
 		flags |= DEVICE_FLAG_REMOTE_WAKEUP;
 	else
-- 
2.49.0

[PATCH BlueZ 3/3] device: Try to set the wake_allowed property only for bonded devices

[Ludovico de Nittis]

When the function `device_set_wake_support()` is called, we don't have
the guarantees for the device to be already bonded.

For example, that function gets called by `hog_probe()`, that is also
triggered when bluez scans for new devices. In that instance, we don't
want to try setting the `wake_allowed` property, because those devices
are only in range of the host and are not connected, paired or bonded
yet.

This fixes the following Bluez error when we scan for new devices and a
new hog or hid is in range:
```
src/device.c:set_wake_allowed_complete() Set device flags return status:
Invalid Parameters
```

Additionally, because that initial `device_set_allowed()` call can fail,
this commit fixes the issue of hog and hid devices that, after the first
pairing, were unexpectedly showing `WakeAllowed: no`. And it required a
reboot to let that property be set to the expected `WakeAllowed: yes` by
default.
---
 src/device.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/src/device.c b/src/device.c
index 474ec5763..727b668af 100644
--- a/src/device.c
+++ b/src/device.c
@@ -1526,10 +1526,13 @@ void device_set_wake_support(struct btd_device *device, bool wake_support)
 	if (device->wake_override == WAKE_FLAG_DEFAULT)
 		device_set_wake_override(device, wake_support);
 
-	/* Set wake_allowed according to the override value. */
-	device_set_wake_allowed(device,
-				device->wake_override == WAKE_FLAG_ENABLED,
-				-1U);
+	/* Set wake_allowed according to the override value.
+	 * Limit this to bonded device to avoid trying to set it
+	 * to new devices that are simply in range. */
+	if (device_is_bonded(device, device->bdaddr_type))
+		device_set_wake_allowed(device,
+					device->wake_override == WAKE_FLAG_ENABLED,
+					-1U);
 }
 
 static bool device_get_wake_allowed(struct btd_device *device)
@@ -6561,6 +6564,10 @@ void device_bonding_complete(struct btd_device *device, uint8_t bdaddr_type,
 
 	device_auth_req_free(device);
 
+	/* Enable the wake_allowed property if required */
+	if (device->wake_override == WAKE_FLAG_ENABLED)
+		device_set_wake_allowed(device, true, -1U);
+
 	/* If we're already paired nothing more is needed */
 	if (state->paired)
 		return;
-- 
2.49.0

Re: [PATCH BlueZ 0/3] Fix wake_allowed reported error and not being set after pairing

[Luiz Augusto von Dentz]

Hi Ludovico,

On Tue, Mar 25, 2025 at 1:30 PM Ludovico de Nittis
<ludovico.denittis@collabora.com> wrote:
>
> When a new HID or HOG device is in range, Bluez reports the following error:
> ```
> src/device.c:set_wake_allowed_complete() Set device flags return status:
> Invalid Parameters
> ```
>
> Also, a side effect of that issue, when pairing a HID or HOG device, the
> wake_allowed property doesn't get enabled by default, as expected.
> It requires a Bluez restart in order to get that property correctly set.
>
> This patch series addresses those issues.
>
> Ludovico de Nittis (3):
>   adapter: Preserve pending flags when setting the Device Privacy Mode
>   device: Preserve pending flags when setting the wake allowed
>   device: Try to set the wake_allowed property only for bonded devices
>
>  src/adapter.c |  5 ++++-
>  src/device.c  | 19 +++++++++++++++----
>  2 files changed, 19 insertions(+), 5 deletions(-)
>
> --
> 2.49.0

One thing I just realized is that we do not persist the flags accross
restart/reboot, so while at it it might be worth adding support for
it, the other remark is that perhaps we should store things like
pending flags into struct mgmt so we can add unit test to make things
a little more future-proof in this respect, any these can probably
come later so we don't need to delay including these fixes.

-- 
Luiz Augusto von Dentz

Re: [PATCH BlueZ 0/3] Fix wake_allowed reported error and not being set after pairing

[patchwork-bot+bluetooth]

Hello:

This series was applied to bluetooth/bluez.git (master)
by Luiz Augusto von Dentz <luiz.von.dentz@intel.com>:

On Tue, 25 Mar 2025 18:28:43 +0100 you wrote:
> When a new HID or HOG device is in range, Bluez reports the following error:
> ```
> src/device.c:set_wake_allowed_complete() Set device flags return status:
> Invalid Parameters
> ```
> 
> Also, a side effect of that issue, when pairing a HID or HOG device, the
> wake_allowed property doesn't get enabled by default, as expected.
> It requires a Bluez restart in order to get that property correctly set.
> 
> [...]

Here is the summary with links:
  - [BlueZ,1/3] adapter: Preserve pending flags when setting the Device Privacy Mode
    (no matching commit)
  - [BlueZ,2/3] device: Preserve pending flags when setting the wake allowed
    https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=41cfe55ea759
  - [BlueZ,3/3] device: Try to set the wake_allowed property only for bonded devices
    (no matching commit)

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html