2024-02-19 06:52:35

by Shahid Vichhi

[permalink] [raw]
Subject: [PATCH v2] profiles/audio/ccp: Add initial code for ccp plugin

This adds initial code for ccp plugin which handle call control profile and
Telephone Bearer Services and Generic Telephone Bearer Services for
client role.
---
Makefile.plugins | 5 +
configure.ac | 4 +
profiles/audio/ccp.c | 238 +++++++++++++++++++++++++++++++++++++++++++
3 files changed, 247 insertions(+)
create mode 100644 profiles/audio/ccp.c

diff --git a/Makefile.plugins b/Makefile.plugins
index e960eedeabd3..4aa2c9c92854 100644
--- a/Makefile.plugins
+++ b/Makefile.plugins
@@ -138,6 +138,11 @@ builtin_modules += micp
builtin_sources += profiles/audio/micp.c
endif

+if CCP
+builtin_modules += ccp
+builtin_sources += profiles/audio/ccp.c
+endif
+
if CSIP
builtin_modules += csip
builtin_sources += profiles/audio/csip.c
diff --git a/configure.ac b/configure.ac
index 70e9d4be8127..c7dadf79a565 100644
--- a/configure.ac
+++ b/configure.ac
@@ -200,6 +200,10 @@ AC_ARG_ENABLE(mcp, AS_HELP_STRING([--disable-mcp],
[disable MCP profile]), [enable_mcp=${enableval}])
AM_CONDITIONAL(MCP, test "${enable_mcp}" != "no")

