diff mbox series

[v2,1/9] adapter: Enable MSFT a2dp offload codec when Experimental is set

Message ID 20211119094235.2432-1-kiran.k@intel.com (mailing list archive)
State Changes Requested
Headers show
Series [v2,1/9] adapter: Enable MSFT a2dp offload codec when Experimental is set | expand

Checks

Context Check Description
tedd_an/checkpatch warning [v2,1/9] adapter: Enable MSFT a2dp offload codec when Experimental is set WARNING:LONG_LINE_STRING: line length of 83 exceeds 80 columns #89: FILE: src/adapter.c:9806: + error("Set MSFT a2dp offload codec failed with status 0x%02x (%s)", /github/workspace/src/12628551.patch total: 0 errors, 1 warnings, 75 lines checked NOTE: For some of the reported defects, checkpatch may be able to mechanically convert to the typical style using --fix or --fix-inplace. /github/workspace/src/12628551.patch has style problems, please review. NOTE: Ignored message types: COMMIT_MESSAGE COMPLEX_MACRO CONST_STRUCT FILE_PATH_CHANGES MISSING_SIGN_OFF PREFER_PACKED SPDX_LICENSE_TAG SPLIT_STRING SSCANF_TO_KSTRTO NOTE: If any of the errors are false positives, please report them to the maintainer, see CHECKPATCH in MAINTAINERS.
tedd_an/gitlint success Gitlint PASS
tedd_an/setupell success Setup ELL PASS
tedd_an/buildprep success Build Prep PASS
tedd_an/build success Build Configuration PASS
tedd_an/makecheck fail Make Check FAIL: profiles/audio/avdtp.c: In function ‘avdtp_new’: profiles/audio/avdtp.c:2477:8: error: unused variable ‘use_offload’ [-Werror=unused-variable] 2477 | char *use_offload; | ^~~~~~~~~~~ cc1: all warnings being treated as errors make[1]: *** [Makefile:8640: profiles/audio/bluetoothd-avdtp.o] Error 1 make: *** [Makefile:10501: check] Error 2
tedd_an/makedistcheck success Make Distcheck PASS
tedd_an/build_extell success Build External ELL PASS
tedd_an/build_extell_make fail Build Make with External ELL FAIL: profiles/audio/avdtp.c: In function ‘avdtp_new’: profiles/audio/avdtp.c:2477:8: error: unused variable ‘use_offload’ [-Werror=unused-variable] 2477 | char *use_offload; | ^~~~~~~~~~~ cc1: all warnings being treated as errors make[1]: *** [Makefile:8640: profiles/audio/bluetoothd-avdtp.o] Error 1 make: *** [Makefile:4175: all] Error 2

Commit Message

Kiran K Nov. 19, 2021, 9:42 a.m. UTC 
This enables codec offload experimental feature if its UUIDs has been
enabled by main.conf:Experimental or -E has been passed in the command
line.
---
 src/adapter.c | 43 +++++++++++++++++++++++++++++++++++++++++++
 src/main.c    |  1 +
 src/main.conf |  1 +
 3 files changed, 45 insertions(+)

Comments

bluez.test.bot@gmail.com Nov. 19, 2021, 9:55 a.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=582945

---Test result---

Test Summary:
CheckPatch                    FAIL      13.45 seconds
GitLint                       PASS      8.97 seconds
Prep - Setup ELL              PASS      41.62 seconds
Build - Prep                  PASS      0.65 seconds
Build - Configure             PASS      7.91 seconds
Build - Make                  FAIL      138.93 seconds
Make Check                    FAIL      1.66 seconds
Make Distcheck                PASS      217.33 seconds
Build w/ext ELL - Configure   PASS      8.13 seconds
Build w/ext ELL - Make        FAIL      127.35 seconds

