[PATCH v2 1/3] battery: Add BT SIG reserved numbers used by Battery Service

[PATCH v2 1/3] battery: Add BT SIG reserved numbers used by Battery Service

[Bastien Nocera]

Add the Battery Level and Battery Power State UUIDs as per:
https://www.bluetooth.com/specifications/gatt/viewer?attributeXmlFile=org.bluetooth.characteristic.battery_level.xml
https://www.bluetooth.com/specifications/gatt/viewer?attributeXmlFile=org.bluetooth.characteristic.battery_power_state.xml
---
 lib/uuid.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/lib/uuid.h b/lib/uuid.h
index 2c57a3378..614b3c4e4 100644
--- a/lib/uuid.h
+++ b/lib/uuid.h
@@ -119,6 +119,8 @@ extern "C" {
 #define GATT_CHARAC_RECONNECTION_ADDRESS		0x2A03
 #define GATT_CHARAC_PERIPHERAL_PREF_CONN		0x2A04
 #define GATT_CHARAC_SERVICE_CHANGED			0x2A05
+#define GATT_CHARAC_BATTERY_LEVEL			0x2A19
+#define GATT_CHARAC_BATTERY_POWER_STATE			0x2A1A
 #define GATT_CHARAC_SYSTEM_ID				0x2A23
 #define GATT_CHARAC_MODEL_NUMBER_STRING			0x2A24
 #define GATT_CHARAC_SERIAL_NUMBER_STRING		0x2A25
-- 
2.14.2

[PATCH v2 2/3] profiles/battery: Add Bluetooth LE Battery service

[Bastien Nocera]

Only the Battery Level characteristic was tested with a
Microsoft Arc Touch Mouse SE.

This patch however hopes to support both the Battery Level and
the Battery Power State characteristics.
---
 Makefile.plugins           |   3 +
 doc/battery-api.txt        |  36 +++
 profiles/battery/battery.c | 570 +++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 609 insertions(+)
 create mode 100644 doc/battery-api.txt
 create mode 100644 profiles/battery/battery.c

diff --git a/Makefile.plugins b/Makefile.plugins
index 73377e532..1f3b5b552 100644
--- a/Makefile.plugins
+++ b/Makefile.plugins
@@ -100,6 +100,9 @@ builtin_sources += profiles/midi/midi.c \
 builtin_ldadd += @ALSA_LIBS@
 endif
 
+builtin_modules += battery
+builtin_sources += profiles/battery/battery.c
+
 if SIXAXIS
 plugin_LTLIBRARIES += plugins/sixaxis.la
 plugins_sixaxis_la_SOURCES = plugins/sixaxis.c
diff --git a/doc/battery-api.txt b/doc/battery-api.txt
new file mode 100644
index 000000000..6d28f7b17
--- /dev/null
+++ b/doc/battery-api.txt
@@ -0,0 +1,36 @@
+BlueZ D-Bus Battery API description
+***********************************
+
+
+Battery hierarchy
+=================
+
+Service		org.bluez
+Interface	org.bluez.Battery1
+Object path	[variable prefix]/{hci0,hci1,...}/dev_XX_XX_XX_XX_XX_XX
+
+Properties	boolean Present [readonly]
+
+			Whether the device has a battery present.
+
+		boolean Rechargeable [readonly]
+
+			Whether the battery built into the device is rechargeable
+			or not.
+
+		uint16 Percentage [readonly]
+
+			The percentage of battery left. Will be 0% percent if State is
+			used instead, depending on the Battery profile used by the device.
+
+		string State [readonly]
+
+			If Percentage is set, State will be an empty string. Otherwise this
+			will be one of 'unknown', 'discharging', 'charging' or
+			'fully-charged'.
+
+		string BatteryLevel [readonly]
+
+			If Percentage is set, BatteryLevel will be an empty string.
+			Otherwise this will be one of 'normal', 'critical' or
+			'unknown'.
diff --git a/profiles/battery/battery.c b/profiles/battery/battery.c
new file mode 100644
index 000000000..ab4603de3
--- /dev/null
+++ b/profiles/battery/battery.c
@@ -0,0 +1,570 @@
+/*
+ *
+ *  BlueZ - Bluetooth protocol stack for Linux
+ *
+ *  Copyright (C) 2012  Instituto Nokia de Tecnologia - INdT
+ *  Copyright (C) 2014  Google Inc.
+ *  Copyright (C) 2017  Red Hat Inc.
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ */
+
+#ifdef HAVE_CONFIG_H
+#include <config.h>
+#endif
+
+#include <ctype.h>
+#include <stdbool.h>
+#include <stdlib.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/plugin.h"
+#include "src/adapter.h"
+#include "src/device.h"
+#include "src/profile.h"
+#include "src/service.h"
+#include "src/log.h"
+#include "attrib/att.h"
+
+#define BATTERY_INTERFACE "org.bluez.Battery1"
+
+#define BATT_UUID16 0x180f
+
+enum {
+	BATTERY_LEVEL,
+	BATTERY_POWER_STATE
+};
+
+/* Generic Attribute/Access Service */
+struct batt {
+	char *path; /* D-Bus path of device */
+	struct btd_device *device;
+	struct gatt_db *db;
+	struct bt_gatt_client *client;
+	struct gatt_db_attribute *attr;
+
+	unsigned int batt_level_cb_id;
+	uint16_t batt_level_io_handle;
+
+	unsigned int batt_power_state_cb_id;
+	uint16_t batt_power_state_io_handle;
+
+	bool present;
+	bool rechargeable;
+	guint16 percentage;
+	const char *state;
+	const char *battery_level;
+};
+
+static void batt_free(struct batt *batt)
+{
+	gatt_db_unref(batt->db);
+	bt_gatt_client_unref(batt->client);
+	btd_device_unref(batt->device);
+	g_free(batt);
+}
+
+static void parse_battery_level(struct batt *batt,
+				const uint8_t *value)
+{
+	uint8_t percentage;
+
+	if (batt->present == false) {
+		batt->present = true;
+		g_dbus_emit_property_changed(btd_get_dbus_connection(), batt->path,
+						BATTERY_INTERFACE, "Present");
+	}
+
+	percentage = value[0];
+	if (batt->percentage != percentage) {
+		batt->percentage = percentage;
+		DBG("Battery Level updated: %d%%", percentage);
+		g_dbus_emit_property_changed(btd_get_dbus_connection(), batt->path,
+						BATTERY_INTERFACE, "Percentage");
+	}
+}
+
+static void parse_battery_power_state(struct batt *batt,
+					const uint8_t *value)
+{
+	guint8 batt_presence;
+	guint8 discharge_state;
+	guint8 charge_state;
+	guint8 battery_level;
+	bool present;
+	bool rechargeable;
+	const char *state;
+	const char *battery_level_s;
+
+	/* Values explained at:
+	 * https://www.bluetooth.com/specifications/gatt/viewer?attributeXmlFile=org.bluetooth.characteristic.battery_power_state.xml */
+	batt_presence = value[0] & 0b11;
+	discharge_state = (value[0] >> 2) & 0b11;
+	charge_state = (value[0] >> 4) & 0b11;
+	battery_level = (value[0] >> 6) & 0b11;
+
+	/* Transform the attribute values into something consumable by UPower
+	 * The string values are a subset of upower's up-types.c */
+
+	if (batt_presence == 3) {
+		present = true;
+	} else {
+		present = false;
+		rechargeable = false;
+		state = "unknown";
+		battery_level_s = "unknown";
+		goto out;
+	}
+
+	rechargeable = !(charge_state == 1);
+
+	if (discharge_state == 3)
+		state = "discharging";
+	else if (charge_state == 3)
+		state = "charging";
+	else
+		state = "fully-charged";
+
+	if (battery_level == 2)
+		battery_level_s = "normal";
+	else if (battery_level == 3)
+		battery_level_s = "critical";
+	else
+		battery_level_s = "unknown";
+
+out:
+	if (present != batt->present) {
+		batt->present = present;
+		g_dbus_emit_property_changed(btd_get_dbus_connection(), batt->path,
+						BATTERY_INTERFACE, "Present");
+	}
+	if (rechargeable != batt->rechargeable) {
+		batt->rechargeable = rechargeable;
+		g_dbus_emit_property_changed(btd_get_dbus_connection(), batt->path,
+						BATTERY_INTERFACE, "Rechargeable");
+	}
+	if (g_strcmp0(state, batt->state) != 0) {
+		batt->state = state;
+		g_dbus_emit_property_changed(btd_get_dbus_connection(), batt->path,
+						BATTERY_INTERFACE, "State");
+	}
+	if (g_strcmp0(battery_level_s, batt->battery_level) != 0) {
+		batt->battery_level = battery_level_s;
+		g_dbus_emit_property_changed(btd_get_dbus_connection(), batt->path,
+						BATTERY_INTERFACE, "BatteryLevel");
+	}
+
+	DBG("Power State 0x%X computed to:", value[0]);
+	DBG("present: %s rechargeable: %s state: %s battery_level: %s",
+			present ? "true" : "false",
+			rechargeable ? "true" : "false",
+			state, battery_level_s);
+}
+
+static void batt_io_value_cb(uint16_t value_handle, const uint8_t *value,
+                             uint16_t length, void *user_data)
+{
+	struct batt *batt = user_data;
+
+	if (value_handle == batt->batt_level_io_handle) {
+		parse_battery_level(batt, value);
+	} else if (value_handle == batt->batt_power_state_io_handle) {
+		parse_battery_power_state(batt, value);
+	} else {
+		g_assert_not_reached();
+	}
+}
+
+static void batt_io_ccc_written_cb(uint16_t att_ecode, void *user_data)
+{
+	guint char_type = GPOINTER_TO_UINT(user_data);
+
+	if (att_ecode != 0) {
+		if (char_type == BATTERY_LEVEL) {
+			error("Battery Level: notifications not enabled %s",
+				att_ecode2str(att_ecode));
+		} else if (char_type == BATTERY_POWER_STATE) {
+			error("Battery Power State: notifications not enabled %s",
+				att_ecode2str(att_ecode));
+		} else {
+			g_assert_not_reached();
+		}
+		return;
+	}
+
+	DBG("Battery Level: notification enabled");
+}
+
+static void read_initial_battery_power_state_cb(bool success,
+							uint8_t att_ecode,
+							const uint8_t *value,
+							uint16_t length,
+							void *user_data)
+{
+	struct batt *batt = user_data;
+
+	if (!success) {
+		DBG("Reading battery power state failed with ATT errror: %u",
+								att_ecode);
+		return;
+	}
+
+	if (!length)
+		return;
+
+	parse_battery_power_state(batt, value);
+
+	/* request notify */
+	batt->batt_power_state_cb_id =
+		bt_gatt_client_register_notify(batt->client,
+		                               batt->batt_power_state_io_handle,
+		                               batt_io_ccc_written_cb,
+		                               batt_io_value_cb,
+		                               batt,
+		                               GUINT_TO_POINTER(BATTERY_POWER_STATE));
+}
+
+static void read_initial_battery_level_cb(bool success,
+						uint8_t att_ecode,
+						const uint8_t *value,
+						uint16_t length,
+						void *user_data)
+{
+	struct batt *batt = user_data;
+
+	if (!success) {
+		DBG("Reading battery level failed with ATT errror: %u",
+								att_ecode);
+		return;
+	}
+
+	if (!length)
+		return;
+
+	parse_battery_level(batt, value);
+
+	/* request notify */
+	batt->batt_level_cb_id =
+		bt_gatt_client_register_notify(batt->client,
+		                               batt->batt_level_io_handle,
+		                               batt_io_ccc_written_cb,
+		                               batt_io_value_cb,
+		                               batt,
+		                               GUINT_TO_POINTER(BATTERY_LEVEL));
+}
+
+static void handle_battery_level(struct batt *batt, uint16_t value_handle)
+{
+	batt->batt_level_io_handle = value_handle;
+
+	if (!bt_gatt_client_read_value(batt->client, batt->batt_level_io_handle,
+						read_initial_battery_level_cb, batt, NULL))
+		DBG("Failed to send request to read battery level");
+}
+
+static void handle_battery_power_state(struct batt *batt, uint16_t value_handle)
+{
+	batt->batt_power_state_io_handle = value_handle;
+
+	if (!bt_gatt_client_read_value(batt->client, batt->batt_power_state_io_handle,
+						read_initial_battery_power_state_cb, batt, NULL))
+		DBG("Failed to send request to read battery power state");
+}
+
+static bool uuid_cmp(uint16_t u16, const bt_uuid_t *uuid)
+{
+	bt_uuid_t lhs;
+
+	bt_uuid16_create(&lhs, u16);
+
+	return bt_uuid_cmp(&lhs, uuid) == 0;
+}
+
+static void handle_characteristic(struct gatt_db_attribute *attr,
+								void *user_data)
+{
+	struct batt *batt = user_data;
+	uint16_t value_handle;
+	bt_uuid_t uuid;
+
+	if (!gatt_db_attribute_get_char_data(attr, NULL, &value_handle, NULL,
+								NULL, &uuid)) {
+		error("Failed to obtain characteristic data");
+		return;
+	}
+
+	if (uuid_cmp(GATT_CHARAC_BATTERY_LEVEL, &uuid))
+		handle_battery_level(batt, value_handle);
+	else if (uuid_cmp(GATT_CHARAC_BATTERY_POWER_STATE, &uuid))
+		handle_battery_power_state(batt, value_handle);
+	else {
+		char uuid_str[MAX_LEN_UUID_STR];
+
+		bt_uuid_to_string(&uuid, uuid_str, sizeof(uuid_str));
+		DBG("Unsupported characteristic: %s", uuid_str);
+	}
+}
+
+static void handle_batt_service(struct batt *batt)
+{
+	gatt_db_service_foreach_char(batt->attr, handle_characteristic, batt);
+}
+
+static int batt_probe(struct btd_service *service)
+{
+	struct btd_device *device = btd_service_get_device(service);
+	struct batt *batt = btd_service_get_user_data(service);
+	char addr[18];
+
+	ba2str(device_get_address(device), addr);
+	DBG("BATT profile probe (%s)", addr);
+
+	/* Ignore, if we were probed for this device already */
+	if (batt) {
+		error("Profile probed twice for the same device!");
+		return -1;
+	}
+
+	batt = g_new0(struct batt, 1);
+	if (!batt)
+		return -1;
+
+	batt->percentage = -1;
+	batt->device = btd_device_ref(device);
+	btd_service_set_user_data(service, batt);
+
+	return 0;
+}
+
+static void batt_remove(struct btd_service *service)
+{
+	struct btd_device *device = btd_service_get_device(service);
+	struct batt *batt;
+	char addr[18];
+
+	ba2str(device_get_address(device), addr);
+	DBG("BATT profile remove (%s)", addr);
+
+	batt = btd_service_get_user_data(service);
+	if (!batt) {
+		error("BATT service not handled by profile");
+		return;
+	}
+
+	batt_free(batt);
+}
+
+static void foreach_batt_service(struct gatt_db_attribute *attr, void *user_data)
+{
+	struct batt *batt = user_data;
+
+	if (batt->attr) {
+		error("More than one BATT service exists for this device");
+		return;
+	}
+
+	batt->attr = attr;
+	handle_batt_service(batt);
+}
+
+static void batt_reset(struct batt *batt)
+{
+	batt->attr = NULL;
+	gatt_db_unref(batt->db);
+	batt->db = NULL;
+	bt_gatt_client_unregister_notify(batt->client, batt->batt_power_state_cb_id);
+	bt_gatt_client_unregister_notify(batt->client, batt->batt_level_cb_id);
+	bt_gatt_client_cancel_all(batt->client);
+	bt_gatt_client_unref(batt->client);
+	batt->client = NULL;
+	if (batt->path) {
+		g_dbus_unregister_interface(btd_get_dbus_connection(),
+					    batt->path, BATTERY_INTERFACE);
+		g_free(batt->path);
+		batt->path = NULL;
+	}
+	btd_device_unref(batt->device);
+}
+
+static gboolean property_get_battery_level(
+					const GDBusPropertyTable *property,
+					DBusMessageIter *iter, void *data)
+{
+	struct batt *batt = data;
+	const char *empty_str = "";
+
+	if (!batt->battery_level)
+		dbus_message_iter_append_basic(iter, DBUS_TYPE_STRING, &empty_str);
+	else
+		dbus_message_iter_append_basic(iter, DBUS_TYPE_STRING, &batt->battery_level);
+
+	return TRUE;
+}
+
+static gboolean property_get_state(
+					const GDBusPropertyTable *property,
+					DBusMessageIter *iter, void *data)
+{
+	struct batt *batt = data;
+	const char *empty_str = "";
+
+	if (!batt->state)
+		dbus_message_iter_append_basic(iter, DBUS_TYPE_STRING, &empty_str);
+	else
+		dbus_message_iter_append_basic(iter, DBUS_TYPE_STRING, &batt->state);
+
+	return TRUE;
+}
+
+static gboolean property_get_percentage(
+					const GDBusPropertyTable *property,
+					DBusMessageIter *iter, void *data)
+{
+	struct batt *batt = data;
+
+	dbus_message_iter_append_basic(iter, DBUS_TYPE_UINT16, &batt->percentage);
+
+	return TRUE;
+}
+
+static gboolean property_get_rechargeable(
+					const GDBusPropertyTable *property,
+					DBusMessageIter *iter, void *data)
+{
+	struct batt *batt = data;
+	dbus_bool_t rechargeable;
+
+	rechargeable = batt->rechargeable ? TRUE : FALSE;
+	dbus_message_iter_append_basic(iter, DBUS_TYPE_BOOLEAN, &rechargeable);
+
+	return TRUE;
+}
+
+static gboolean property_get_present(
+					const GDBusPropertyTable *property,
+					DBusMessageIter *iter, void *data)
+{
+	struct batt *batt = data;
+	dbus_bool_t present;
+
+	present = batt->present ? TRUE : FALSE;
+	dbus_message_iter_append_basic(iter, DBUS_TYPE_BOOLEAN, &present);
+
+	return TRUE;
+}
+
+static const GDBusPropertyTable battery_properties[] = {
+	{ "Present", "b", property_get_present },
+	{ "Rechargeable", "b", property_get_rechargeable },
+	{ "Percentage", "q", property_get_percentage },
+	{ "State", "s", property_get_state },
+	{ "BatteryLevel", "s", property_get_battery_level },
+	{ }
+};
+
+static int batt_accept(struct btd_service *service)
+{
+	struct btd_device *device = btd_service_get_device(service);
+	struct gatt_db *db = btd_device_get_gatt_db(device);
+	struct bt_gatt_client *client = btd_device_get_gatt_client(device);
+	struct batt *batt = btd_service_get_user_data(service);
+	char addr[18];
+	bt_uuid_t batt_uuid;
+
+	ba2str(device_get_address(device), addr);
+	DBG("BATT profile accept (%s)", addr);
+
+	if (!batt) {
+		error("BATT service not handled by profile");
+		return -1;
+	}
+
+	batt->db = gatt_db_ref(db);
+	batt->client = bt_gatt_client_clone(client);
+
+	/* Handle the BATT services */
+	bt_uuid16_create(&batt_uuid, BATT_UUID16);
+	gatt_db_foreach_service(db, &batt_uuid, foreach_batt_service, batt);
+
+	if (!batt->attr) {
+		error("BATT attribute not found");
+		batt_reset(batt);
+		return -1;
+	}
+
+	batt->path = g_strdup (device_get_path(device));
+
+	if (g_dbus_register_interface(btd_get_dbus_connection(),
+					batt->path, BATTERY_INTERFACE,
+					NULL, NULL,
+					battery_properties, batt,
+					NULL) == FALSE) {
+		error("Unable to register %s interface for %s",
+			BATTERY_INTERFACE, batt->path);
+		batt_reset(batt);
+		return -EINVAL;
+	}
+
+	btd_service_connecting_complete(service, 0);
+
+	return 0;
+}
+
+static int batt_disconnect(struct btd_service *service)
+{
+	struct batt *batt = btd_service_get_user_data(service);
+
+	batt_reset(batt);
+
+	btd_service_disconnecting_complete(service, 0);
+
+	return 0;
+}
+
+static struct btd_profile batt_profile = {
+	.name		= "batt-profile",
+	.remote_uuid	= BATTERY_UUID,
+	.device_probe	= batt_probe,
+	.device_remove	= batt_remove,
+	.accept		= batt_accept,
+	.disconnect	= batt_disconnect,
+};
+
+static int batt_init(void)
+{
+	return btd_profile_register(&batt_profile);
+}
+
+static void batt_exit(void)
+{
+	btd_profile_unregister(&batt_profile);
+}
+
+BLUETOOTH_PLUGIN_DEFINE(battery, VERSION, BLUETOOTH_PLUGIN_PRIORITY_DEFAULT,
+							batt_init, batt_exit)
-- 
2.14.2

[PATCH v2 3/3] profiles/battery: Remove unused bas.[ch]

[Bastien Nocera]

The Battery Service characteristics are not children of the HoG
characteristics, so there's nothing to consume in the input profile.
Remove that code which duplicated most of the battery profile plugin.
---
 Makefile.am              |   1 -
 Makefile.plugins         |   1 -
 android/Android.mk       |   1 -
 android/Makefile.am      |   1 -
 profiles/battery/bas.c   | 340 -----------------------------------------------
 profiles/battery/bas.h   |  32 -----
 profiles/input/hog-lib.c |  25 +---
 7 files changed, 1 insertion(+), 400 deletions(-)
 delete mode 100644 profiles/battery/bas.c
 delete mode 100644 profiles/battery/bas.h

diff --git a/Makefile.am b/Makefile.am
index 8faabf44b..46358c7e9 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -404,7 +404,6 @@ unit_test_hog_SOURCES = unit/test-hog.c \
 			$(btio_sources) \
 			profiles/input/hog-lib.h profiles/input/hog-lib.c \
 			profiles/scanparam/scpp.h profiles/scanparam/scpp.c \
-			profiles/battery/bas.h profiles/battery/bas.c \
 			profiles/deviceinfo/dis.h profiles/deviceinfo/dis.c \
 			src/log.h src/log.c \
 			attrib/att.h attrib/att.c \
diff --git a/Makefile.plugins b/Makefile.plugins
index 1f3b5b552..7416ad6bf 100644
--- a/Makefile.plugins
+++ b/Makefile.plugins
@@ -66,7 +66,6 @@ builtin_modules += hog
 builtin_sources += profiles/input/hog.c profiles/input/uhid_copy.h \
 			profiles/input/hog-lib.c profiles/input/hog-lib.h \
 			profiles/deviceinfo/dis.c profiles/deviceinfo/dis.h \
-			profiles/battery/bas.c profiles/battery/bas.h \
 			profiles/scanparam/scpp.c profiles/scanparam/scpp.h \
 			profiles/input/suspend.h profiles/input/suspend-none.c
 
diff --git a/android/Android.mk b/android/Android.mk
index 38ef4aa97..7b70d0129 100644
--- a/android/Android.mk
+++ b/android/Android.mk
@@ -41,7 +41,6 @@ LOCAL_SRC_FILES := \
 	bluez/android/bluetooth.c \
 	bluez/profiles/scanparam/scpp.c \
 	bluez/profiles/deviceinfo/dis.c \
-	bluez/profiles/battery/bas.c \
 	bluez/profiles/input/hog-lib.c \
 	bluez/android/hidhost.c \
 	bluez/android/socket.c \
diff --git a/android/Makefile.am b/android/Makefile.am
index 154f8db56..35d6e1e9d 100644
--- a/android/Makefile.am
+++ b/android/Makefile.am
@@ -32,7 +32,6 @@ android_bluetoothd_SOURCES = android/main.c \
 				profiles/scanparam/scpp.c \
 				profiles/deviceinfo/dis.h \
 				profiles/deviceinfo/dis.c \
-				profiles/battery/bas.h profiles/battery/bas.c \
 				profiles/input/hog-lib.h \
 				profiles/input/hog-lib.c \
 				android/ipc-common.h \
diff --git a/profiles/battery/bas.c b/profiles/battery/bas.c
deleted file mode 100644
index de369fd3c..000000000
--- a/profiles/battery/bas.c
+++ /dev/null
@@ -1,340 +0,0 @@
-/*
- *
- *  BlueZ - Bluetooth protocol stack for Linux
- *
- *  Copyright (C) 2014  Intel Corporation. All rights reserved.
- *
- *
- *  This library is free software; you can rebastribute it and/or
- *  modify it under the terms of the GNU Lesser General Public
- *  License as published by the Free Software Foundation; either
- *  version 2.1 of the License, or (at your option) any later version.
- *
- *  This library is bastributed in the hope that it will be useful,
- *  but WITHOUT ANY WARRANTY; without even the implied warranty of
- *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
- *  Lesser General Public License for more details.
- *
- *  You should have received a copy of the GNU Lesser General Public
- *  License along with this library; if not, write to the Free Software
- *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
- *
- */
-
-#ifdef HAVE_CONFIG_H
-#include <config.h>
-#endif
-
-#include <stdbool.h>
-#include <errno.h>
-
-#include <glib.h>
-
-#include "src/log.h"
-
-#include "lib/bluetooth.h"
-#include "lib/sdp.h"
-#include "lib/uuid.h"
-
-#include "src/shared/util.h"
-#include "src/shared/queue.h"
-
-#include "attrib/gattrib.h"
-#include "attrib/att.h"
-#include "attrib/gatt.h"
-
-#include "profiles/battery/bas.h"
-
-#define ATT_NOTIFICATION_HEADER_SIZE 3
-#define ATT_READ_RESPONSE_HEADER_SIZE 1
-
-struct bt_bas {
-	int ref_count;
-	GAttrib *attrib;
-	struct gatt_primary *primary;
-	uint16_t handle;
-	uint16_t ccc_handle;
-	guint id;
-	struct queue *gatt_op;
-};
-
-struct gatt_request {
-	unsigned int id;
-	struct bt_bas *bas;
-	void *user_data;
-};
-
-static void destroy_gatt_req(struct gatt_request *req)
-{
-	queue_remove(req->bas->gatt_op, req);
-	bt_bas_unref(req->bas);
-	free(req);
-}
-
-static void bas_free(struct bt_bas *bas)
-{
-	bt_bas_detach(bas);
-
-	g_free(bas->primary);
-	queue_destroy(bas->gatt_op, (void *) destroy_gatt_req);
-	free(bas);
-}
-
-struct bt_bas *bt_bas_new(void *primary)
-{
-	struct bt_bas *bas;
-
-	bas = new0(struct bt_bas, 1);
-	bas->gatt_op = queue_new();
-
-	if (primary)
-		bas->primary = g_memdup(primary, sizeof(*bas->primary));
-
-	return bt_bas_ref(bas);
-}
-
-struct bt_bas *bt_bas_ref(struct bt_bas *bas)
-{
-	if (!bas)
-		return NULL;
-
-	__sync_fetch_and_add(&bas->ref_count, 1);
-
-	return bas;
-}
-
-void bt_bas_unref(struct bt_bas *bas)
-{
-	if (!bas)
-		return;
-
-	if (__sync_sub_and_fetch(&bas->ref_count, 1))
-		return;
-
-	bas_free(bas);
-}
-
-static struct gatt_request *create_request(struct bt_bas *bas,
-							void *user_data)
-{
-	struct gatt_request *req;
-
-	req = new0(struct gatt_request, 1);
-	req->user_data = user_data;
-	req->bas = bt_bas_ref(bas);
-
-	return req;
-}
-
-static void set_and_store_gatt_req(struct bt_bas *bas,
-						struct gatt_request *req,
-						unsigned int id)
-{
-	req->id = id;
-	queue_push_head(bas->gatt_op, req);
-}
-
-static void write_char(struct bt_bas *bas, GAttrib *attrib, uint16_t handle,
-					const uint8_t *value, size_t vlen,
-					GAttribResultFunc func,
-					gpointer user_data)
-{
-	struct gatt_request *req;
-	unsigned int id;
-
-	req = create_request(bas, user_data);
-
-	id = gatt_write_char(attrib, handle, value, vlen, func, req);
-
-	set_and_store_gatt_req(bas, req, id);
-}
-
-static void read_char(struct bt_bas *bas, GAttrib *attrib, uint16_t handle,
-				GAttribResultFunc func, gpointer user_data)
-{
-	struct gatt_request *req;
-	unsigned int id;
-
-	req = create_request(bas, user_data);
-
-	id = gatt_read_char(attrib, handle, func, req);
-
-	set_and_store_gatt_req(bas, req, id);
-}
-
-static void discover_char(struct bt_bas *bas, GAttrib *attrib,
-						uint16_t start, uint16_t end,
-						bt_uuid_t *uuid, gatt_cb_t func,
-						gpointer user_data)
-{
-	struct gatt_request *req;
-	unsigned int id;
-
-	req = create_request(bas, user_data);
-
-	id = gatt_discover_char(attrib, start, end, uuid, func, req);
-
-	set_and_store_gatt_req(bas, req, id);
-}
-
-static void discover_desc(struct bt_bas *bas, GAttrib *attrib,
-				uint16_t start, uint16_t end, bt_uuid_t *uuid,
-				gatt_cb_t func, gpointer user_data)
-{
-	struct gatt_request *req;
-	unsigned int id;
-
-	req = create_request(bas, user_data);
-
-	id = gatt_discover_desc(attrib, start, end, uuid, func, req);
-	set_and_store_gatt_req(bas, req, id);
-}
-
-static void notification_cb(const guint8 *pdu, guint16 len, gpointer user_data)
-{
-	DBG("Battery Level at %u", pdu[ATT_NOTIFICATION_HEADER_SIZE]);
-}
-
-static void read_value_cb(guint8 status, const guint8 *pdu, guint16 len,
-					gpointer user_data)
-{
-	DBG("Battery Level at %u", pdu[ATT_READ_RESPONSE_HEADER_SIZE]);
-}
-
-static void ccc_written_cb(guint8 status, const guint8 *pdu,
-					guint16 plen, gpointer user_data)
-{
-	struct gatt_request *req = user_data;
-	struct bt_bas *bas = req->user_data;
-
-	destroy_gatt_req(req);
-
-	if (status != 0) {
-		error("Write Scan Refresh CCC failed: %s",
-						att_ecode2str(status));
-		return;
-	}
-
-	DBG("Battery Level: notification enabled");
-
-	bas->id = g_attrib_register(bas->attrib, ATT_OP_HANDLE_NOTIFY,
-					bas->handle, notification_cb, bas,
-					NULL);
-}
-
-static void write_ccc(struct bt_bas *bas, GAttrib *attrib, uint16_t handle,
-							void *user_data)
-{
-	uint8_t value[2];
-
-	put_le16(GATT_CLIENT_CHARAC_CFG_NOTIF_BIT, value);
-
-	write_char(bas, attrib, handle, value, sizeof(value), ccc_written_cb,
-								user_data);
-}
-
-static void ccc_read_cb(guint8 status, const guint8 *pdu, guint16 len,
-							gpointer user_data)
-{
-	struct gatt_request *req = user_data;
-	struct bt_bas *bas = req->user_data;
-
-	destroy_gatt_req(req);
-
-	if (status != 0) {
-		error("Error reading CCC value: %s", att_ecode2str(status));
-		return;
-	}
-
-	write_ccc(bas, bas->attrib, bas->ccc_handle, bas);
-}
-
-static void discover_descriptor_cb(uint8_t status, GSList *descs,
-								void *user_data)
-{
-	struct gatt_request *req = user_data;
-	struct bt_bas *bas = req->user_data;
-	struct gatt_desc *desc;
-
-	destroy_gatt_req(req);
-
-	if (status != 0) {
-		error("Discover descriptors failed: %s", att_ecode2str(status));
-		return;
-	}
-
-	/* There will be only one descriptor on list and it will be CCC */
-	desc = descs->data;
-	bas->ccc_handle = desc->handle;
-
-	read_char(bas, bas->attrib, desc->handle, ccc_read_cb, bas);
-}
-
-static void bas_discovered_cb(uint8_t status, GSList *chars, void *user_data)
-{
-	struct gatt_request *req = user_data;
-	struct bt_bas *bas = req->user_data;
-	struct gatt_char *chr;
-	uint16_t start, end;
-	bt_uuid_t uuid;
-
-	destroy_gatt_req(req);
-
-	if (status) {
-		error("Battery: %s", att_ecode2str(status));
-		return;
-	}
-
-	chr = chars->data;
-	bas->handle = chr->value_handle;
-
-	DBG("Battery handle: 0x%04x", bas->handle);
-
-	read_char(bas, bas->attrib, bas->handle, read_value_cb, bas);
-
-	start = chr->value_handle + 1;
-	end = bas->primary->range.end;
-
-	bt_uuid16_create(&uuid, GATT_CLIENT_CHARAC_CFG_UUID);
-
-	discover_desc(bas, bas->attrib, start, end, &uuid,
-						discover_descriptor_cb, bas);
-}
-
-bool bt_bas_attach(struct bt_bas *bas, void *attrib)
-{
-	if (!bas || bas->attrib || !bas->primary)
-		return false;
-
-	bas->attrib = g_attrib_ref(attrib);
-
-	if (bas->handle > 0)
-		return true;
-
-	discover_char(bas, bas->attrib, bas->primary->range.start,
-					bas->primary->range.end, NULL,
-					bas_discovered_cb, bas);
-
-	return true;
-}
-
-static void cancel_gatt_req(struct gatt_request *req)
-{
-	if (g_attrib_cancel(req->bas->attrib, req->id))
-		destroy_gatt_req(req);
-}
-
-void bt_bas_detach(struct bt_bas *bas)
-{
-	if (!bas || !bas->attrib)
-		return;
-
-	if (bas->id > 0) {
-		g_attrib_unregister(bas->attrib, bas->id);
-		bas->id = 0;
-	}
-
-	queue_foreach(bas->gatt_op, (void *) cancel_gatt_req, NULL);
-	g_attrib_unref(bas->attrib);
-	bas->attrib = NULL;
-}
diff --git a/profiles/battery/bas.h b/profiles/battery/bas.h
deleted file mode 100644
index 3e175b5b5..000000000
--- a/profiles/battery/bas.h
+++ /dev/null
@@ -1,32 +0,0 @@
-/*
- *
- *  BlueZ - Bluetooth protocol stack for Linux
- *
- *  Copyright (C) 2014  Intel Corporation. All rights reserved.
- *
- *
- *  This library is free software; you can rebastribute it and/or
- *  modify it under the terms of the GNU Lesser General Public
- *  License as published by the Free Software Foundation; either
- *  version 2.1 of the License, or (at your option) any later version.
- *
- *  This library is bastributed in the hope that it will be useful,
- *  but WITHOUT ANY WARRANTY; without even the implied warranty of
- *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
- *  Lesser General Public License for more details.
- *
- *  You should have received a copy of the GNU Lesser General Public
- *  License along with this library; if not, write to the Free Software
- *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
- *
- */
-
-struct bt_bas;
-
-struct bt_bas *bt_bas_new(void *primary);
-
-struct bt_bas *bt_bas_ref(struct bt_bas *bas);
-void bt_bas_unref(struct bt_bas *bas);
-
-bool bt_bas_attach(struct bt_bas *bas, void *gatt);
-void bt_bas_detach(struct bt_bas *bas);
diff --git a/profiles/input/hog-lib.c b/profiles/input/hog-lib.c
index dab385fe7..7b0097cb6 100644
--- a/profiles/input/hog-lib.c
+++ b/profiles/input/hog-lib.c
@@ -57,7 +57,6 @@
 
 #include "profiles/scanparam/scpp.h"
 #include "profiles/deviceinfo/dis.h"
-#include "profiles/battery/bas.h"
 #include "profiles/input/hog-lib.h"
 
 #define HOG_UUID		"00001812-0000-1000-8000-00805f9b34fb"
@@ -105,7 +104,6 @@ struct bt_hog {
 	uint16_t		setrep_id;
 	struct bt_scpp		*scpp;
 	struct bt_dis		*dis;
-	struct queue		*bas;
 	GSList			*instances;
 	struct queue		*gatt_op;
 };
@@ -1174,7 +1172,6 @@ static void hog_free(void *data)
 
 	bt_hog_detach(hog);
 
-	queue_destroy(hog->bas, (void *) bt_bas_unref);
 	g_slist_free_full(hog->instances, hog_free);
 
 	bt_scpp_unref(hog->scpp);
@@ -1323,7 +1320,6 @@ static struct bt_hog *hog_new(int fd, const char *name, uint16_t vendor,
 		return NULL;
 
 	hog->gatt_op = queue_new();
-	hog->bas = queue_new();
 
 	if (fd < 0)
 		hog->uhid = bt_uhid_new_default();
@@ -1332,7 +1328,7 @@ static struct bt_hog *hog_new(int fd, const char *name, uint16_t vendor,
 
 	hog->uhid_fd = fd;
 
-	if (!hog->gatt_op || !hog->bas || !hog->uhid) {
+	if (!hog->gatt_op || !hog->uhid) {
 		hog_free(hog);
 		return NULL;
 	}
@@ -1483,16 +1479,6 @@ static void hog_attach_dis(struct bt_hog *hog, struct gatt_primary *primary)
 	}
 }
 
-static void hog_attach_bas(struct bt_hog *hog, struct gatt_primary *primary)
-{
-	struct bt_bas *instance;
-
-	instance = bt_bas_new(primary);
-
-	bt_bas_attach(instance, hog->attrib);
-	queue_push_head(hog->bas, instance);
-}
-
 static void hog_attach_hog(struct bt_hog *hog, struct gatt_primary *primary)
 {
 	struct bt_hog *instance;
@@ -1555,11 +1541,6 @@ static void primary_cb(uint8_t status, GSList *services, void *user_data)
 			continue;
 		}
 
-		if (strcmp(primary->uuid, BATTERY_UUID) == 0) {
-			hog_attach_bas(hog, primary);
-			continue;
-		}
-
 		if (strcmp(primary->uuid, HOG_UUID) == 0)
 			hog_attach_hog(hog, primary);
 	}
@@ -1585,8 +1566,6 @@ bool bt_hog_attach(struct bt_hog *hog, void *gatt)
 	if (hog->dis)
 		bt_dis_attach(hog->dis, gatt);
 
-	queue_foreach(hog->bas, (void *) bt_bas_attach, gatt);
-
 	for (l = hog->instances; l; l = l->next) {
 		struct bt_hog *instance = l->data;
 
@@ -1625,8 +1604,6 @@ void bt_hog_detach(struct bt_hog *hog)
 	if (!hog->attrib)
 		return;
 
-	queue_foreach(hog->bas, (void *) bt_bas_detach, NULL);
-
 	for (l = hog->instances; l; l = l->next) {
 		struct bt_hog *instance = l->data;
 
-- 
2.14.2

Re: [PATCH v2 2/3] profiles/battery: Add Bluetooth LE Battery service

[Bastien Nocera]

On Mon, 2017-10-23 at 17:48 +0200, Bastien Nocera wrote:
> Only the Battery Level characteristic was tested with a
> Microsoft Arc Touch Mouse SE.
> 
> This patch however hopes to support both the Battery Level and
> the Battery Power State characteristics.

Interdiff for this one, fixing the "Connected" state not changing:

diff --git a/profiles/battery/battery.c b/profiles/battery/battery.c
index 8e967e159..ab4603de3 100644
--- a/profiles/battery/battery.c
+++ b/profiles/battery/battery.c
@@ -400,6 +400,7 @@ static void batt_reset(struct batt *batt)
        batt->db = NULL;
        bt_gatt_client_unregister_notify(batt->client, batt->batt_power_state_cb_id);
        bt_gatt_client_unregister_notify(batt->client, batt->batt_level_cb_id);
+       bt_gatt_client_cancel_all(batt->client);
        bt_gatt_client_unref(batt->client);
        batt->client = NULL;
        if (batt->path) {
@@ -408,6 +409,7 @@ static void batt_reset(struct batt *batt)
                g_free(batt->path);
                batt->path = NULL;
        }
+       btd_device_unref(batt->device);
 }
 
 static gboolean property_get_battery_level(

Re: [PATCH v2 3/3] profiles/battery: Remove unused bas.[ch]

[Luiz Augusto von Dentz]

Hi Bastian,

On Mon, Oct 23, 2017 at 6:48 PM, Bastien Nocera <hadess@hadess.net> wrote:
> The Battery Service characteristics are not children of the HoG
> characteristics, so there's nothing to consume in the input profile.
> Remove that code which duplicated most of the battery profile plugin.

Id rather not do this, we actually do intend the maintain a small
helper library that can be unit tested, what bt_bas lacks is just
proper support of bt_gatt_client nothing else. Also for the record
this would probably break BfA HoG qualification which requires battery
service to be supported as well, Android code does not have any other
bas implementation.

> ---
>  Makefile.am              |   1 -
>  Makefile.plugins         |   1 -
>  android/Android.mk       |   1 -
>  android/Makefile.am      |   1 -
>  profiles/battery/bas.c   | 340 -----------------------------------------------
>  profiles/battery/bas.h   |  32 -----
>  profiles/input/hog-lib.c |  25 +---
>  7 files changed, 1 insertion(+), 400 deletions(-)
>  delete mode 100644 profiles/battery/bas.c
>  delete mode 100644 profiles/battery/bas.h
>
> diff --git a/Makefile.am b/Makefile.am
> index 8faabf44b..46358c7e9 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -404,7 +404,6 @@ unit_test_hog_SOURCES = unit/test-hog.c \
>                         $(btio_sources) \
>                         profiles/input/hog-lib.h profiles/input/hog-lib.c \
>                         profiles/scanparam/scpp.h profiles/scanparam/scpp.c \
> -                       profiles/battery/bas.h profiles/battery/bas.c \
>                         profiles/deviceinfo/dis.h profiles/deviceinfo/dis.c \
>                         src/log.h src/log.c \
>                         attrib/att.h attrib/att.c \
> diff --git a/Makefile.plugins b/Makefile.plugins
> index 1f3b5b552..7416ad6bf 100644
> --- a/Makefile.plugins
> +++ b/Makefile.plugins
> @@ -66,7 +66,6 @@ builtin_modules += hog
>  builtin_sources += profiles/input/hog.c profiles/input/uhid_copy.h \
>                         profiles/input/hog-lib.c profiles/input/hog-lib.h \
>                         profiles/deviceinfo/dis.c profiles/deviceinfo/dis.h \
> -                       profiles/battery/bas.c profiles/battery/bas.h \
>                         profiles/scanparam/scpp.c profiles/scanparam/scpp.h \
>                         profiles/input/suspend.h profiles/input/suspend-none.c
>
> diff --git a/android/Android.mk b/android/Android.mk
> index 38ef4aa97..7b70d0129 100644
> --- a/android/Android.mk
> +++ b/android/Android.mk
> @@ -41,7 +41,6 @@ LOCAL_SRC_FILES := \
>         bluez/android/bluetooth.c \
>         bluez/profiles/scanparam/scpp.c \
>         bluez/profiles/deviceinfo/dis.c \
> -       bluez/profiles/battery/bas.c \
>         bluez/profiles/input/hog-lib.c \
>         bluez/android/hidhost.c \
>         bluez/android/socket.c \
> diff --git a/android/Makefile.am b/android/Makefile.am
> index 154f8db56..35d6e1e9d 100644
> --- a/android/Makefile.am
> +++ b/android/Makefile.am
> @@ -32,7 +32,6 @@ android_bluetoothd_SOURCES = android/main.c \
>                                 profiles/scanparam/scpp.c \
>                                 profiles/deviceinfo/dis.h \
>                                 profiles/deviceinfo/dis.c \
> -                               profiles/battery/bas.h profiles/battery/bas.c \
>                                 profiles/input/hog-lib.h \
>                                 profiles/input/hog-lib.c \
>                                 android/ipc-common.h \
> diff --git a/profiles/battery/bas.c b/profiles/battery/bas.c
> deleted file mode 100644
> index de369fd3c..000000000
> --- a/profiles/battery/bas.c
> +++ /dev/null
> @@ -1,340 +0,0 @@
> -/*
> - *
> - *  BlueZ - Bluetooth protocol stack for Linux
> - *
> - *  Copyright (C) 2014  Intel Corporation. All rights reserved.
> - *
> - *
> - *  This library is free software; you can rebastribute it and/or
> - *  modify it under the terms of the GNU Lesser General Public
> - *  License as published by the Free Software Foundation; either
> - *  version 2.1 of the License, or (at your option) any later version.
> - *
> - *  This library is bastributed in the hope that it will be useful,
> - *  but WITHOUT ANY WARRANTY; without even the implied warranty of
> - *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> - *  Lesser General Public License for more details.
> - *
> - *  You should have received a copy of the GNU Lesser General Public
> - *  License along with this library; if not, write to the Free Software
> - *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
> - *
> - */
> -
> -#ifdef HAVE_CONFIG_H
> -#include <config.h>
> -#endif
> -
> -#include <stdbool.h>
> -#include <errno.h>
> -
> -#include <glib.h>
> -
> -#include "src/log.h"
> -
> -#include "lib/bluetooth.h"
> -#include "lib/sdp.h"
> -#include "lib/uuid.h"
> -
> -#include "src/shared/util.h"
> -#include "src/shared/queue.h"
> -
> -#include "attrib/gattrib.h"
> -#include "attrib/att.h"
> -#include "attrib/gatt.h"
> -
> -#include "profiles/battery/bas.h"
> -
> -#define ATT_NOTIFICATION_HEADER_SIZE 3
> -#define ATT_READ_RESPONSE_HEADER_SIZE 1
> -
> -struct bt_bas {
> -       int ref_count;
> -       GAttrib *attrib;
> -       struct gatt_primary *primary;
> -       uint16_t handle;
> -       uint16_t ccc_handle;
> -       guint id;
> -       struct queue *gatt_op;
> -};
> -
> -struct gatt_request {
> -       unsigned int id;
> -       struct bt_bas *bas;
> -       void *user_data;
> -};
> -
> -static void destroy_gatt_req(struct gatt_request *req)
> -{
> -       queue_remove(req->bas->gatt_op, req);
> -       bt_bas_unref(req->bas);
> -       free(req);
> -}
> -
> -static void bas_free(struct bt_bas *bas)
> -{
> -       bt_bas_detach(bas);
> -
> -       g_free(bas->primary);
> -       queue_destroy(bas->gatt_op, (void *) destroy_gatt_req);
> -       free(bas);
> -}
> -
> -struct bt_bas *bt_bas_new(void *primary)
> -{
> -       struct bt_bas *bas;
> -
> -       bas = new0(struct bt_bas, 1);
> -       bas->gatt_op = queue_new();
> -
> -       if (primary)
> -               bas->primary = g_memdup(primary, sizeof(*bas->primary));
> -
> -       return bt_bas_ref(bas);
> -}
> -
> -struct bt_bas *bt_bas_ref(struct bt_bas *bas)
> -{
> -       if (!bas)
> -               return NULL;
> -
> -       __sync_fetch_and_add(&bas->ref_count, 1);
> -
> -       return bas;
> -}
> -
> -void bt_bas_unref(struct bt_bas *bas)
> -{
> -       if (!bas)
> -               return;
> -
> -       if (__sync_sub_and_fetch(&bas->ref_count, 1))
> -               return;
> -
> -       bas_free(bas);
> -}
> -
> -static struct gatt_request *create_request(struct bt_bas *bas,
> -                                                       void *user_data)
> -{
> -       struct gatt_request *req;
> -
> -       req = new0(struct gatt_request, 1);
> -       req->user_data = user_data;
> -       req->bas = bt_bas_ref(bas);
> -
> -       return req;
> -}
> -
> -static void set_and_store_gatt_req(struct bt_bas *bas,
> -                                               struct gatt_request *req,
> -                                               unsigned int id)
> -{
> -       req->id = id;
> -       queue_push_head(bas->gatt_op, req);
> -}
> -
> -static void write_char(struct bt_bas *bas, GAttrib *attrib, uint16_t handle,
> -                                       const uint8_t *value, size_t vlen,
> -                                       GAttribResultFunc func,
> -                                       gpointer user_data)
> -{
> -       struct gatt_request *req;
> -       unsigned int id;
> -
> -       req = create_request(bas, user_data);
> -
> -       id = gatt_write_char(attrib, handle, value, vlen, func, req);
> -
> -       set_and_store_gatt_req(bas, req, id);
> -}
> -
> -static void read_char(struct bt_bas *bas, GAttrib *attrib, uint16_t handle,
> -                               GAttribResultFunc func, gpointer user_data)
> -{
> -       struct gatt_request *req;
> -       unsigned int id;
> -
> -       req = create_request(bas, user_data);
> -
> -       id = gatt_read_char(attrib, handle, func, req);
> -
> -       set_and_store_gatt_req(bas, req, id);
> -}
> -
> -static void discover_char(struct bt_bas *bas, GAttrib *attrib,
> -                                               uint16_t start, uint16_t end,
> -                                               bt_uuid_t *uuid, gatt_cb_t func,
> -                                               gpointer user_data)
> -{
> -       struct gatt_request *req;
> -       unsigned int id;
> -
> -       req = create_request(bas, user_data);
> -
> -       id = gatt_discover_char(attrib, start, end, uuid, func, req);
> -
> -       set_and_store_gatt_req(bas, req, id);
> -}
> -
> -static void discover_desc(struct bt_bas *bas, GAttrib *attrib,
> -                               uint16_t start, uint16_t end, bt_uuid_t *uuid,
> -                               gatt_cb_t func, gpointer user_data)
> -{
> -       struct gatt_request *req;
> -       unsigned int id;
> -
> -       req = create_request(bas, user_data);
> -
> -       id = gatt_discover_desc(attrib, start, end, uuid, func, req);
> -       set_and_store_gatt_req(bas, req, id);
> -}
> -
> -static void notification_cb(const guint8 *pdu, guint16 len, gpointer user_data)
> -{
> -       DBG("Battery Level at %u", pdu[ATT_NOTIFICATION_HEADER_SIZE]);
> -}
> -
> -static void read_value_cb(guint8 status, const guint8 *pdu, guint16 len,
> -                                       gpointer user_data)
> -{
> -       DBG("Battery Level at %u", pdu[ATT_READ_RESPONSE_HEADER_SIZE]);
> -}
> -
> -static void ccc_written_cb(guint8 status, const guint8 *pdu,
> -                                       guint16 plen, gpointer user_data)
> -{
> -       struct gatt_request *req = user_data;
> -       struct bt_bas *bas = req->user_data;
> -
> -       destroy_gatt_req(req);
> -
> -       if (status != 0) {
> -               error("Write Scan Refresh CCC failed: %s",
> -                                               att_ecode2str(status));
> -               return;
> -       }
> -
> -       DBG("Battery Level: notification enabled");
> -
> -       bas->id = g_attrib_register(bas->attrib, ATT_OP_HANDLE_NOTIFY,
> -                                       bas->handle, notification_cb, bas,
> -                                       NULL);
> -}
> -
> -static void write_ccc(struct bt_bas *bas, GAttrib *attrib, uint16_t handle,
> -                                                       void *user_data)
> -{
> -       uint8_t value[2];
> -
> -       put_le16(GATT_CLIENT_CHARAC_CFG_NOTIF_BIT, value);
> -
> -       write_char(bas, attrib, handle, value, sizeof(value), ccc_written_cb,
> -                                                               user_data);
> -}
> -
> -static void ccc_read_cb(guint8 status, const guint8 *pdu, guint16 len,
> -                                                       gpointer user_data)
> -{
> -       struct gatt_request *req = user_data;
> -       struct bt_bas *bas = req->user_data;
> -
> -       destroy_gatt_req(req);
> -
> -       if (status != 0) {
> -               error("Error reading CCC value: %s", att_ecode2str(status));
> -               return;
> -       }
> -
> -       write_ccc(bas, bas->attrib, bas->ccc_handle, bas);
> -}
> -
> -static void discover_descriptor_cb(uint8_t status, GSList *descs,
> -                                                               void *user_data)
> -{
> -       struct gatt_request *req = user_data;
> -       struct bt_bas *bas = req->user_data;
> -       struct gatt_desc *desc;
> -
> -       destroy_gatt_req(req);
> -
> -       if (status != 0) {
> -               error("Discover descriptors failed: %s", att_ecode2str(status));
> -               return;
> -       }
> -
> -       /* There will be only one descriptor on list and it will be CCC */
> -       desc = descs->data;
> -       bas->ccc_handle = desc->handle;
> -
> -       read_char(bas, bas->attrib, desc->handle, ccc_read_cb, bas);
> -}
> -
> -static void bas_discovered_cb(uint8_t status, GSList *chars, void *user_data)
> -{
> -       struct gatt_request *req = user_data;
> -       struct bt_bas *bas = req->user_data;
> -       struct gatt_char *chr;
> -       uint16_t start, end;
> -       bt_uuid_t uuid;
> -
> -       destroy_gatt_req(req);
> -
> -       if (status) {
> -               error("Battery: %s", att_ecode2str(status));
> -               return;
> -       }
> -
> -       chr = chars->data;
> -       bas->handle = chr->value_handle;
> -
> -       DBG("Battery handle: 0x%04x", bas->handle);
> -
> -       read_char(bas, bas->attrib, bas->handle, read_value_cb, bas);
> -
> -       start = chr->value_handle + 1;
> -       end = bas->primary->range.end;
> -
> -       bt_uuid16_create(&uuid, GATT_CLIENT_CHARAC_CFG_UUID);
> -
> -       discover_desc(bas, bas->attrib, start, end, &uuid,
> -                                               discover_descriptor_cb, bas);
> -}
> -
> -bool bt_bas_attach(struct bt_bas *bas, void *attrib)
> -{
> -       if (!bas || bas->attrib || !bas->primary)
> -               return false;
> -
> -       bas->attrib = g_attrib_ref(attrib);
> -
> -       if (bas->handle > 0)
> -               return true;
> -
> -       discover_char(bas, bas->attrib, bas->primary->range.start,
> -                                       bas->primary->range.end, NULL,
> -                                       bas_discovered_cb, bas);
> -
> -       return true;
> -}
> -
> -static void cancel_gatt_req(struct gatt_request *req)
> -{
> -       if (g_attrib_cancel(req->bas->attrib, req->id))
> -               destroy_gatt_req(req);
> -}
> -
> -void bt_bas_detach(struct bt_bas *bas)
> -{
> -       if (!bas || !bas->attrib)
> -               return;
> -
> -       if (bas->id > 0) {
> -               g_attrib_unregister(bas->attrib, bas->id);
> -               bas->id = 0;
> -       }
> -
> -       queue_foreach(bas->gatt_op, (void *) cancel_gatt_req, NULL);
> -       g_attrib_unref(bas->attrib);
> -       bas->attrib = NULL;
> -}
> diff --git a/profiles/battery/bas.h b/profiles/battery/bas.h
> deleted file mode 100644
> index 3e175b5b5..000000000
> --- a/profiles/battery/bas.h
> +++ /dev/null
> @@ -1,32 +0,0 @@
> -/*
> - *
> - *  BlueZ - Bluetooth protocol stack for Linux
> - *
> - *  Copyright (C) 2014  Intel Corporation. All rights reserved.
> - *
> - *
> - *  This library is free software; you can rebastribute it and/or
> - *  modify it under the terms of the GNU Lesser General Public
> - *  License as published by the Free Software Foundation; either
> - *  version 2.1 of the License, or (at your option) any later version.
> - *
> - *  This library is bastributed in the hope that it will be useful,
> - *  but WITHOUT ANY WARRANTY; without even the implied warranty of
> - *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> - *  Lesser General Public License for more details.
> - *
> - *  You should have received a copy of the GNU Lesser General Public
> - *  License along with this library; if not, write to the Free Software
> - *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
> - *
> - */
> -
> -struct bt_bas;
> -
> -struct bt_bas *bt_bas_new(void *primary);
> -
> -struct bt_bas *bt_bas_ref(struct bt_bas *bas);
> -void bt_bas_unref(struct bt_bas *bas);
> -
> -bool bt_bas_attach(struct bt_bas *bas, void *gatt);
> -void bt_bas_detach(struct bt_bas *bas);
> diff --git a/profiles/input/hog-lib.c b/profiles/input/hog-lib.c
> index dab385fe7..7b0097cb6 100644
> --- a/profiles/input/hog-lib.c
> +++ b/profiles/input/hog-lib.c
> @@ -57,7 +57,6 @@
>
>  #include "profiles/scanparam/scpp.h"
>  #include "profiles/deviceinfo/dis.h"
> -#include "profiles/battery/bas.h"
>  #include "profiles/input/hog-lib.h"
>
>  #define HOG_UUID               "00001812-0000-1000-8000-00805f9b34fb"
> @@ -105,7 +104,6 @@ struct bt_hog {
>         uint16_t                setrep_id;
>         struct bt_scpp          *scpp;
>         struct bt_dis           *dis;
> -       struct queue            *bas;
>         GSList                  *instances;
>         struct queue            *gatt_op;
>  };
> @@ -1174,7 +1172,6 @@ static void hog_free(void *data)
>
>         bt_hog_detach(hog);
>
> -       queue_destroy(hog->bas, (void *) bt_bas_unref);
>         g_slist_free_full(hog->instances, hog_free);
>
>         bt_scpp_unref(hog->scpp);
> @@ -1323,7 +1320,6 @@ static struct bt_hog *hog_new(int fd, const char *name, uint16_t vendor,
>                 return NULL;
>
>         hog->gatt_op = queue_new();
> -       hog->bas = queue_new();
>
>         if (fd < 0)
>                 hog->uhid = bt_uhid_new_default();
> @@ -1332,7 +1328,7 @@ static struct bt_hog *hog_new(int fd, const char *name, uint16_t vendor,
>
>         hog->uhid_fd = fd;
>
> -       if (!hog->gatt_op || !hog->bas || !hog->uhid) {
> +       if (!hog->gatt_op || !hog->uhid) {
>                 hog_free(hog);
>                 return NULL;
>         }
> @@ -1483,16 +1479,6 @@ static void hog_attach_dis(struct bt_hog *hog, struct gatt_primary *primary)
>         }
>  }
>
> -static void hog_attach_bas(struct bt_hog *hog, struct gatt_primary *primary)
> -{
> -       struct bt_bas *instance;
> -
> -       instance = bt_bas_new(primary);
> -
> -       bt_bas_attach(instance, hog->attrib);
> -       queue_push_head(hog->bas, instance);
> -}
> -
>  static void hog_attach_hog(struct bt_hog *hog, struct gatt_primary *primary)
>  {
>         struct bt_hog *instance;
> @@ -1555,11 +1541,6 @@ static void primary_cb(uint8_t status, GSList *services, void *user_data)
>                         continue;
>                 }
>
> -               if (strcmp(primary->uuid, BATTERY_UUID) == 0) {
> -                       hog_attach_bas(hog, primary);
> -                       continue;
> -               }
> -
>                 if (strcmp(primary->uuid, HOG_UUID) == 0)
>                         hog_attach_hog(hog, primary);
>         }
> @@ -1585,8 +1566,6 @@ bool bt_hog_attach(struct bt_hog *hog, void *gatt)
>         if (hog->dis)
>                 bt_dis_attach(hog->dis, gatt);
>
> -       queue_foreach(hog->bas, (void *) bt_bas_attach, gatt);
> -
>         for (l = hog->instances; l; l = l->next) {
>                 struct bt_hog *instance = l->data;
>
> @@ -1625,8 +1604,6 @@ void bt_hog_detach(struct bt_hog *hog)
>         if (!hog->attrib)
>                 return;
>
> -       queue_foreach(hog->bas, (void *) bt_bas_detach, NULL);
> -
>         for (l = hog->instances; l; l = l->next) {
>                 struct bt_hog *instance = l->data;
>
> --
> 2.14.2
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html



-- 
Luiz Augusto von Dentz

Re: [PATCH v2 3/3] profiles/battery: Remove unused bas.[ch]

[Bastien Nocera]

On Mon, 2017-10-23 at 23:39 +0300, Luiz Augusto von Dentz wrote:
> Hi Bastian,
> 
> On Mon, Oct 23, 2017 at 6:48 PM, Bastien Nocera <hadess@hadess.net>
> wrote:
> > The Battery Service characteristics are not children of the HoG
> > characteristics, so there's nothing to consume in the input
> > profile.
> > Remove that code which duplicated most of the battery profile
> > plugin.
> 
> Id rather not do this, we actually do intend the maintain a small
> helper library that can be unit tested, what bt_bas lacks is just
> proper support of bt_gatt_client nothing else. Also for the record
> this would probably break BfA HoG qualification which requires
> battery
> service to be supported as well, Android code does not have any other
> bas implementation.

That's fine, we can leave this file around. What about the other
patches?

Re: [PATCH v2 3/3] profiles/battery: Remove unused bas.[ch]

[Luiz Augusto von Dentz]

Hi Bastien,

On Tue, Oct 24, 2017 at 1:13 AM, Bastien Nocera <hadess@hadess.net> wrote:
> On Mon, 2017-10-23 at 23:39 +0300, Luiz Augusto von Dentz wrote:
>> Hi Bastian,
>>
>> On Mon, Oct 23, 2017 at 6:48 PM, Bastien Nocera <hadess@hadess.net>
>> wrote:
>> > The Battery Service characteristics are not children of the HoG
>> > characteristics, so there's nothing to consume in the input
>> > profile.
>> > Remove that code which duplicated most of the battery profile
>> > plugin.
>>
>> Id rather not do this, we actually do intend the maintain a small
>> helper library that can be unit tested, what bt_bas lacks is just
>> proper support of bt_gatt_client nothing else. Also for the record
>> this would probably break BfA HoG qualification which requires
>> battery
>> service to be supported as well, Android code does not have any other
>> bas implementation.
>
> That's fine, we can leave this file around. What about the other
> patches?

Id move the code around bt_gatt_client to bt_bas and let it handle the
internal details, the long term plan is to get rid of GAttrib so
perhaps we should start with bas.

-- 
Luiz Augusto von Dentz

Re: [PATCH v2 3/3] profiles/battery: Remove unused bas.[ch]

[Bastien Nocera]

On Tue, 2017-10-24 at 10:40 +0300, Luiz Augusto von Dentz wrote:
> Hi Bastien,
> 
> On Tue, Oct 24, 2017 at 1:13 AM, Bastien Nocera <hadess@hadess.net>
> wrote:
> > On Mon, 2017-10-23 at 23:39 +0300, Luiz Augusto von Dentz wrote:
> > > Hi Bastian,
> > > 
> > > On Mon, Oct 23, 2017 at 6:48 PM, Bastien Nocera <hadess@hadess.ne
> > > t>
> > > wrote:
> > > > The Battery Service characteristics are not children of the HoG
> > > > characteristics, so there's nothing to consume in the input
> > > > profile.
> > > > Remove that code which duplicated most of the battery profile
> > > > plugin.
> > > 
> > > Id rather not do this, we actually do intend the maintain a small
> > > helper library that can be unit tested, what bt_bas lacks is just
> > > proper support of bt_gatt_client nothing else. Also for the
> > > record
> > > this would probably break BfA HoG qualification which requires
> > > battery
> > > service to be supported as well, Android code does not have any
> > > other
> > > bas implementation.
> > 
> > That's fine, we can leave this file around. What about the other
> > patches?
> 
> Id move the code around bt_gatt_client to bt_bas and let it handle
> the
> internal details, the long term plan is to get rid of GAttrib so
> perhaps we should start with bas.

Right. That won't be me though. bas.c is a stub which isn't even
capable of getting a value from the device.

If somebody wants to split off the battery.c code into a newer bas.c,
I'm fine with that, but I'm not porting that code, _then_ implementing
its logic when I've already done that in battery.c.

Cheers