+AC_ARG_ENABLE(ccp, AS_HELP_STRING([--disable-ccp],
+ [disable CCP profile]), [enable_ccp=${enableval}])
+AM_CONDITIONAL(CCP, test "${enable_ccp}" != "no")
+
AC_ARG_ENABLE(vcp, AS_HELP_STRING([--disable-vcp],
[disable VCP profile]), [enable_vcp=${enableval}])
AM_CONDITIONAL(VCP, test "${enable_vcp}" != "no")
diff --git a/profiles/audio/ccp.c b/profiles/audio/ccp.c
new file mode 100644
index 000000000000..fe97911e75f4
--- /dev/null
+++ b/profiles/audio/ccp.c
@@ -0,0 +1,238 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ *
+ * BlueZ - Bluetooth protocol stack for Linux
+ *
+ * Copyright (C) 2024 Intel Corporation. All rights reserved.
+ *
+ *
+ */
+
+#ifdef HAVE_CONFIG_H
+#include <config.h>
+#endif
+
+#define _GNU_SOURCE
+
+#include <ctype.h>
+#include <stdbool.h>
+#include <stdlib.h>
+#include <stdio.h>
+#include <string.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <fcntl.h>
+#include <errno.h>
+
+#include <glib.h>
+
+#include "gdbus/gdbus.h"
+
+#include "lib/bluetooth.h"
+#include "lib/hci.h"
+#include "lib/sdp.h"
+#include "lib/uuid.h"
+
+#include "src/dbus-common.h"
+#include "src/shared/util.h"
+#include "src/shared/att.h"
+#include "src/shared/queue.h"
+#include "src/shared/gatt-db.h"
+#include "src/shared/gatt-client.h"
+#include "src/shared/gatt-server.h"
+#include "src/shared/ccp.h"
+
+#include "btio/btio.h"
+#include "src/plugin.h"
+#include "src/adapter.h"
+#include "src/gatt-database.h"
+#include "src/device.h"
+#include "src/profile.h"
+#include "src/service.h"
+#include "src/log.h"
+#include "src/error.h"
+
+#define GTBS_UUID_STR "0000184C-0000-1000-8000-00805f9b34fb"
+
+struct ccp_data {
+ struct btd_device *device;
+ struct btd_service *service;
+ struct bt_ccp *ccp;
+ unsigned int state_id;
+};
+
+static void ccp_debug(const char *str, void *user_data)
+{
+ DBG_IDX(0xffff, "%s", str);
+}
+
+static struct ccp_data *ccp_data_new(struct btd_device *device)
+{
+ struct ccp_data *data;
+
+ data = new0(struct ccp_data, 1);
+ data->device = device;
+
+ return data;
+}
+
+static int ccp_probe(struct btd_service *service)
+{
+ struct btd_device *device = btd_service_get_device(service);
+ struct btd_adapter *adapter = device_get_adapter(device);
+ struct btd_gatt_database *database = btd_adapter_get_database(adapter);
+ struct ccp_data *data = btd_service_get_user_data(service);
+ char addr[18];
+
+ ba2str(device_get_address(device), addr);
+ DBG("%s", addr);
+
+ /* Ignore, if we were probed for this device already */
+ if (data) {
+ error("Profile probed twice for the same device!");
+ return -EINVAL;
+ }
+
+ data = ccp_data_new(device);
+ data->service = service;
+
+ data->ccp = bt_ccp_new(btd_gatt_database_get_db(database),
+ btd_device_get_gatt_db(device));
+
+ bt_ccp_set_debug(data->ccp, ccp_debug, NULL, NULL);
+ btd_service_set_user_data(service, data);
+
+ return 0;
+}
+
+static void ccp_data_free(struct ccp_data *data)
+{
+ if (data->service) {
+ btd_service_set_user_data(data->service, NULL);
+ bt_ccp_set_user_data(data->ccp, NULL);
+ }
+
+ bt_ccp_unref(data->ccp);
+ free(data);
+}
+
+static void ccp_data_remove(struct ccp_data *data)
+{
+ DBG("data %p", data);
+
+ ccp_data_free(data);
+}
+
+static void ccp_remove(struct btd_service *service)
+{
+ struct btd_device *device = btd_service_get_device(service);
+ struct ccp_data *data;
+ char addr[18];
+
+ ba2str(device_get_address(device), addr);
+ DBG("%s", addr);
+
+ data = btd_service_get_user_data(service);
+ if (!data) {
+ error("CCP service not handled by profile");
+ return;
+ }
+
+ ccp_data_remove(data);
+}
+
+static int ccp_accept(struct btd_service *service)
+{
+ struct btd_device *device = btd_service_get_device(service);
+ struct bt_gatt_client *client = btd_device_get_gatt_client(device);
+ struct ccp_data *data = btd_service_get_user_data(service);
+ char addr[18];
+
+ ba2str(device_get_address(device), addr);
+ DBG("%s", addr);
+
+ if (!bt_ccp_attach(data->ccp, client)) {
+ error("VCP unable to attach");
+ return -EINVAL;
+ }
+
+ /*TODO: register telephony operations here*/
+
+ btd_service_connecting_complete(service, 0);
+
+ return 0;
+}
+
+static int ccp_connect(struct btd_service *service)
+{
+ struct btd_device *device = btd_service_get_device(service);
+ char addr[18];
+
+ ba2str(device_get_address(device), addr);
+ DBG("%s", addr);
+
+ return 0;
+}
+
+static int ccp_disconnect(struct btd_service *service)
+{
+ struct btd_device *device = btd_service_get_device(service);
+ struct ccp_data *data = btd_service_get_user_data(service);
+ char addr[18];
+
+ ba2str(device_get_address(device), addr);
+ DBG("%s", addr);
+
+ bt_ccp_detach(data->ccp);
+
+ btd_service_disconnecting_complete(service, 0);
+
+ return 0;
+}
+
+static int
+ccp_server_probe(struct btd_profile *p,
+ struct btd_adapter *adapter)
+{
+ struct btd_gatt_database *database = btd_adapter_get_database(adapter);
+
+ bt_ccp_register(btd_gatt_database_get_db(database));
+
+ return 0;
+}
+
+static void
+ccp_server_remove(struct btd_profile *p,
+ struct btd_adapter *adapter)
+{
+ DBG("CCP remove Adapter");
+}
+
+static struct btd_profile ccp_profile = {
+ .name = "ccp",
+ .priority = BTD_PROFILE_PRIORITY_MEDIUM,
+ .remote_uuid = GTBS_UUID_STR,
+ .device_probe = ccp_probe,
+ .device_remove = ccp_remove,
+ .accept = ccp_accept,
+ .connect = ccp_connect,
+ .disconnect = ccp_disconnect,
+
+ .adapter_probe = ccp_server_probe,
+ .adapter_remove = ccp_server_remove,
+
+ .experimental = true,
+};
+
+static int ccp_init(void)
+{
+ return btd_profile_register(&ccp_profile);
+}
+
+static void ccp_exit(void)
+{
+ btd_profile_unregister(&ccp_profile);
+}
+
+BLUETOOTH_PLUGIN_DEFINE(ccp, VERSION, BLUETOOTH_PLUGIN_PRIORITY_DEFAULT,
+ ccp_init, ccp_exit)
--
2.34.1