Details
##############################
Test: CheckPatch - FAIL
Desc: Run checkpatch.pl script with rule in .checkpatch.conf
Output:
[v2,1/9] adapter: Enable MSFT a2dp offload codec when Experimental is set
WARNING:LONG_LINE_STRING: line length of 83 exceeds 80 columns
#89: FILE: src/adapter.c:9806:
+		error("Set MSFT a2dp offload codec failed with status 0x%02x (%s)",

/github/workspace/src/12628551.patch total: 0 errors, 1 warnings, 75 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
      mechanically convert to the typical style using --fix or --fix-inplace.

/github/workspace/src/12628551.patch has style problems, please review.

NOTE: Ignored message types: COMMIT_MESSAGE COMPLEX_MACRO CONST_STRUCT FILE_PATH_CHANGES MISSING_SIGN_OFF PREFER_PACKED SPDX_LICENSE_TAG SPLIT_STRING SSCANF_TO_KSTRTO

NOTE: If any of the errors are false positives, please report
      them to the maintainer, see CHECKPATCH in MAINTAINERS.

[v2,6/9] avdtp: Add support for offload MSFT open command
WARNING:PREFER_DEFINED_ATTRIBUTE_MACRO: Prefer __packed over __attribute__((packed))
#72: FILE: lib/bluetooth.h:166:
+} __attribute__((packed));

/github/workspace/src/12628561.patch total: 0 errors, 1 warnings, 71 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
      mechanically convert to the typical style using --fix or --fix-inplace.

/github/workspace/src/12628561.patch has style problems, please review.

NOTE: Ignored message types: COMMIT_MESSAGE COMPLEX_MACRO CONST_STRUCT FILE_PATH_CHANGES MISSING_SIGN_OFF PREFER_PACKED SPDX_LICENSE_TAG SPLIT_STRING SSCANF_TO_KSTRTO

NOTE: If any of the errors are false positives, please report
      them to the maintainer, see CHECKPATCH in MAINTAINERS.


##############################
Test: Build - Make - FAIL
Desc: Build the BlueZ source tree
Output:
profiles/audio/avdtp.c: In function ‘avdtp_new’:
profiles/audio/avdtp.c:2477:8: error: unused variable ‘use_offload’ [-Werror=unused-variable]
 2477 |  char *use_offload;
      |        ^~~~~~~~~~~
cc1: all warnings being treated as errors
make[1]: *** [Makefile:8640: profiles/audio/bluetoothd-avdtp.o] Error 1
make: *** [Makefile:4175: all] Error 2


##############################
Test: Make Check - FAIL
Desc: Run 'make check'
Output:
profiles/audio/avdtp.c: In function ‘avdtp_new’:
profiles/audio/avdtp.c:2477:8: error: unused variable ‘use_offload’ [-Werror=unused-variable]
 2477 |  char *use_offload;
      |        ^~~~~~~~~~~
cc1: all warnings being treated as errors
make[1]: *** [Makefile:8640: profiles/audio/bluetoothd-avdtp.o] Error 1
make: *** [Makefile:10501: check] Error 2


##############################
Test: Build w/ext ELL - Make - FAIL
Desc: Build BlueZ source with '--enable-external-ell' configuration
Output:
profiles/audio/avdtp.c: In function ‘avdtp_new’:
profiles/audio/avdtp.c:2477:8: error: unused variable ‘use_offload’ [-Werror=unused-variable]
 2477 |  char *use_offload;
      |        ^~~~~~~~~~~
cc1: all warnings being treated as errors
make[1]: *** [Makefile:8640: profiles/audio/bluetoothd-avdtp.o] Error 1
make: *** [Makefile:4175: all] Error 2




---
Regards,
Linux Bluetooth
Marcel Holtmann Nov. 19, 2021, 10:06 a.m. UTC | #2 
Hi Kiran,

