diff mbox series

[BlueZ] gatt-client: allow AcquireNotify when characteristic has indicate flag

Message ID 20240822113226.223790-1-kupper.benedek@gmail.com (mailing list archive)
State New, archived
Headers show
Series [BlueZ] gatt-client: allow AcquireNotify when characteristic has indicate flag | expand

Checks

Context Check Description
tedd_an/pre-ci_am success Success
tedd_an/CheckPatch success CheckPatch PASS
tedd_an/GitLint success Gitlint PASS
tedd_an/BuildEll success Build ELL PASS
tedd_an/BluezMake success Bluez Make PASS
tedd_an/MakeCheck fail BlueZ Make Check FAIL:
tedd_an/MakeDistcheck fail Make Distcheck FAIL: Package cups was not found in the pkg-config search path. Perhaps you should add the directory containing `cups.pc' to the PKG_CONFIG_PATH environment variable No package 'cups' found make[4]: *** [Makefile:11766: test-suite.log] Error 1 make[3]: *** [Makefile:11874: check-TESTS] Error 2 make[2]: *** [Makefile:12303: check-am] Error 2 make[1]: *** [Makefile:12305: check] Error 2 make: *** [Makefile:12226: distcheck] Error 1
tedd_an/CheckValgrind fail Check Valgrind FAIL: tools/mgmt-tester.c: In function ‘main’: tools/mgmt-tester.c:12725:5: note: variable tracking size limit exceeded with ‘-fvar-tracking-assignments’, retrying without 12725 | int main(int argc, char *argv[]) | ^~~~ make[3]: *** [Makefile:11766: test-suite.log] Error 1 make[2]: *** [Makefile:11874: check-TESTS] Error 2 make[1]: *** [Makefile:12303: check-am] Error 2 make: *** [Makefile:12305: check] Error 2
tedd_an/CheckSmatch success CheckSparse PASS
tedd_an/bluezmakeextell success Make External ELL PASS
tedd_an/IncrementalBuild success Incremental Build PASS
tedd_an/ScanBuild success Scan Build PASS

Commit Message

Benedek Kupper Aug. 22, 2024, 11:32 a.m. UTC
From: Benedek Kupper <benedek.kupper@epitome.inc>

StartNotify / StopNotify already correctly allows usage when the
characteristic indicate is present (simplify this check though),
apply the same to AcquireNotify.
---
 src/gatt-client.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

Comments

bluez.test.bot@gmail.com Aug. 22, 2024, 1:03 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=882131

---Test result---

Test Summary:
CheckPatch                    PASS      0.46 seconds
GitLint                       PASS      0.32 seconds
BuildEll                      PASS      24.50 seconds
BluezMake                     PASS      1685.40 seconds
MakeCheck                     FAIL      13.82 seconds
MakeDistcheck                 FAIL      159.20 seconds
CheckValgrind                 FAIL      251.10 seconds
CheckSmatch                   PASS      353.95 seconds
bluezmakeextell               PASS      119.71 seconds
IncrementalBuild              PASS      1440.61 seconds
ScanBuild                     PASS      1031.19 seconds

Details
##############################
Test: MakeCheck - FAIL
Desc: Run Bluez Make Check
Output:

make[3]: *** [Makefile:11766: test-suite.log] Error 1
make[2]: *** [Makefile:11874: check-TESTS] Error 2
make[1]: *** [Makefile:12303: check-am] Error 2
make: *** [Makefile:12305: check] Error 2
##############################
Test: MakeDistcheck - FAIL
Desc: Run Bluez Make Distcheck
Output:

Package cups was not found in the pkg-config search path.
Perhaps you should add the directory containing `cups.pc'
to the PKG_CONFIG_PATH environment variable
No package 'cups' found
make[4]: *** [Makefile:11766: test-suite.log] Error 1
make[3]: *** [Makefile:11874: check-TESTS] Error 2
make[2]: *** [Makefile:12303: check-am] Error 2
make[1]: *** [Makefile:12305: check] Error 2
make: *** [Makefile:12226: distcheck] Error 1
##############################
Test: CheckValgrind - FAIL
Desc: Run Bluez Make Check with Valgrind
Output:

tools/mgmt-tester.c: In function ‘main’:
tools/mgmt-tester.c:12725:5: note: variable tracking size limit exceeded with ‘-fvar-tracking-assignments’, retrying without
12725 | int main(int argc, char *argv[])
      |     ^~~~