2024-02-19 07:38:04

by Paul Menzel

[permalink] [raw]
Subject: Re: [PATCH v2] profiles/audio/ccp: Add initial code for ccp plugin

Dear Shahid,


Thank you very much for your patch. Should the string BlueZ be part of
the tag? Also, “Add … code” is redundant in my opinion, as that often
done. Maybe:

Add CCP plugin for Call Control Profile

Am 19.02.24 um 00:54 schrieb Shahid Vichhi:
> This adds initial code for ccp plugin which handle call control profile and

handle*s* and I would capitalize Call Control Profile

> Telephone Bearer Services and Generic Telephone Bearer Services for
> client role.

Please document, which specification you used, and how you tested this.

I comment some more nits below.

> ---
> Makefile.plugins | 5 +
> configure.ac | 4 +
> profiles/audio/ccp.c | 238 +++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 247 insertions(+)
> create mode 100644 profiles/audio/ccp.c
>
> diff --git a/Makefile.plugins b/Makefile.plugins
> index e960eedeabd3..4aa2c9c92854 100644
> --- a/Makefile.plugins
> +++ b/Makefile.plugins
> @@ -138,6 +138,11 @@ builtin_modules += micp
> builtin_sources += profiles/audio/micp.c
> endif
>
> +if CCP
> +builtin_modules += ccp
> +builtin_sources += profiles/audio/ccp.c
> +endif
> +
> if CSIP
> builtin_modules += csip
> builtin_sources += profiles/audio/csip.c
> diff --git a/configure.ac b/configure.ac
> index 70e9d4be8127..c7dadf79a565 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -200,6 +200,10 @@ AC_ARG_ENABLE(mcp, AS_HELP_STRING([--disable-mcp],
> [disable MCP profile]), [enable_mcp=${enableval}])
> AM_CONDITIONAL(MCP, test "${enable_mcp}" != "no")
>
> +AC_ARG_ENABLE(ccp, AS_HELP_STRING([--disable-ccp],
> + [disable CCP profile]), [enable_ccp=${enableval}])

I would also add the long name in the help text: Call Control Profile.