> This enables codec offload experimental feature if its UUIDs has been
> enabled by main.conf:Experimental or -E has been passed in the command
> line.
> ---
> src/adapter.c | 43 +++++++++++++++++++++++++++++++++++++++++++
> src/main.c    |  1 +
> src/main.conf |  1 +
> 3 files changed, 45 insertions(+)
> 
> diff --git a/src/adapter.c b/src/adapter.c
> index 309956bbb5be..1627cc127057 100644
> --- a/src/adapter.c
> +++ b/src/adapter.c
> @@ -142,6 +142,13 @@ static const struct mgmt_exp_uuid codec_offload_uuid = {
> 	.str = "a6695ace-ee7f-4fb9-881a-5fac66c629af"
> };
> 
> +/* 0cc2131f-96f0-4cd1-b313-b97e7cbc8335 */
> +static const struct mgmt_exp_uuid msft_a2dp_offload_codecs_uuid = {
> +	.val = { 0x35, 0x83, 0xbc, 0x7c, 0x7e, 0xb9, 0x13, 0xb3,
> +		0xd1, 0x4c, 0xf0, 0x96, 0x1f, 0x13, 0xc2, 0x0c},
> +	.str = "0cc2131f-96f0-4cd1-b313-b97e7cbc8335"
> +};
> +
> static DBusConnection *dbus_conn = NULL;
> 
> static uint32_t kernel_features = 0;
> @@ -9789,6 +9796,41 @@ static void codec_offload_func(struct btd_adapter *adapter, uint8_t action)
> 	btd_error(adapter->dev_id, "Failed to set Codec Offload");
> }
> 
> +static void msft_a2dp_offload_complete(uint8_t status, uint16_t len,
> +				       const void *param, void *user_data)
> +{
> +	struct btd_adapter *adapter = user_data;
> +	uint8_t action = btd_opts.experimental ? 0x01 : 0x00;
> +
> +	if (status != 0) {
> +		error("Set MSFT a2dp offload codec failed with status 0x%02x (%s)",
> +		       status, mgmt_errstr(status));
> +		return;
> +	}
> +
> +	DBG("MSFT a2dp offload codecs successfully set");

we need to switch to using btd_debug or DBG_IDX to include the index number in the traces.

> +
> +	if (action)
> +		queue_push_tail(adapter->exps,
> +				(void *)msft_a2dp_offload_codecs_uuid.val);
> +}
> +
> +static void msft_a2dp_offload_func(struct btd_adapter *adapter, uint8_t action)
> +{
> +	struct mgmt_cp_set_exp_feature cp;
> +
> +	memset(&cp, 0, sizeof(cp));
> +	memcpy(cp.uuid, msft_a2dp_offload_codecs_uuid.val, 16);
> +	cp.action = action;
> +
> +	if (mgmt_send(adapter->mgmt, MGMT_OP_SET_EXP_FEATURE,
> +		      adapter->dev_id, sizeof(cp), &cp,
> +		      msft_a2dp_offload_complete, adapter, NULL) > 0)
> +		return;
> +
> +	btd_error(adapter->dev_id, "Failed to set RPA Resolution");
> +}

We are no longer dealing with the blunt copy-and-paste mistakes, please do a proper review before sending any patch.

Regards

Marcel
Kiran K Nov. 19, 2021, 10:10 a.m. UTC | #3 
Hi Marcel,

