* [PATCH 1/3] battery: Add BT SIG reserved numbers used by Battery Service
@ 2017-10-20 23:03 Bastien Nocera
2017-10-20 23:03 ` [PATCH 2/3] profiles/battery: Add Bluetooth LE Battery service Bastien Nocera
2017-10-20 23:03 ` [PATCH 3/3] profiles/battery: Remove unused bas.[ch] Bastien Nocera
0 siblings, 2 replies; 10+ messages in thread
From: Bastien Nocera @ 2017-10-20 23:03 UTC (permalink / raw)
To: linux-bluetooth; +Cc: 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
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 2/3] profiles/battery: Add Bluetooth LE Battery service
2017-10-20 23:03 [PATCH 1/3] battery: Add BT SIG reserved numbers used by Battery Service Bastien Nocera
@ 2017-10-20 23:03 ` Bastien Nocera
2017-10-23 13:55 ` Bastien Nocera
2017-10-24 11:58 ` Luiz Augusto von Dentz
2017-10-20 23:03 ` [PATCH 3/3] profiles/battery: Remove unused bas.[ch] Bastien Nocera
1 sibling, 2 replies; 10+ messages in thread
From: Bastien Nocera @ 2017-10-20 23:03 UTC (permalink / raw)
To: linux-bluetooth; +Cc: 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 | 568 +++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 607 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..8e967e159
--- /dev/null
+++ b/profiles/battery/battery.c
@@ -0,0 +1,568 @@
+/*
+ *
+ * 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_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;
+ }
+}
+
+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
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 3/3] profiles/battery: Remove unused bas.[ch]
2017-10-20 23:03 [PATCH 1/3] battery: Add BT SIG reserved numbers used by Battery Service Bastien Nocera
2017-10-20 23:03 ` [PATCH 2/3] profiles/battery: Add Bluetooth LE Battery service Bastien Nocera
@ 2017-10-20 23:03 ` Bastien Nocera
1 sibling, 0 replies; 10+ messages in thread
From: Bastien Nocera @ 2017-10-20 23:03 UTC (permalink / raw)
To: linux-bluetooth; +Cc: 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
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 2/3] profiles/battery: Add Bluetooth LE Battery service
2017-10-20 23:03 ` [PATCH 2/3] profiles/battery: Add Bluetooth LE Battery service Bastien Nocera
@ 2017-10-23 13:55 ` Bastien Nocera
2017-10-24 11:58 ` Luiz Augusto von Dentz
1 sibling, 0 replies; 10+ messages in thread
From: Bastien Nocera @ 2017-10-23 13:55 UTC (permalink / raw)
To: linux-bluetooth
On Sat, 2017-10-21 at 01:03 +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.
There might be a bug somewhere in this code which caused devices not to
send updated "Connected" properties when disconnected, I'm debugging
this now.
Comments on the API would be appreciated though.
Cheers
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/3] profiles/battery: Add Bluetooth LE Battery service
2017-10-20 23:03 ` [PATCH 2/3] profiles/battery: Add Bluetooth LE Battery service Bastien Nocera
2017-10-23 13:55 ` Bastien Nocera
@ 2017-10-24 11:58 ` Luiz Augusto von Dentz
2017-10-24 12:30 ` Bastien Nocera
1 sibling, 1 reply; 10+ messages in thread
From: Luiz Augusto von Dentz @ 2017-10-24 11:58 UTC (permalink / raw)
To: Bastien Nocera; +Cc: linux-bluetooth
Hi Bastien,
On Sat, Oct 21, 2017 at 2:03 AM, Bastien Nocera <hadess@hadess.net> 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.
> ---
> Makefile.plugins | 3 +
> doc/battery-api.txt | 36 +++
> profiles/battery/battery.c | 568 +++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 607 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.
Not sure why would we have this as a property, if the battery is not
present then we should not even publish the interface.
> + 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..8e967e159
> --- /dev/null
> +++ b/profiles/battery/battery.c
> @@ -0,0 +1,568 @@
> +/*
> + *
> + * 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 */
I guess it is better to quote the actual text instead of the link here.
> + 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_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;
> + }
> +}
> +
> +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
Otherwise looks good.
--
Luiz Augusto von Dentz
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/3] profiles/battery: Add Bluetooth LE Battery service
2017-10-24 11:58 ` Luiz Augusto von Dentz
@ 2017-10-24 12:30 ` Bastien Nocera
2017-10-26 13:19 ` Luiz Augusto von Dentz
0 siblings, 1 reply; 10+ messages in thread
From: Bastien Nocera @ 2017-10-24 12:30 UTC (permalink / raw)
To: Luiz Augusto von Dentz; +Cc: linux-bluetooth
On Tue, 2017-10-24 at 14:58 +0300, Luiz Augusto von Dentz wrote:
> Hi Bastien,
>
> On Sat, Oct 21, 2017 at 2:03 AM, Bastien Nocera <hadess@hadess.net>
> 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.
> > ---
> > Makefile.plugins | 3 +
> > doc/battery-api.txt | 36 +++
> > profiles/battery/battery.c | 568
> > +++++++++++++++++++++++++++++++++++++++++++++
> > 3 files changed, 607 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.
>
> Not sure why would we have this as a property, if the battery is not
> present then we should not even publish the interface.
UPower has a "present" property as well, so this matches both the
service and the UPower properties. It avoids logic bugs where the
battery presence changes but we'd forget to export/unexport the
interface (or races). Finally, it's useful information for devices
which have another means of power (I can imagine some devices having
batteries, but also working wired, with a removable battery).
> > + boolean Rechargeable [readonly]
> > +
> > + Whether the battery built into the device
> > is rechargeable
> > + or not.
We wouldn't be able to export this property either if the battery
wasn't present above, and we did hide the interface.
<snip>
> > + /* Values explained at:
> > + * https://www.bluetooth.com/specifications/gatt/viewer?att
> > ributeXmlFile=org.bluetooth.characteristic.battery_power_state.xml
> > */
>
> I guess it is better to quote the actual text instead of the link
> here.
Sure. I'll send an updated patch with that.
<snip>
> > --
> > 2.14.2
>
> Otherwise looks good.
Great!
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/3] profiles/battery: Add Bluetooth LE Battery service
2017-10-24 12:30 ` Bastien Nocera
@ 2017-10-26 13:19 ` Luiz Augusto von Dentz
2017-10-26 13:36 ` Luiz Augusto von Dentz
0 siblings, 1 reply; 10+ messages in thread
From: Luiz Augusto von Dentz @ 2017-10-26 13:19 UTC (permalink / raw)
To: Bastien Nocera; +Cc: linux-bluetooth
Hi Bastien,
On Tue, Oct 24, 2017 at 3:30 PM, Bastien Nocera <hadess@hadess.net> wrote:
> On Tue, 2017-10-24 at 14:58 +0300, Luiz Augusto von Dentz wrote:
>> Hi Bastien,
>>
>> On Sat, Oct 21, 2017 at 2:03 AM, Bastien Nocera <hadess@hadess.net>
>> 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.
>> > ---
>> > Makefile.plugins | 3 +
>> > doc/battery-api.txt | 36 +++
>> > profiles/battery/battery.c | 568
>> > +++++++++++++++++++++++++++++++++++++++++++++
>> > 3 files changed, 607 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.
>>
>> Not sure why would we have this as a property, if the battery is not
>> present then we should not even publish the interface.
>
> UPower has a "present" property as well, so this matches both the
> service and the UPower properties. It avoids logic bugs where the
> battery presence changes but we'd forget to export/unexport the
> interface (or races). Finally, it's useful information for devices
> which have another means of power (I can imagine some devices having
> batteries, but also working wired, with a removable battery).
Not sure what another power source has to do with it? Power and
battery are 2 different things. About race conditions, if you do have
the interface it means it is battery powered if we do set its presence
to false all other properties should be invalidated as well and the
same should go when battery is plugged again all properties have to be
fetched once again, so imo it easier to add and remove the entire
interface to avoid having properties in inconsistent state. Btw,
forgetting to export/unexport may happen either way if the device
decides to remove the entire service that is what should happen, in
fact is what I would do if implementing the service so there is no
chance the remote side would maintain a subscription for something
that is not actually present anymore.
>> > + boolean Rechargeable [readonly]
>> > +
>> > + Whether the battery built into the device
>> > is rechargeable
>> > + or not.
>
> We wouldn't be able to export this property either if the battery
> wasn't present above, and we did hide the interface.
> <snip>
>> > + /* Values explained at:
>> > + * https://www.bluetooth.com/specifications/gatt/viewer?att
>> > ributeXmlFile=org.bluetooth.characteristic.battery_power_state.xml
>> > */
>>
>> I guess it is better to quote the actual text instead of the link
>> here.
>
> Sure. I'll send an updated patch with that.
>
> <snip>
>> > --
>> > 2.14.2
>>
>> Otherwise looks good.
>
> Great!
--
Luiz Augusto von Dentz
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/3] profiles/battery: Add Bluetooth LE Battery service
2017-10-26 13:19 ` Luiz Augusto von Dentz
@ 2017-10-26 13:36 ` Luiz Augusto von Dentz
2017-10-26 14:47 ` Bastien Nocera
0 siblings, 1 reply; 10+ messages in thread
From: Luiz Augusto von Dentz @ 2017-10-26 13:36 UTC (permalink / raw)
To: Bastien Nocera; +Cc: linux-bluetooth
Hi Bastien,
On Thu, Oct 26, 2017 at 4:19 PM, Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
> Hi Bastien,
>
> On Tue, Oct 24, 2017 at 3:30 PM, Bastien Nocera <hadess@hadess.net> wrote:
>> On Tue, 2017-10-24 at 14:58 +0300, Luiz Augusto von Dentz wrote:
>>> Hi Bastien,
>>>
>>> On Sat, Oct 21, 2017 at 2:03 AM, Bastien Nocera <hadess@hadess.net>
>>> 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.
>>> > ---
>>> > Makefile.plugins | 3 +
>>> > doc/battery-api.txt | 36 +++
>>> > profiles/battery/battery.c | 568
>>> > +++++++++++++++++++++++++++++++++++++++++++++
>>> > 3 files changed, 607 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.
>>>
>>> Not sure why would we have this as a property, if the battery is not
>>> present then we should not even publish the interface.
>>
>> UPower has a "present" property as well, so this matches both the
>> service and the UPower properties. It avoids logic bugs where the
>> battery presence changes but we'd forget to export/unexport the
>> interface (or races). Finally, it's useful information for devices
>> which have another means of power (I can imagine some devices having
>> batteries, but also working wired, with a removable battery).
>
> Not sure what another power source has to do with it? Power and
> battery are 2 different things. About race conditions, if you do have
> the interface it means it is battery powered if we do set its presence
> to false all other properties should be invalidated as well and the
> same should go when battery is plugged again all properties have to be
> fetched once again, so imo it easier to add and remove the entire
> interface to avoid having properties in inconsistent state. Btw,
> forgetting to export/unexport may happen either way if the device
> decides to remove the entire service that is what should happen, in
> fact is what I would do if implementing the service so there is no
> chance the remote side would maintain a subscription for something
> that is not actually present anymore.
Btw, the battery state don't seem to be even mentioned in the spec,
also the service declaration don't mention it only the battery level:
https://www.bluetooth.com/specifications/gatt/viewer?attributeXmlFile=org.bluetooth.service.battery_service.xml
>>> > + boolean Rechargeable [readonly]
>>> > +
>>> > + Whether the battery built into the device
>>> > is rechargeable
>>> > + or not.
>>
>> We wouldn't be able to export this property either if the battery
>> wasn't present above, and we did hide the interface.
>> <snip>
>>> > + /* Values explained at:
>>> > + * https://www.bluetooth.com/specifications/gatt/viewer?att
>>> > ributeXmlFile=org.bluetooth.characteristic.battery_power_state.xml
>>> > */
>>>
>>> I guess it is better to quote the actual text instead of the link
>>> here.
>>
>> Sure. I'll send an updated patch with that.
>>
>> <snip>
>>> > --
>>> > 2.14.2
>>>
>>> Otherwise looks good.
>>
>> Great!
>
>
>
> --
> Luiz Augusto von Dentz
--
Luiz Augusto von Dentz
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/3] profiles/battery: Add Bluetooth LE Battery service
2017-10-26 13:36 ` Luiz Augusto von Dentz
@ 2017-10-26 14:47 ` Bastien Nocera
2017-10-26 18:13 ` Luiz Augusto von Dentz
0 siblings, 1 reply; 10+ messages in thread
From: Bastien Nocera @ 2017-10-26 14:47 UTC (permalink / raw)
To: Luiz Augusto von Dentz; +Cc: linux-bluetooth
On Thu, 2017-10-26 at 16:36 +0300, Luiz Augusto von Dentz wrote:
> Hi Bastien,
>
> On Thu, Oct 26, 2017 at 4:19 PM, Luiz Augusto von Dentz
> <luiz.dentz@gmail.com> wrote:
> > Hi Bastien,
> >
> > On Tue, Oct 24, 2017 at 3:30 PM, Bastien Nocera <hadess@hadess.net>
> > wrote:
> > > On Tue, 2017-10-24 at 14:58 +0300, Luiz Augusto von Dentz wrote:
> > > > Hi Bastien,
> > > >
> > > > On Sat, Oct 21, 2017 at 2:03 AM, Bastien Nocera <hadess@hadess.
> > > > net>
> > > > 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.
> > > > > ---
> > > > > Makefile.plugins | 3 +
> > > > > doc/battery-api.txt | 36 +++
> > > > > profiles/battery/battery.c | 568
> > > > > +++++++++++++++++++++++++++++++++++++++++++++
> > > > > 3 files changed, 607 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.
> > > >
> > > > Not sure why would we have this as a property, if the battery
> > > > is not
> > > > present then we should not even publish the interface.
> > >
> > > UPower has a "present" property as well, so this matches both the
> > > service and the UPower properties. It avoids logic bugs where the
> > > battery presence changes but we'd forget to export/unexport the
> > > interface (or races). Finally, it's useful information for
> > > devices
> > > which have another means of power (I can imagine some devices
> > > having
> > > batteries, but also working wired, with a removable battery).
> >
> > Not sure what another power source has to do with it? Power and
> > battery are 2 different things. About race conditions, if you do
> > have
> > the interface it means it is battery powered if we do set its
> > presence
> > to false all other properties should be invalidated as well and the
> > same should go when battery is plugged again all properties have to
> > be
> > fetched once again, so imo it easier to add and remove the entire
> > interface to avoid having properties in inconsistent state. Btw,
> > forgetting to export/unexport may happen either way if the device
> > decides to remove the entire service that is what should happen, in
> > fact is what I would do if implementing the service so there is no
> > chance the remote side would maintain a subscription for something
> > that is not actually present anymore.
>
> Btw, the battery state don't seem to be even mentioned in the spec,
> also the service declaration don't mention it only the battery level:
>
> https://www.bluetooth.com/specifications/gatt/viewer?attributeXmlFile
> =org.bluetooth.service.battery_service.xml
You're looking at the wrong spec. There's 2 different set of
characteristics, one called "Battery Level", which pretty much only
exports the percentage, and "Battery State" which has a presence value:
https://www.bluetooth.com/specifications/gatt/viewer?attributeXmlFile=org.bluetooth.characteristic.battery_power_state.xml
I don't know whether it's actually an adopted spec, or whether it's
used, and I'm fine simply removing the support for the time being. Or
we could implement it and see whether it breaks anything :)
Cheers
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/3] profiles/battery: Add Bluetooth LE Battery service
2017-10-26 14:47 ` Bastien Nocera
@ 2017-10-26 18:13 ` Luiz Augusto von Dentz
0 siblings, 0 replies; 10+ messages in thread
From: Luiz Augusto von Dentz @ 2017-10-26 18:13 UTC (permalink / raw)
To: Bastien Nocera; +Cc: linux-bluetooth
Hi Bastien,
On Thu, Oct 26, 2017 at 5:47 PM, Bastien Nocera <hadess@hadess.net> wrote:
> On Thu, 2017-10-26 at 16:36 +0300, Luiz Augusto von Dentz wrote:
>> Hi Bastien,
>>
>> On Thu, Oct 26, 2017 at 4:19 PM, Luiz Augusto von Dentz
>> <luiz.dentz@gmail.com> wrote:
>> > Hi Bastien,
>> >
>> > On Tue, Oct 24, 2017 at 3:30 PM, Bastien Nocera <hadess@hadess.net>
>> > wrote:
>> > > On Tue, 2017-10-24 at 14:58 +0300, Luiz Augusto von Dentz wrote:
>> > > > Hi Bastien,
>> > > >
>> > > > On Sat, Oct 21, 2017 at 2:03 AM, Bastien Nocera <hadess@hadess.
>> > > > net>
>> > > > 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.
>> > > > > ---
>> > > > > Makefile.plugins | 3 +
>> > > > > doc/battery-api.txt | 36 +++
>> > > > > profiles/battery/battery.c | 568
>> > > > > +++++++++++++++++++++++++++++++++++++++++++++
>> > > > > 3 files changed, 607 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.
>> > > >
>> > > > Not sure why would we have this as a property, if the battery
>> > > > is not
>> > > > present then we should not even publish the interface.
>> > >
>> > > UPower has a "present" property as well, so this matches both the
>> > > service and the UPower properties. It avoids logic bugs where the
>> > > battery presence changes but we'd forget to export/unexport the
>> > > interface (or races). Finally, it's useful information for
>> > > devices
>> > > which have another means of power (I can imagine some devices
>> > > having
>> > > batteries, but also working wired, with a removable battery).
>> >
>> > Not sure what another power source has to do with it? Power and
>> > battery are 2 different things. About race conditions, if you do
>> > have
>> > the interface it means it is battery powered if we do set its
>> > presence
>> > to false all other properties should be invalidated as well and the
>> > same should go when battery is plugged again all properties have to
>> > be
>> > fetched once again, so imo it easier to add and remove the entire
>> > interface to avoid having properties in inconsistent state. Btw,
>> > forgetting to export/unexport may happen either way if the device
>> > decides to remove the entire service that is what should happen, in
>> > fact is what I would do if implementing the service so there is no
>> > chance the remote side would maintain a subscription for something
>> > that is not actually present anymore.
>>
>> Btw, the battery state don't seem to be even mentioned in the spec,
>> also the service declaration don't mention it only the battery level:
>>
>> https://www.bluetooth.com/specifications/gatt/viewer?attributeXmlFile
>> =org.bluetooth.service.battery_service.xml
>
> You're looking at the wrong spec. There's 2 different set of
> characteristics, one called "Battery Level", which pretty much only
> exports the percentage, and "Battery State" which has a presence value:
> https://www.bluetooth.com/specifications/gatt/viewer?attributeXmlFile=org.bluetooth.characteristic.battery_power_state.xml
But it is just a characteristic, it must belong to service, and
apparently, it doesn't belong to BAS as not even the qualification
tests mention it.
> I don't know whether it's actually an adopted spec, or whether it's
> used, and I'm fine simply removing the support for the time being. Or
> we could implement it and see whether it breaks anything :)
We may enable it if we find out there are devices device implementing
it, not sure what happened with it perhaps it got detached from BAS to
not make it mandatory, which would be quite confusing. If really means
it is optional attribute though then we should make sure the interface
don't have any hard dependency on it, but Im having a second thought
that there might be disagreements on how to handle such attribute
since things like presence may influence the level as well, since they
are 2 different characteristics what do we do when it say the battery
is not present.
--
Luiz Augusto von Dentz
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2017-10-26 18:13 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-10-20 23:03 [PATCH 1/3] battery: Add BT SIG reserved numbers used by Battery Service Bastien Nocera
2017-10-20 23:03 ` [PATCH 2/3] profiles/battery: Add Bluetooth LE Battery service Bastien Nocera
2017-10-23 13:55 ` Bastien Nocera
2017-10-24 11:58 ` Luiz Augusto von Dentz
2017-10-24 12:30 ` Bastien Nocera
2017-10-26 13:19 ` Luiz Augusto von Dentz
2017-10-26 13:36 ` Luiz Augusto von Dentz
2017-10-26 14:47 ` Bastien Nocera
2017-10-26 18:13 ` Luiz Augusto von Dentz
2017-10-20 23:03 ` [PATCH 3/3] profiles/battery: Remove unused bas.[ch] Bastien Nocera
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.