> +AM_CONDITIONAL(CCP, test "${enable_ccp}" != "no")
> +
> AC_ARG_ENABLE(vcp, AS_HELP_STRING([--disable-vcp],
> [disable VCP profile]), [enable_vcp=${enableval}])
> AM_CONDITIONAL(VCP, test "${enable_vcp}" != "no")
> diff --git a/profiles/audio/ccp.c b/profiles/audio/ccp.c
> new file mode 100644
> index 000000000000..fe97911e75f4
> --- /dev/null
> +++ b/profiles/audio/ccp.c
> @@ -0,0 +1,238 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + *
> + * BlueZ - Bluetooth protocol stack for Linux
> + *
> + * Copyright (C) 2024 Intel Corporation. All rights reserved.
> + *
> + *
> + */
> +
> +#ifdef HAVE_CONFIG_H
> +#include <config.h>
> +#endif
> +
> +#define _GNU_SOURCE
> +
> +#include <ctype.h>
> +#include <stdbool.h>
> +#include <stdlib.h>
> +#include <stdio.h>
> +#include <string.h>
> +#include <sys/types.h>
> +#include <sys/stat.h>
> +#include <fcntl.h>
> +#include <errno.h>
> +
> +#include <glib.h>
> +
> +#include "gdbus/gdbus.h"
> +
> +#include "lib/bluetooth.h"
> +#include "lib/hci.h"
> +#include "lib/sdp.h"
> +#include "lib/uuid.h"
> +
> +#include "src/dbus-common.h"
> +#include "src/shared/util.h"
> +#include "src/shared/att.h"
> +#include "src/shared/queue.h"
> +#include "src/shared/gatt-db.h"
> +#include "src/shared/gatt-client.h"
> +#include "src/shared/gatt-server.h"
> +#include "src/shared/ccp.h"
> +
> +#include "btio/btio.h"
> +#include "src/plugin.h"
> +#include "src/adapter.h"
> +#include "src/gatt-database.h"
> +#include "src/device.h"
> +#include "src/profile.h"
> +#include "src/service.h"
> +#include "src/log.h"
> +#include "src/error.h"
> +
> +#define GTBS_UUID_STR "0000184C-0000-1000-8000-00805f9b34fb"
> +
> +struct ccp_data {
> + struct btd_device *device;
> + struct btd_service *service;
> + struct bt_ccp *ccp;
> + unsigned int state_id;
> +};
> +
> +static void ccp_debug(const char *str, void *user_data)
> +{
> + DBG_IDX(0xffff, "%s", str);
> +}
> +
> +static struct ccp_data *ccp_data_new(struct btd_device *device)
> +{
> + struct ccp_data *data;
> +
> + data = new0(struct ccp_data, 1);
> + data->device = device;
> +
> + return data;
> +}
> +
> +static int ccp_probe(struct btd_service *service)
> +{
> + struct btd_device *device = btd_service_get_device(service);
> + struct btd_adapter *adapter = device_get_adapter(device);
> + struct btd_gatt_database *database = btd_adapter_get_database(adapter);
> + struct ccp_data *data = btd_service_get_user_data(service);
> + char addr[18];
> +
> + ba2str(device_get_address(device), addr);
> + DBG("%s", addr);
> +
> + /* Ignore, if we were probed for this device already */

The comment is redundant, as the error message says the same.

> + if (data) {
> + error("Profile probed twice for the same device!");
> + return -EINVAL;
> + }
> +
> + data = ccp_data_new(device);
> + data->service = service;
> +
> + data->ccp = bt_ccp_new(btd_gatt_database_get_db(database),
> + btd_device_get_gatt_db(device));
> +
> + bt_ccp_set_debug(data->ccp, ccp_debug, NULL, NULL);
> + btd_service_set_user_data(service, data);
> +
> + return 0;
> +}
> +
> +static void ccp_data_free(struct ccp_data *data)
> +{
> + if (data->service) {
> + btd_service_set_user_data(data->service, NULL);
> + bt_ccp_set_user_data(data->ccp, NULL);
> + }
> +
> + bt_ccp_unref(data->ccp);
> + free(data);
> +}
> +
> +static void ccp_data_remove(struct ccp_data *data)
> +{
> + DBG("data %p", data);
> +
> + ccp_data_free(data);
> +}
> +
> +static void ccp_remove(struct btd_service *service)
> +{
> + struct btd_device *device = btd_service_get_device(service);
> + struct ccp_data *data;
> + char addr[18];
> +
> + ba2str(device_get_address(device), addr);
> + DBG("%s", addr);
> +
> + data = btd_service_get_user_data(service);
> + if (!data) {
> + error("CCP service not handled by profile");
> + return;
> + }
> +
> + ccp_data_remove(data);
> +}
> +
> +static int ccp_accept(struct btd_service *service)
> +{
> + struct btd_device *device = btd_service_get_device(service);
> + struct bt_gatt_client *client = btd_device_get_gatt_client(device);
> + struct ccp_data *data = btd_service_get_user_data(service);
> + char addr[18];
> +
> + ba2str(device_get_address(device), addr);
> + DBG("%s", addr);
> +
> + if (!bt_ccp_attach(data->ccp, client)) {
> + error("VCP unable to attach");

What is VCP?

> + return -EINVAL;
> + }
> +
> + /*TODO: register telephony operations here*/

I’d add a space after /* and before */.

> +
> + btd_service_connecting_complete(service, 0);
> +
> + return 0;
> +}
> +
> +static int ccp_connect(struct btd_service *service)
> +{
> + struct btd_device *device = btd_service_get_device(service);
> + char addr[18];
> +
> + ba2str(device_get_address(device), addr);
> + DBG("%s", addr);
> +
> + return 0;
> +}
> +
> +static int ccp_disconnect(struct btd_service *service)
> +{
> + struct btd_device *device = btd_service_get_device(service);
> + struct ccp_data *data = btd_service_get_user_data(service);
> + char addr[18];
> +
> + ba2str(device_get_address(device), addr);
> + DBG("%s", addr);
> +
> + bt_ccp_detach(data->ccp);
> +
> + btd_service_disconnecting_complete(service, 0);
> +
> + return 0;
> +}
> +
> +static int
> +ccp_server_probe(struct btd_profile *p,
> + struct btd_adapter *adapter)

Breaking the line like this looks inconsistent with above.