> -----Original Message-----
> From: Marcel Holtmann <marcel@holtmann.org>
> Sent: Friday, November 19, 2021 3:36 PM
> To: K, Kiran <kiran.k@intel.com>
> Cc: linux-bluetooth@vger.kernel.org; Srivatsa, Ravishankar
> <ravishankar.srivatsa@intel.com>; Tumkur Narayan, Chethan
> <chethan.tumkur.narayan@intel.com>; Von Dentz, Luiz
> <luiz.von.dentz@intel.com>
> Subject: Re: [PATCH v2 1/9] adapter: Enable MSFT a2dp offload codec when
> Experimental is set
> 
> Hi Kiran,
> 
> > This enables codec offload experimental feature if its UUIDs has been
> > enabled by main.conf:Experimental or -E has been passed in the command
> > line.
> > ---
> > src/adapter.c | 43 +++++++++++++++++++++++++++++++++++++++++++
> > src/main.c    |  1 +
> > src/main.conf |  1 +
> > 3 files changed, 45 insertions(+)
> >
> > diff --git a/src/adapter.c b/src/adapter.c index
> > 309956bbb5be..1627cc127057 100644
> > --- a/src/adapter.c
> > +++ b/src/adapter.c
> > @@ -142,6 +142,13 @@ static const struct mgmt_exp_uuid
> codec_offload_uuid = {
> > 	.str = "a6695ace-ee7f-4fb9-881a-5fac66c629af"
> > };
> >
> > +/* 0cc2131f-96f0-4cd1-b313-b97e7cbc8335 */ static const struct
> > +mgmt_exp_uuid msft_a2dp_offload_codecs_uuid = {
> > +	.val = { 0x35, 0x83, 0xbc, 0x7c, 0x7e, 0xb9, 0x13, 0xb3,
> > +		0xd1, 0x4c, 0xf0, 0x96, 0x1f, 0x13, 0xc2, 0x0c},
> > +	.str = "0cc2131f-96f0-4cd1-b313-b97e7cbc8335"
> > +};
> > +
> > static DBusConnection *dbus_conn = NULL;
> >
> > static uint32_t kernel_features = 0;
> > @@ -9789,6 +9796,41 @@ static void codec_offload_func(struct
> btd_adapter *adapter, uint8_t action)
> > 	btd_error(adapter->dev_id, "Failed to set Codec Offload"); }
> >
> > +static void msft_a2dp_offload_complete(uint8_t status, uint16_t len,
> > +				       const void *param, void *user_data) {
> > +	struct btd_adapter *adapter = user_data;
> > +	uint8_t action = btd_opts.experimental ? 0x01 : 0x00;
> > +
> > +	if (status != 0) {
> > +		error("Set MSFT a2dp offload codec failed with status 0x%02x
> (%s)",
> > +		       status, mgmt_errstr(status));
> > +		return;
> > +	}
> > +
> > +	DBG("MSFT a2dp offload codecs successfully set");
> 
> we need to switch to using btd_debug or DBG_IDX to include the index
> number in the traces.

Ok.
> 
> > +
> > +	if (action)
> > +		queue_push_tail(adapter->exps,
> > +				(void *)msft_a2dp_offload_codecs_uuid.val);
> > +}
> > +
> > +static void msft_a2dp_offload_func(struct btd_adapter *adapter,
> > +uint8_t action) {
> > +	struct mgmt_cp_set_exp_feature cp;
> > +
> > +	memset(&cp, 0, sizeof(cp));
> > +	memcpy(cp.uuid, msft_a2dp_offload_codecs_uuid.val, 16);
> > +	cp.action = action;
> > +
> > +	if (mgmt_send(adapter->mgmt, MGMT_OP_SET_EXP_FEATURE,
> > +		      adapter->dev_id, sizeof(cp), &cp,
> > +		      msft_a2dp_offload_complete, adapter, NULL) > 0)
> > +		return;
> > +
> > +	btd_error(adapter->dev_id, "Failed to set RPA Resolution"); }
> 
> We are no longer dealing with the blunt copy-and-paste mistakes, please do
> a proper review before sending any patch.

My bad. I will address in the next patchset.
> 
> Regards
> 
> Marcel

Thanks,
Kiran
diff mbox series

Patch

diff --git a/src/adapter.c b/src/adapter.c
index 309956bbb5be..1627cc127057 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -142,6 +142,13 @@  static const struct mgmt_exp_uuid codec_offload_uuid = {
 	.str = "a6695ace-ee7f-4fb9-881a-5fac66c629af"
 };
 