make[3]: *** [Makefile:11766: test-suite.log] Error 1
make[2]: *** [Makefile:11874: check-TESTS] Error 2
make[1]: *** [Makefile:12303: check-am] Error 2
make: *** [Makefile:12305: check] Error 2


---
Regards,
Linux Bluetooth
Luiz Augusto von Dentz Aug. 22, 2024, 4:17 p.m. UTC | #2
Hi Benedek,

On Thu, Aug 22, 2024 at 7:32 AM Benedek Kupper <kupper.benedek@gmail.com> wrote:
>
> From: Benedek Kupper <benedek.kupper@epitome.inc>
>
> StartNotify / StopNotify already correctly allows usage when the
> characteristic indicate is present (simplify this check though),
> apply the same to AcquireNotify.
> ---
>  src/gatt-client.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/src/gatt-client.c b/src/gatt-client.c
> index 8d83a9577..a67e04eee 100644
> --- a/src/gatt-client.c
> +++ b/src/gatt-client.c
> @@ -1556,7 +1556,8 @@ static DBusMessage *characteristic_acquire_notify(DBusConnection *conn,
>         if (!queue_isempty(chrc->notify_clients))
>                 return btd_error_in_progress(msg);
>
> -       if (!(chrc->props & BT_GATT_CHRC_PROP_NOTIFY))
> +       if (!(chrc->props & (BT_GATT_CHRC_PROP_NOTIFY |
> +                       BT_GATT_CHRC_PROP_INDICATE)))
>                 return btd_error_not_supported(msg);
>
>         client = notify_client_create(chrc, sender);
> @@ -1601,8 +1602,8 @@ static DBusMessage *characteristic_start_notify(DBusConnection *conn,
>         if (chrc->notify_io)
>                 return btd_error_not_permitted(msg, "Notify acquired");
>
> -       if (!(chrc->props & BT_GATT_CHRC_PROP_NOTIFY ||
> -                               chrc->props & BT_GATT_CHRC_PROP_INDICATE))
> +       if (!(chrc->props & (BT_GATT_CHRC_PROP_NOTIFY |
> +                               BT_GATT_CHRC_PROP_INDICATE)))
>                 return btd_error_not_supported(msg);

Afaik this is working according to the documentation:

'Only works with characteristic that has NotifyAcquired property which
relies on "notify" Flag and no other client have called
StartNotify().'

https://github.com/bluez/bluez/blob/master/doc/org.bluez.GattCharacteristic.rst#fd-uint16-acquirenotifydict-options-optional[1]

So if we want it to work with Indication as well then we need to
change the documentation to reflect these changes, indication does
mean one need to confirm the reception which is something that would
require some work if done via fd, but in the other hand I think we are
auto confirming in case of D-Bus since that is using a D-Bus signals
so perhaps it would be fine to do the same with fd as well.

>         /* Each client can only have one active notify session. */
> --
> 2.34.1
>
>
diff mbox series

Patch

diff --git a/src/gatt-client.c b/src/gatt-client.c
index 8d83a9577..a67e04eee 100644
--- a/src/gatt-client.c
+++ b/src/gatt-client.c
@@ -1556,7 +1556,8 @@  static DBusMessage *characteristic_acquire_notify(DBusConnection *conn,
 	if (!queue_isempty(chrc->notify_clients))
 		return btd_error_in_progress(msg);
 
-	if (!(chrc->props & BT_GATT_CHRC_PROP_NOTIFY))
+	if (!(chrc->props & (BT_GATT_CHRC_PROP_NOTIFY |
+			BT_GATT_CHRC_PROP_INDICATE)))
 		return btd_error_not_supported(msg);
 
 	client = notify_client_create(chrc, sender);
@@ -1601,8 +1602,8 @@  static DBusMessage *characteristic_start_notify(DBusConnection *conn,
 	if (chrc->notify_io)
 		return btd_error_not_permitted(msg, "Notify acquired");
 
-	if (!(chrc->props & BT_GATT_CHRC_PROP_NOTIFY ||
-				chrc->props & BT_GATT_CHRC_PROP_INDICATE))
+	if (!(chrc->props & (BT_GATT_CHRC_PROP_NOTIFY |
+				BT_GATT_CHRC_PROP_INDICATE)))
 		return btd_error_not_supported(msg);
 
 	/* Each client can only have one active notify session. */