> +{
> + struct btd_gatt_database *database = btd_adapter_get_database(adapter);
> +
> + bt_ccp_register(btd_gatt_database_get_db(database));
> +
> + return 0;
> +}
> +
> +static void
> +ccp_server_remove(struct btd_profile *p,
> + struct btd_adapter *adapter)
> +{
> + DBG("CCP remove Adapter");

I’d use lower-case adapter.

> +}
> +
> +static struct btd_profile ccp_profile = {
> + .name = "ccp",
> + .priority = BTD_PROFILE_PRIORITY_MEDIUM,
> + .remote_uuid = GTBS_UUID_STR,
> + .device_probe = ccp_probe,
> + .device_remove = ccp_remove,
> + .accept = ccp_accept,
> + .connect = ccp_connect,
> + .disconnect = ccp_disconnect,
> +
> + .adapter_probe = ccp_server_probe,
> + .adapter_remove = ccp_server_remove,
> +
> + .experimental = true,
> +};
> +
> +static int ccp_init(void)
> +{
> + return btd_profile_register(&ccp_profile);
> +}
> +
> +static void ccp_exit(void)
> +{
> + btd_profile_unregister(&ccp_profile);
> +}
> +
> +BLUETOOTH_PLUGIN_DEFINE(ccp, VERSION, BLUETOOTH_PLUGIN_PRIORITY_DEFAULT,
> + ccp_init, ccp_exit)


Kind regards,

Paul

2024-02-19 08:10:45

by bluez.test.bot

[permalink] [raw]
Subject: RE: [v2] profiles/audio/ccp: Add initial code for ccp plugin

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

---Test result---

Test Summary:
CheckPatch PASS 0.36 seconds
GitLint PASS 0.19 seconds
BuildEll PASS 24.11 seconds
BluezMake PASS 745.45 seconds
MakeCheck PASS 11.58 seconds
MakeDistcheck PASS 163.30 seconds
CheckValgrind PASS 225.72 seconds
CheckSmatch PASS 330.98 seconds
bluezmakeextell PASS 107.32 seconds
IncrementalBuild PASS 685.96 seconds
ScanBuild WARNING 990.69 seconds

Details
##############################
Test: ScanBuild - WARNING
Desc: Run Scan Build
Output:
Makefile:13722: warning: overriding recipe for target 'install-data-hook'
Makefile:13717: warning: ignoring old recipe for target 'install-data-hook'
Makefile:13722: warning: overriding recipe for target 'install-data-hook'
Makefile:13717: warning: ignoring old recipe for target 'install-data-hook'
src/shared/gatt-client.c:451:21: warning: Use of memory after it is freed
gatt_db_unregister(op->client->db, op->db_id);
^~~~~~~~~~
src/shared/gatt-client.c:696:2: warning: Use of memory after it is freed
discovery_op_complete(op, false, att_ecode);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
src/shared/gatt-client.c:993:2: warning: Use of memory after it is freed
discovery_op_complete(op, success, att_ecode);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
src/shared/gatt-client.c:1099:2: warning: Use of memory after it is freed
discovery_op_complete(op, success, att_ecode);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
src/shared/gatt-client.c:1291:2: warning: Use of memory after it is freed
discovery_op_complete(op, success, att_ecode);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
src/shared/gatt-client.c:1356:2: warning: Use of memory after it is freed
discovery_op_complete(op, success, att_ecode);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
src/shared/gatt-client.c:1631:6: warning: Use of memory after it is freed
if (read_db_hash(op)) {
^~~~~~~~~~~~~~~~
src/shared/gatt-client.c:1636:2: warning: Use of memory after it is freed
discover_all(op);
^~~~~~~~~~~~~~~~
src/shared/gatt-client.c:2140:6: warning: Use of memory after it is freed
if (read_db_hash(op)) {
^~~~~~~~~~~~~~~~
src/shared/gatt-client.c:2148:8: warning: Use of memory after it is freed
discovery_op_ref(op),
^~~~~~~~~~~~~~~~~~~~
src/shared/gatt-client.c:3237:2: warning: Use of memory after it is freed
complete_write_long_op(req, success, 0, false);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
src/shared/gatt-client.c:3259:2: warning: Use of memory after it is freed
request_unref(req);
^~~~~~~~~~~~~~~~~~
12 warnings generated.



---
Regards,
Linux Bluetooth