+/* 0cc2131f-96f0-4cd1-b313-b97e7cbc8335 */
+static const struct mgmt_exp_uuid msft_a2dp_offload_codecs_uuid = {
+	.val = { 0x35, 0x83, 0xbc, 0x7c, 0x7e, 0xb9, 0x13, 0xb3,
+		0xd1, 0x4c, 0xf0, 0x96, 0x1f, 0x13, 0xc2, 0x0c},
+	.str = "0cc2131f-96f0-4cd1-b313-b97e7cbc8335"
+};
+
 static DBusConnection *dbus_conn = NULL;
 
 static uint32_t kernel_features = 0;
@@ -9789,6 +9796,41 @@  static void codec_offload_func(struct btd_adapter *adapter, uint8_t action)
 	btd_error(adapter->dev_id, "Failed to set Codec Offload");
 }
 
+static void msft_a2dp_offload_complete(uint8_t status, uint16_t len,
+				       const void *param, void *user_data)
+{
+	struct btd_adapter *adapter = user_data;
+	uint8_t action = btd_opts.experimental ? 0x01 : 0x00;
+
+	if (status != 0) {
+		error("Set MSFT a2dp offload codec failed with status 0x%02x (%s)",
+		       status, mgmt_errstr(status));
+		return;
+	}
+
+	DBG("MSFT a2dp offload codecs successfully set");
+
+	if (action)
+		queue_push_tail(adapter->exps,
+				(void *)msft_a2dp_offload_codecs_uuid.val);
+}
+
+static void msft_a2dp_offload_func(struct btd_adapter *adapter, uint8_t action)
+{
+	struct mgmt_cp_set_exp_feature cp;
+
+	memset(&cp, 0, sizeof(cp));
+	memcpy(cp.uuid, msft_a2dp_offload_codecs_uuid.val, 16);
+	cp.action = action;
+
+	if (mgmt_send(adapter->mgmt, MGMT_OP_SET_EXP_FEATURE,
+		      adapter->dev_id, sizeof(cp), &cp,
+		      msft_a2dp_offload_complete, adapter, NULL) > 0)
+		return;
+
+	btd_error(adapter->dev_id, "Failed to set RPA Resolution");
+}
+
 static const struct exp_feat {
 	const struct mgmt_exp_uuid *uuid;
 	void (*func)(struct btd_adapter *adapter, uint8_t action);
@@ -9799,6 +9841,7 @@  static const struct exp_feat {
 	EXP_FEAT(&quality_report_uuid, quality_report_func),
 	EXP_FEAT(&rpa_resolution_uuid, rpa_resolution_func),
 	EXP_FEAT(&codec_offload_uuid, codec_offload_func),
+	EXP_FEAT(&msft_a2dp_offload_codecs_uuid, msft_a2dp_offload_func),
 };
 
 static void read_exp_features_complete(uint8_t status, uint16_t length,
diff --git a/src/main.c b/src/main.c
index dd954e1abfe9..9776ff89a1d9 100644
--- a/src/main.c
+++ b/src/main.c
@@ -571,6 +571,7 @@  static const char *valid_uuids[] = {
 	"15c0a148-c273-11ea-b3de-0242ac130004",
 	"330859bc-7506-492d-9370-9a6f0614037f",
 	"a6695ace-ee7f-4fb9-881a-5fac66c629af",
+	"0cc2131f-96f0-4cd1-b313-b97e7cbc8335",
 	"*"
 };
 
diff --git a/src/main.conf b/src/main.conf
index c82d7e6482c4..ab4a6128ad5c 100644
--- a/src/main.conf
+++ b/src/main.conf
@@ -116,6 +116,7 @@ 
 # 15c0a148-c273-11ea-b3de-0242ac130004 (BlueZ Experimental LL privacy)
 # 330859bc-7506-492d-9370-9a6f0614037f (BlueZ Experimental Bluetooth Quality Report)
 # a6695ace-ee7f-4fb9-881a-5fac66c629af (BlueZ Experimental Offload Codecs)
+# 0cc2131f-96f0-4cd1-b313-b97e7cbc8335 (BlueZ Experimental MSFT a2dp offload Codecs)
 # Defaults to false.
 #Experimental = false