This patch adds an API for internal BlueZ code to expose battery
information to org.bluez.Battery1 interface. The motivation behind this
is that there is going to be other places than GATT BAS handler that
exposes battery information, for example internal plugins and the
BatteryProvider1 D-Bus API for external clients.
Reviewed-by: Daniel Winkler <[email protected]>
---
Makefile.am | 3 +-
src/battery.c | 191 ++++++++++++++++++++++++++++++++++++++++++++++++++
src/battery.h | 15 ++++
3 files changed, 208 insertions(+), 1 deletion(-)
create mode 100644 src/battery.c
create mode 100644 src/battery.h
diff --git a/Makefile.am b/Makefile.am
index 69b95828f..95917f695 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -294,7 +294,8 @@ src_bluetoothd_SOURCES = $(builtin_sources) \
src/device.h src/device.c \
src/dbus-common.c src/dbus-common.h \
src/eir.h src/eir.c \
- src/adv_monitor.h src/adv_monitor.c
+ src/adv_monitor.h src/adv_monitor.c \
+ src/battery.h src/battery.c
src_bluetoothd_LDADD = lib/libbluetooth-internal.la \
gdbus/libgdbus-internal.la \
src/libshared-glib.la \
diff --git a/src/battery.c b/src/battery.c
new file mode 100644
index 000000000..87a6b91fb
--- /dev/null
+++ b/src/battery.c
@@ -0,0 +1,191 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ *
+ * BlueZ - Bluetooth protocol stack for Linux
+ *
+ * Copyright (C) 2020 Google LLC
+ *
+ *
+ */
+
+#ifdef HAVE_CONFIG_H
+#include <config.h>
+#endif
+
+#define _GNU_SOURCE
+#include <stdbool.h>
+#include <stdint.h>
+#include <glib.h>
+
+#include "gdbus/gdbus.h"
+#include "lib/bluetooth.h"
+#include "src/shared/queue.h"
+#include "src/shared/util.h"
+#include "battery.h"
+#include "dbus-common.h"
+#include "adapter.h"
+#include "log.h"
+
+#define BATTERY_INTERFACE "org.bluez.Battery1"
+
+#define BATTERY_MAX_PERCENTAGE 100
+
+struct btd_battery {
+ char *path; /* D-Bus object path, owns pointer */
+ uint8_t percentage; /* valid between 0 to 100 inclusively */
+};
+
+static struct queue *batteries = NULL;
+
+static void battery_add(struct btd_battery *battery)
+{
+ if (!batteries)
+ batteries = queue_new();
+
+ queue_push_head(batteries, battery);
+}
+
+static void battery_remove(struct btd_battery *battery)
+{
+ queue_remove(batteries, battery);
+ if (queue_isempty(batteries)) {
+ queue_destroy(batteries, NULL);
+ batteries = NULL;
+ }
+}
+
+static bool match_path(const void *data, const void *user_data)
+{
+ const struct btd_battery *battery = data;
+ const char *path = user_data;
+
+ return g_strcmp0(battery->path, path) == 0;
+}
+
+static struct btd_battery *battery_new(const char *path)
+{
+ struct btd_battery *battery;
+
+ battery = new0(struct btd_battery, 1);
+ battery->path = g_strdup(path);
+ battery->percentage = UINT8_MAX;
+
+ return battery;
+}
+
+static void battery_free(struct btd_battery *battery)
+{
+ if (battery->path)
+ g_free(battery->path);
+
+ free(battery);
+}
+
+static gboolean property_percentage_get(const GDBusPropertyTable *property,
+ DBusMessageIter *iter, void *data)
+{
+ struct btd_battery *battery = data;
+
+ dbus_message_iter_append_basic(iter, DBUS_TYPE_BYTE,
+ &battery->percentage);
+
+ return TRUE;
+}
+
+static gboolean property_percentage_exists(const GDBusPropertyTable *property,
+ void *data)
+{
+ struct btd_battery *battery = data;
+
+ return battery->percentage <= BATTERY_MAX_PERCENTAGE;
+}
+
+static const GDBusPropertyTable battery_properties[] = {
+ { "Percentage", "y", property_percentage_get, NULL,
+ property_percentage_exists },
+ {}
+};
+
+struct btd_battery *btd_battery_register(const char *path)
+{
+ struct btd_battery *battery;
+
+ DBG("path = %s", path);
+
+ if (queue_find(batteries, match_path, path)) {
+ error("error registering battery: path exists");
+ return NULL;
+ }
+
+ if (!g_str_has_prefix(path, "/")) {
+ error("error registering battery: invalid D-Bus object path");
+ return NULL;
+ }
+
+ battery = battery_new(path);
+ battery_add(battery);
+
+ if (!g_dbus_register_interface(btd_get_dbus_connection(), battery->path,
+ BATTERY_INTERFACE, NULL, NULL,
+ battery_properties, battery, NULL)) {
+ error("error registering D-Bus interface for %s",
+ battery->path);
+
+ battery_remove(battery);
+ battery_free(battery);
+
+ return NULL;
+ }
+
+ DBG("registered Battery object: %s", battery->path);
+
+ return battery;
+}
+
+bool btd_battery_unregister(struct btd_battery *battery)
+{
+ DBG("path = %s", battery->path);
+
+ if (!queue_find(batteries, NULL, battery)) {
+ error("error unregistering battery: "
+ "battery %s is not registered",
+ battery->path);
+ return false;
+ }
+
+ if (!g_dbus_unregister_interface(btd_get_dbus_connection(),
+ battery->path, BATTERY_INTERFACE)) {
+ error("error unregistering battery %s from D-Bus interface",
+ battery->path);
+ return false;
+ }
+
+ battery_remove(battery);
+ battery_free(battery);
+
+ return true;
+}
+
+bool btd_battery_update(struct btd_battery *battery, uint8_t percentage)
+{
+ DBG("path = %s", battery->path);
+
+ if (!queue_find(batteries, NULL, battery)) {
+ error("error updating battery: battery is not registered");
+ return false;
+ }
+
+ if (percentage > BATTERY_MAX_PERCENTAGE) {
+ error("error updating battery: percentage is not valid");
+ return false;
+ }
+
+ if (battery->percentage == percentage)
+ return true;
+
+ battery->percentage = percentage;
+ g_dbus_emit_property_changed(btd_get_dbus_connection(), battery->path,
+ BATTERY_INTERFACE, "Percentage");
+
+ return true;
+}
diff --git a/src/battery.h b/src/battery.h
new file mode 100644
index 000000000..9c69b7afa
--- /dev/null
+++ b/src/battery.h
@@ -0,0 +1,15 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ *
+ * BlueZ - Bluetooth protocol stack for Linux
+ *
+ * Copyright (C) 2020 Google LLC
+ *
+ *
+ */
+
+struct btd_battery;
+
+struct btd_battery *btd_battery_register(const char *path);
+bool btd_battery_unregister(struct btd_battery *battery);
+bool btd_battery_update(struct btd_battery *battery, uint8_t percentage);
--
2.26.2
This patch add the documentation of the Battery Provider which lets
external clients feed battery information to BlueZ if they are able to
decode battery reporting via any profile. BlueZ UI clients can then use
the org.bluez.Battery1 API as a single source of battery information
coming from many different profiles.
Reviewed-by: Miao-chen Chou <[email protected]>
---
Changes in v3:
* Remove doc duplication in BatteryProvider1 and mention that it's the
same as Battery1 instead.
* Suggest profile UUID in Source property.
doc/battery-api.txt | 49 +++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 49 insertions(+)
diff --git a/doc/battery-api.txt b/doc/battery-api.txt
index dc7dbeda2..b5c9a7be1 100644
--- a/doc/battery-api.txt
+++ b/doc/battery-api.txt
@@ -12,3 +12,52 @@ Object path [variable prefix]/{hci0,hci1,...}/dev_XX_XX_XX_XX_XX_XX
Properties byte Percentage [readonly]
The percentage of battery left as an unsigned 8-bit integer.
+
+ string Source [readonly, optional, experimental]
+
+ Describes where the battery information comes from
+ This property is informational only and may be useful
+ for debugging purposes.
+ Providers from BatteryProvider1 may make use of this
+ property to indicate where the battery report comes from
+ (e.g. "HFP 1.7", "HID", or the profile UUID).
+
+
+Battery Provider Manager hierarchy
+==================================
+A battery provider starts by registering itself as a battery provider with the
+RegisterBatteryProvider method passing an object path as the provider ID. Then,
+it can start exposing org.bluez.BatteryProvider1 objects having the path
+starting with the given provider ID. It can also remove objects at any time.
+The objects and their properties exposed by battery providers will be reflected
+on org.bluez.Battery1 interface.
+
+BlueZ will stop monitoring these exposed and removed objects after
+UnregisterBatteryProvider is called for that provider ID.
+
+Service org.bluez
+Interface org.bluez.BatteryProviderManager1 [experimental]
+Object path /org/bluez/{hci0,hci1,...}
+
+Methods void RegisterBatteryProvider(object provider)
+
+ This registers a battery provider. A registered
+ battery provider can then expose objects with
+ org.bluez.BatteryProvider1 interface described below.
+
+ void UnregisterBatteryProvider(object provider)
+
+ This unregisters a battery provider. After
+ unregistration, the BatteryProvider1 objects provided
+ by this client are ignored by BlueZ.
+
+
+Battery Provider hierarchy
+==========================
+
+Service <client D-Bus address>
+Interface org.bluez.BatteryProvider1 [experimental]
+Object path {provider_root}/org/bluez/{hci0,hci1,...}/dev_XX_XX_XX_XX_XX_XX
+
+Properties Objects provided on this interface contain the same properties
+ as org.bluez.Battery1 interface.
--
2.26.2
The python test app simulates an application registering to BlueZ as a
Battery Provider providing three fake batteries drained periodically.
Reviewed-by: Miao-chen Chou <[email protected]>
---
test/example-battery-provider | 230 ++++++++++++++++++++++++++++++++++
1 file changed, 230 insertions(+)
create mode 100755 test/example-battery-provider
diff --git a/test/example-battery-provider b/test/example-battery-provider
new file mode 100755
index 000000000..3dbb08563
--- /dev/null
+++ b/test/example-battery-provider
@@ -0,0 +1,230 @@
+#!/usr/bin/env python3
+# SPDX-License-Identifier: LGPL-2.1-or-later
+
+import dbus
+import dbus.exceptions
+import dbus.mainloop.glib
+import dbus.service
+
+try:
+ from gi.repository import GObject
+except ImportError:
+ import gobject as GObject
+import sys
+
+mainloop = None
+app = None
+bus = None
+
+BLUEZ_SERVICE_NAME = 'org.bluez'
+DBUS_OM_IFACE = 'org.freedesktop.DBus.ObjectManager'
+DBUS_PROP_IFACE = 'org.freedesktop.DBus.Properties'
+
+BATTERY_PROVIDER_MANAGER_IFACE = 'org.bluez.BatteryProviderManager1'
+BATTERY_PROVIDER_IFACE = 'org.bluez.BatteryProvider1'
+BATTERY_PROVIDER_PATH = '/path/to/provider'
+
+BATTERY_PATH1 = '11_11_11_11_11_11'
+BATTERY_PATH2 = '22_22_22_22_22_22'
+BATTERY_PATH3 = '33_33_33_33_33_33'
+
+class InvalidArgsException(dbus.exceptions.DBusException):
+ _dbus_error_name = 'org.freedesktop.DBus.Error.InvalidArgs'
+
+
+class Application(dbus.service.Object):
+ def __init__(self, bus):
+ self.path = BATTERY_PROVIDER_PATH
+ self.services = []
+ self.batteries = []
+ dbus.service.Object.__init__(self, bus, self.path)
+
+ def get_path(self):
+ return dbus.ObjectPath(self.path)
+
+ def add_battery(self, battery):
+ self.batteries.append(battery)
+ self.InterfacesAdded(battery.get_path(), battery.get_properties())
+ GObject.timeout_add(1000, drain_battery, battery)
+
+ def remove_battery(self, battery):
+ self.batteries.remove(battery)
+ self.InterfacesRemoved(battery.get_path(), [BATTERY_PROVIDER_IFACE])
+
+ @dbus.service.method(DBUS_OM_IFACE, out_signature='a{oa{sa{sv}}}')
+ def GetManagedObjects(self):
+ response = {}
+ print('GetManagedObjects called')
+
+ for battery in self.batteries:
+ response[battery.get_path()] = battery.get_properties()
+
+ return response
+
+ @dbus.service.signal(DBUS_OM_IFACE, signature='oa{sa{sv}}')
+ def InterfacesAdded(self, object_path, interfaces_and_properties):
+ return
+
+ @dbus.service.signal(DBUS_OM_IFACE, signature='oas')
+ def InterfacesRemoved(self, object_path, interfaces):
+ return
+
+
+class Battery(dbus.service.Object):
+ """
+ org.bluez.BatteryProvider1 interface implementation
+ """
+ def __init__(self, bus, dev, percentage, source = None):
+ self.path = BATTERY_PROVIDER_PATH + '/org/bluez/hci0/dev_' + dev
+ self.bus = bus
+ self.percentage = percentage
+ self.source = source
+ dbus.service.Object.__init__(self, bus, self.path)
+
+ def get_battery_properties(self):
+ properties = {}
+ if self.percentage != None:
+ properties['Percentage'] = dbus.Byte(self.percentage)
+ if self.source != None:
+ properties['Source'] = self.source
+ return properties
+
+ def get_properties(self):
+ return { BATTERY_PROVIDER_IFACE: self.get_battery_properties() }
+
+ def get_path(self):
+ return dbus.ObjectPath(self.path)
+
+ def set_percentage(self, percentage):
+ if percentage < 0 or percentage > 100:
+ print('percentage not valid')
+ return
+
+ self.percentage = percentage
+ print('battery %s percentage %d' % (self.path, self.percentage))
+ self.PropertiesChanged(
+ BATTERY_PROVIDER_IFACE, self.get_battery_properties())
+
+ @dbus.service.method(DBUS_PROP_IFACE,
+ in_signature='s',
+ out_signature='a{sv}')
+ def GetAll(self, interface):
+ if interface != BATTERY_PROVIDER_IFACE:
+ raise InvalidArgsException()
+
+ return self.get_properties()[BATTERY_PROVIDER_IFACE]
+
+ @dbus.service.signal(DBUS_PROP_IFACE, signature='sa{sv}')
+ def PropertiesChanged(self, interface, properties):
+ return
+
+
+def add_late_battery():
+ app.add_battery(Battery(bus, BATTERY_PATH3, 70, 'Protocol 2'))
+
+
+def drain_battery(battery):
+ new_percentage = 100
+ if battery.percentage != None:
+ new_percentage = battery.percentage - 5
+ if new_percentage < 0:
+ new_percentage = 0
+
+ battery.set_percentage(new_percentage)
+
+ if new_percentage <= 0:
+ return False
+
+ return True
+
+def register_provider_cb():
+ print('Battery Provider registered')
+
+ # Battery added early right after RegisterBatteryProvider succeeds
+ app.add_battery(Battery(bus, BATTERY_PATH2, None))
+ # Battery added later
+ GObject.timeout_add(5000, add_late_battery)
+
+
+def register_provider_error_cb(error):
+ print('Failed to register Battery Provider: ' + str(error))
+ mainloop.quit()
+
+
+def find_manager(bus):
+ remote_om = dbus.Interface(bus.get_object(BLUEZ_SERVICE_NAME, '/'),
+ DBUS_OM_IFACE)
+ objects = remote_om.GetManagedObjects()
+
+ for o, props in objects.items():
+ if BATTERY_PROVIDER_MANAGER_IFACE in props.keys():
+ return o
+
+ return None
+
+
+def unregister_provider_cb():
+ print('Battery Provider unregistered')
+
+
+def unregister_provider_error_cb(error):
+ print('Failed to unregister Battery Provider: ' + str(error))
+
+
+def unregister_battery_provider(battery_provider_manager):
+ battery_provider_manager.UnregisterBatteryProvider(BATTERY_PROVIDER_PATH,
+ reply_handler=unregister_provider_cb,
+ error_handler=unregister_provider_error_cb)
+
+
+def remove_battery(app, battery):
+ app.remove_battery(battery)
+
+
+"""
+Simulates an application registering to BlueZ as a Battery Provider providing
+fake batteries drained periodically.
+"""
+def main():
+ global mainloop, bus, app
+
+ dbus.mainloop.glib.DBusGMainLoop(set_as_default=True)
+
+ bus = dbus.SystemBus()
+
+ manager_path = find_manager(bus)
+ if not manager_path:
+ print('BatteryProviderManager1 interface not found')
+ return
+
+ print('BatteryProviderManager1 path = ', manager_path)
+
+ battery_provider_manager = dbus.Interface(
+ bus.get_object(BLUEZ_SERVICE_NAME, manager_path),
+ BATTERY_PROVIDER_MANAGER_IFACE)
+
+ app = Application(bus)
+
+ # Battery pre-added before RegisterBatteryProvider
+ battery1 = Battery(bus, BATTERY_PATH1, 87, 'Protocol 1')
+ app.add_battery(battery1)
+
+ mainloop = GObject.MainLoop()
+
+ print('Registering Battery Provider...')
+
+ battery_provider_manager.RegisterBatteryProvider(BATTERY_PROVIDER_PATH,
+ reply_handler=register_provider_cb,
+ error_handler=register_provider_error_cb)
+
+ # Unregister the Battery Provider after an arbitrary amount of time
+ GObject.timeout_add(
+ 12000, unregister_battery_provider, battery_provider_manager)
+ # Simulate battery removal by a provider
+ GObject.timeout_add(8000, remove_battery, app, battery1)
+
+ mainloop.run()
+
+
+if __name__ == '__main__':
+ main()
--
2.26.2
This patch implements the BatteryProvider1 and BatteryProviderManager1
API. This is a means for external clients to feed battery information to
BlueZ if they handle some profile and can decode battery reporting.
The battery information is then exposed externally via the existing
Battery1 interface. UI components can consume this API to display
Bluetooth peripherals' battery via a unified BlueZ API.
Reviewed-by: Miao-chen Chou <[email protected]>
---
Changes in v3:
* add check to reject battery provided for temporary devices
profiles/battery/battery.c | 2 +-
src/adapter.c | 11 ++
src/battery.c | 374 ++++++++++++++++++++++++++++++++++++-
src/battery.h | 10 +-
4 files changed, 392 insertions(+), 5 deletions(-)
diff --git a/profiles/battery/battery.c b/profiles/battery/battery.c
index 2478816a4..81f849d57 100644
--- a/profiles/battery/battery.c
+++ b/profiles/battery/battery.c
@@ -127,7 +127,7 @@ static void batt_io_ccc_written_cb(uint16_t att_ecode, void *user_data)
}
batt->battery = btd_battery_register(device_get_path(batt->device),
- "GATT Battery Service");
+ "GATT Battery Service", NULL);
if (!batt->battery) {
batt_reset(batt);
diff --git a/src/adapter.c b/src/adapter.c
index 03d9d29e9..ec6a6a64c 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -66,6 +66,7 @@
#include "advertising.h"
#include "adv_monitor.h"
#include "eir.h"
+#include "battery.h"
#define MODE_OFF 0x00
#define MODE_CONNECTABLE 0x01
@@ -254,6 +255,8 @@ struct btd_adapter {
struct btd_adv_monitor_manager *adv_monitor_manager;
+ struct btd_battery_provider_manager *battery_provider_manager;
+
gboolean initialized;
GSList *pin_callbacks;
@@ -6339,6 +6342,9 @@ static void adapter_remove(struct btd_adapter *adapter)
btd_adv_monitor_manager_destroy(adapter->adv_monitor_manager);
adapter->adv_monitor_manager = NULL;
+ btd_battery_provider_manager_destroy(adapter->battery_provider_manager);
+ adapter->battery_provider_manager = NULL;
+
g_slist_free(adapter->pin_callbacks);
adapter->pin_callbacks = NULL;
@@ -8659,6 +8665,11 @@ static int adapter_register(struct btd_adapter *adapter)
}
}
+ if (g_dbus_get_flags() & G_DBUS_FLAG_ENABLE_EXPERIMENTAL) {
+ adapter->battery_provider_manager =
+ btd_battery_provider_manager_create(adapter);
+ }
+
db = btd_gatt_database_get_db(adapter->database);
adapter->db_id = gatt_db_register(db, services_modified,
services_modified,
diff --git a/src/battery.c b/src/battery.c
index 8613d6e23..bdc37822f 100644
--- a/src/battery.c
+++ b/src/battery.c
@@ -24,9 +24,13 @@
#include "battery.h"
#include "dbus-common.h"
#include "adapter.h"
+#include "device.h"
#include "log.h"
+#include "error.h"
#define BATTERY_INTERFACE "org.bluez.Battery1"
+#define BATTERY_PROVIDER_INTERFACE "org.bluez.BatteryProvider1"
+#define BATTERY_PROVIDER_MANAGER_INTERFACE "org.bluez.BatteryProviderManager1"
#define BATTERY_MAX_PERCENTAGE 100
@@ -34,10 +38,27 @@ struct btd_battery {
char *path; /* D-Bus object path */
uint8_t percentage; /* valid between 0 to 100 inclusively */
char *source; /* Descriptive source of the battery info */
+ char *provider_path; /* The provider root path, if any */
+};
+
+struct btd_battery_provider_manager {
+ struct btd_adapter *adapter; /* Does not own pointer */
+ struct queue *battery_providers;
+};
+
+struct battery_provider {
+ struct btd_battery_provider_manager *manager; /* Does not own pointer */
+
+ char *owner; /* Owner D-Bus address */
+ char *path; /* D-Bus object path */
+
+ GDBusClient *client;
};
static struct queue *batteries = NULL;
+static void provider_disconnect_cb(DBusConnection *conn, void *user_data);
+
static void battery_add(struct btd_battery *battery)
{
if (!batteries)
@@ -63,7 +84,8 @@ static bool match_path(const void *data, const void *user_data)
return g_strcmp0(battery->path, path) == 0;
}
-static struct btd_battery *battery_new(const char *path, const char *source)
+static struct btd_battery *battery_new(const char *path, const char *source,
+ const char *provider_path)
{
struct btd_battery *battery;
@@ -72,6 +94,8 @@ static struct btd_battery *battery_new(const char *path, const char *source)
battery->percentage = UINT8_MAX;
if (source)
battery->source = g_strdup(source);
+ if (provider_path)
+ battery->provider_path = g_strdup(provider_path);
return battery;
}
@@ -133,7 +157,8 @@ static const GDBusPropertyTable battery_properties[] = {
{}
};
-struct btd_battery *btd_battery_register(const char *path, const char *source)
+struct btd_battery *btd_battery_register(const char *path, const char *source,
+ const char *provider_path)
{
struct btd_battery *battery;
@@ -149,7 +174,7 @@ struct btd_battery *btd_battery_register(const char *path, const char *source)
return NULL;
}
- battery = battery_new(path, source);
+ battery = battery_new(path, source, provider_path);
battery_add(battery);
if (!g_dbus_register_interface(btd_get_dbus_connection(), battery->path,
@@ -216,3 +241,346 @@ bool btd_battery_update(struct btd_battery *battery, uint8_t percentage)
return true;
}
+
+static struct btd_battery *find_battery_by_path(const char *path)
+{
+ return queue_find(batteries, match_path, path);
+}
+
+static void provided_battery_property_changed_cb(GDBusProxy *proxy,
+ const char *name,
+ DBusMessageIter *iter,
+ void *user_data)
+{
+ uint8_t percentage;
+ struct battery_provider *provider = user_data;
+ const char *path = g_dbus_proxy_get_path(proxy);
+ const char *export_path;
+
+ export_path = &path[strlen(provider->path)];
+
+ if (strcmp(name, "Percentage") != 0)
+ return;
+
+ if (dbus_message_iter_get_arg_type(iter) != DBUS_TYPE_BYTE)
+ return;
+
+ dbus_message_iter_get_basic(iter, &percentage);
+
+ DBG("battery percentage changed on %s, percentage = %d",
+ g_dbus_proxy_get_path(proxy), percentage);
+
+ btd_battery_update(find_battery_by_path(export_path), percentage);
+}
+
+static void provided_battery_added_cb(GDBusProxy *proxy, void *user_data)
+{
+ struct battery_provider *provider = user_data;
+ struct btd_battery *battery;
+ struct btd_device *device;
+ const char *path = g_dbus_proxy_get_path(proxy);
+ const char *export_path;
+ const char *source = NULL;
+ uint8_t percentage;
+ DBusMessageIter iter;
+
+ export_path = &path[strlen(provider->path)];
+
+ if (strcmp(g_dbus_proxy_get_interface(proxy),
+ BATTERY_PROVIDER_INTERFACE) != 0)
+ return;
+
+ device = btd_adapter_find_device_by_path(provider->manager->adapter,
+ export_path);
+ if (!device || device_is_temporary(device)) {
+ warn("Ignoring non-existent device path for battery %s",
+ export_path);
+ return;
+ }
+
+ if (find_battery_by_path(export_path))
+ return;
+
+ g_dbus_proxy_set_property_watch(
+ proxy, provided_battery_property_changed_cb, provider);
+
+ if (g_dbus_proxy_get_property(proxy, "Source", &iter) == TRUE)
+ dbus_message_iter_get_basic(&iter, &source);
+
+ battery = btd_battery_register(export_path, source, provider->path);
+
+ DBG("provided battery added %s", path);
+
+ /* Percentage property may not be immediately available, that's okay
+ * since we monitor changes to this property.
+ */
+ if (g_dbus_proxy_get_property(proxy, "Percentage", &iter) == FALSE)
+ return;
+
+ dbus_message_iter_get_basic(&iter, &percentage);
+
+ btd_battery_update(battery, percentage);
+}
+
+static void provided_battery_removed_cb(GDBusProxy *proxy, void *user_data)
+{
+ struct battery_provider *provider = user_data;
+ struct btd_battery *battery;
+ const char *path = g_dbus_proxy_get_path(proxy);
+ const char *export_path;
+
+ export_path = &path[strlen(provider->path)];
+
+ if (strcmp(g_dbus_proxy_get_interface(proxy),
+ BATTERY_PROVIDER_INTERFACE) != 0)
+ return;
+
+ DBG("provided battery removed %s", g_dbus_proxy_get_path(proxy));
+
+ battery = find_battery_by_path(export_path);
+ if (!battery)
+ return;
+
+ if (g_strcmp0(battery->provider_path, provider->path) != 0)
+ return;
+
+ g_dbus_proxy_set_property_watch(proxy, NULL, NULL);
+
+ btd_battery_unregister(battery);
+}
+
+static bool match_provider_path(const void *data, const void *user_data)
+{
+ const struct battery_provider *provider = data;
+ const char *path = user_data;
+
+ return strcmp(provider->path, path) == 0;
+}
+
+static void unregister_if_path_has_prefix(void *data, void *user_data)
+{
+ struct btd_battery *battery = data;
+ struct battery_provider *provider = user_data;
+
+ if (g_strcmp0(battery->provider_path, provider->path) == 0)
+ btd_battery_unregister(battery);
+}
+
+static void battery_provider_free(gpointer data)
+{
+ struct battery_provider *provider = data;
+
+ /* Unregister batteries under the root path of provider->path */
+ queue_foreach(batteries, unregister_if_path_has_prefix, provider);
+
+ if (provider->owner)
+ g_free(provider->owner);
+
+ if (provider->path)
+ g_free(provider->path);
+
+ if (provider->client) {
+ g_dbus_client_set_disconnect_watch(provider->client, NULL,
+ NULL);
+ g_dbus_client_set_proxy_handlers(provider->client, NULL, NULL,
+ NULL, NULL);
+ g_dbus_client_unref(provider->client);
+ }
+
+ free(provider);
+}
+
+static struct battery_provider *
+battery_provider_new(DBusConnection *conn,
+ struct btd_battery_provider_manager *manager,
+ const char *path, const char *sender)
+{
+ struct battery_provider *provider;
+
+ provider = new0(struct battery_provider, 1);
+ provider->manager = manager;
+ provider->owner = g_strdup(sender);
+ provider->path = g_strdup(path);
+
+ provider->client = g_dbus_client_new_full(conn, sender, path, path);
+
+ if (!provider->client) {
+ error("error creating D-Bus client %s", path);
+ battery_provider_free(provider);
+ return NULL;
+ }
+
+ g_dbus_client_set_disconnect_watch(provider->client,
+ provider_disconnect_cb, provider);
+
+ g_dbus_client_set_proxy_handlers(provider->client,
+ provided_battery_added_cb,
+ provided_battery_removed_cb, NULL,
+ provider);
+
+ return provider;
+}
+
+static void provider_disconnect_cb(DBusConnection *conn, void *user_data)
+{
+ struct battery_provider *provider = user_data;
+ struct btd_battery_provider_manager *manager = provider->manager;
+
+ DBG("battery provider client disconnected %s root path %s",
+ provider->owner, provider->path);
+
+ if (!queue_find(manager->battery_providers, NULL, provider)) {
+ warn("Disconnection on a non-existing provider %s",
+ provider->path);
+ return;
+ }
+
+ queue_remove(manager->battery_providers, provider);
+ battery_provider_free(provider);
+}
+
+static DBusMessage *register_battery_provider(DBusConnection *conn,
+ DBusMessage *msg, void *user_data)
+{
+ struct btd_battery_provider_manager *manager = user_data;
+ const char *sender = dbus_message_get_sender(msg);
+ DBusMessageIter args;
+ const char *path;
+ struct battery_provider *provider;
+
+ if (!dbus_message_iter_init(msg, &args))
+ return btd_error_invalid_args(msg);
+
+ if (dbus_message_iter_get_arg_type(&args) != DBUS_TYPE_OBJECT_PATH)
+ return btd_error_invalid_args(msg);
+
+ dbus_message_iter_get_basic(&args, &path);
+
+ DBG("register battery provider path = %s", path);
+
+ if (!g_str_has_prefix(path, "/"))
+ return btd_error_invalid_args(msg);
+
+ if (queue_find(manager->battery_providers, match_provider_path, path)) {
+ return dbus_message_new_error(msg,
+ ERROR_INTERFACE ".AlreadyExists",
+ "Provider already exists");
+ }
+
+ provider = battery_provider_new(conn, manager, path, sender);
+ queue_push_head(manager->battery_providers, provider);
+
+ return dbus_message_new_method_return(msg);
+}
+
+static DBusMessage *unregister_battery_provider(DBusConnection *conn,
+ DBusMessage *msg,
+ void *user_data)
+{
+ struct btd_battery_provider_manager *manager = user_data;
+ const char *sender = dbus_message_get_sender(msg);
+ DBusMessageIter args;
+ const char *path;
+ struct battery_provider *provider;
+
+ if (!dbus_message_iter_init(msg, &args))
+ return btd_error_invalid_args(msg);
+
+ if (dbus_message_iter_get_arg_type(&args) != DBUS_TYPE_OBJECT_PATH)
+ return btd_error_invalid_args(msg);
+
+ dbus_message_iter_get_basic(&args, &path);
+
+ DBG("unregister battery provider path = %s", path);
+
+ provider = queue_find(manager->battery_providers, match_provider_path,
+ path);
+ if (!provider || strcmp(provider->owner, sender) != 0) {
+ return dbus_message_new_error(msg,
+ ERROR_INTERFACE ".DoesNotExist",
+ "Provider does not exist");
+ }
+
+ queue_remove(manager->battery_providers, provider);
+ battery_provider_free(provider);
+
+ return dbus_message_new_method_return(msg);
+}
+
+static const GDBusMethodTable methods[] = {
+ { GDBUS_EXPERIMENTAL_METHOD("RegisterBatteryProvider",
+ GDBUS_ARGS({ "provider", "o" }), NULL,
+ register_battery_provider) },
+ { GDBUS_EXPERIMENTAL_METHOD("UnregisterBatteryProvider",
+ GDBUS_ARGS({ "provider", "o" }), NULL,
+ unregister_battery_provider) },
+ {}
+};
+
+static struct btd_battery_provider_manager *
+manager_new(struct btd_adapter *adapter)
+{
+ struct btd_battery_provider_manager *manager;
+
+ DBG("");
+
+ manager = new0(struct btd_battery_provider_manager, 1);
+ manager->adapter = adapter;
+ manager->battery_providers = queue_new();
+
+ return manager;
+}
+
+static void manager_free(struct btd_battery_provider_manager *manager)
+{
+ if (!manager)
+ return;
+
+ DBG("");
+
+ queue_destroy(manager->battery_providers, battery_provider_free);
+
+ free(manager);
+}
+
+struct btd_battery_provider_manager *
+btd_battery_provider_manager_create(struct btd_adapter *adapter)
+{
+ struct btd_battery_provider_manager *manager;
+
+ if (!adapter)
+ return NULL;
+
+ manager = manager_new(adapter);
+ if (!manager)
+ return NULL;
+
+ if (!g_dbus_register_interface(btd_get_dbus_connection(),
+ adapter_get_path(manager->adapter),
+ BATTERY_PROVIDER_MANAGER_INTERFACE,
+ methods, NULL, NULL, manager, NULL)) {
+ error("error registering " BATTERY_PROVIDER_MANAGER_INTERFACE
+ " interface");
+ manager_free(manager);
+ return NULL;
+ }
+
+ info("Battery Provider Manager created");
+
+ return manager;
+}
+
+void btd_battery_provider_manager_destroy(
+ struct btd_battery_provider_manager *manager)
+{
+ if (!manager)
+ return;
+
+ g_dbus_unregister_interface(btd_get_dbus_connection(),
+ adapter_get_path(manager->adapter),
+ BATTERY_PROVIDER_MANAGER_INTERFACE);
+
+ info("Battery Provider Manager destroyed");
+
+ manager_free(manager);
+}
diff --git a/src/battery.h b/src/battery.h
index ff63454cd..271659474 100644
--- a/src/battery.h
+++ b/src/battery.h
@@ -8,8 +8,16 @@
*
*/
+struct btd_adapter;
struct btd_battery;
+struct btd_battery_provider_manager;
-struct btd_battery *btd_battery_register(const char *path, const char *source);
+struct btd_battery *btd_battery_register(const char *path, const char *source,
+ const char *provider_path);
bool btd_battery_unregister(struct btd_battery *battery);
bool btd_battery_update(struct btd_battery *battery, uint8_t percentage);
+
+struct btd_battery_provider_manager *
+btd_battery_provider_manager_create(struct btd_adapter *adapter);
+void btd_battery_provider_manager_destroy(
+ struct btd_battery_provider_manager *manager);
--
2.26.2
The public function is motivated by the Battery Provider API code which
needs to probe whether a device exists.
Reviewed-by: Miao-chen Chou <[email protected]>
---
src/adapter.c | 33 ++++++++++++++++++++++++---------
src/adapter.h | 2 ++
2 files changed, 26 insertions(+), 9 deletions(-)
diff --git a/src/adapter.c b/src/adapter.c
index 56d0c6eaa..03d9d29e9 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -872,6 +872,30 @@ struct btd_device *btd_adapter_find_device(struct btd_adapter *adapter,
return device;
}
+static int device_path_cmp(gconstpointer a, gconstpointer b)
+{
+ const struct btd_device *device = a;
+ const char *path = b;
+ const char *dev_path = device_get_path(device);
+
+ return strcasecmp(dev_path, path);
+}
+
+struct btd_device *btd_adapter_find_device_by_path(struct btd_adapter *adapter,
+ const char *path)
+{
+ GSList *list;
+
+ if (!adapter)
+ return NULL;
+
+ list = g_slist_find_custom(adapter->devices, path, device_path_cmp);
+ if (!list)
+ return NULL;
+
+ return list->data;
+}
+
static void uuid_to_uuid128(uuid_t *uuid128, const uuid_t *uuid)
{
if (uuid->type == SDP_UUID16)
@@ -3192,15 +3216,6 @@ static gboolean property_get_roles(const GDBusPropertyTable *property,
return TRUE;
}
-static int device_path_cmp(gconstpointer a, gconstpointer b)
-{
- const struct btd_device *device = a;
- const char *path = b;
- const char *dev_path = device_get_path(device);
-
- return strcasecmp(dev_path, path);
-}
-
static DBusMessage *remove_device(DBusConnection *conn,
DBusMessage *msg, void *user_data)
{
diff --git a/src/adapter.h b/src/adapter.h
index e5750a37b..60b5e3bcc 100644
--- a/src/adapter.h
+++ b/src/adapter.h
@@ -83,6 +83,8 @@ sdp_list_t *btd_adapter_get_services(struct btd_adapter *adapter);
struct btd_device *btd_adapter_find_device(struct btd_adapter *adapter,
const bdaddr_t *dst,
uint8_t dst_type);
+struct btd_device *btd_adapter_find_device_by_path(struct btd_adapter *adapter,
+ const char *path);
const char *adapter_get_path(struct btd_adapter *adapter);
const bdaddr_t *btd_adapter_get_address(struct btd_adapter *adapter);
--
2.26.2
Hi Sonny,
On Fri, Nov 20, 2020 at 1:00 PM Sonny Sasaka <[email protected]> wrote:
>
> This patch add the documentation of the Battery Provider which lets
> external clients feed battery information to BlueZ if they are able to
> decode battery reporting via any profile. BlueZ UI clients can then use
> the org.bluez.Battery1 API as a single source of battery information
> coming from many different profiles.
>
> Reviewed-by: Miao-chen Chou <[email protected]>
>
> ---
> Changes in v3:
> * Remove doc duplication in BatteryProvider1 and mention that it's the
> same as Battery1 instead.
> * Suggest profile UUID in Source property.
>
> doc/battery-api.txt | 49 +++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 49 insertions(+)
>
> diff --git a/doc/battery-api.txt b/doc/battery-api.txt
> index dc7dbeda2..b5c9a7be1 100644
> --- a/doc/battery-api.txt
> +++ b/doc/battery-api.txt
> @@ -12,3 +12,52 @@ Object path [variable prefix]/{hci0,hci1,...}/dev_XX_XX_XX_XX_XX_XX
> Properties byte Percentage [readonly]
>
> The percentage of battery left as an unsigned 8-bit integer.
> +
> + string Source [readonly, optional, experimental]
> +
> + Describes where the battery information comes from
> + This property is informational only and may be useful
> + for debugging purposes.
> + Providers from BatteryProvider1 may make use of this
> + property to indicate where the battery report comes from
> + (e.g. "HFP 1.7", "HID", or the profile UUID).
We might need to remove the version number here since there is no
equivalent on UUID, in fact friendly names may be a bad idea after all
since for new profiles we may not have a friendly name to do the
translation and since this is property that would be hard to notify
the provider that we don't understand what is the Source while UUIDs,
if well formatted, should not have this problem so Id just get rid of
the use of friendly names altogether and expect the Source to be a
128bits UUID in string format.
> +
> +
> +Battery Provider Manager hierarchy
> +==================================
> +A battery provider starts by registering itself as a battery provider with the
> +RegisterBatteryProvider method passing an object path as the provider ID. Then,
> +it can start exposing org.bluez.BatteryProvider1 objects having the path
> +starting with the given provider ID. It can also remove objects at any time.
> +The objects and their properties exposed by battery providers will be reflected
> +on org.bluez.Battery1 interface.
> +
> +BlueZ will stop monitoring these exposed and removed objects after
> +UnregisterBatteryProvider is called for that provider ID.
> +
> +Service org.bluez
> +Interface org.bluez.BatteryProviderManager1 [experimental]
> +Object path /org/bluez/{hci0,hci1,...}
> +
> +Methods void RegisterBatteryProvider(object provider)
> +
> + This registers a battery provider. A registered
> + battery provider can then expose objects with
> + org.bluez.BatteryProvider1 interface described below.
We should probably mention this expects an object implementing
ObjectManaged in order to list the Battery1 provider.
> + void UnregisterBatteryProvider(object provider)
> +
> + This unregisters a battery provider. After
> + unregistration, the BatteryProvider1 objects provided
> + by this client are ignored by BlueZ.
> +
> +
> +Battery Provider hierarchy
> +==========================
> +
> +Service <client D-Bus address>
> +Interface org.bluez.BatteryProvider1 [experimental]
> +Object path {provider_root}/org/bluez/{hci0,hci1,...}/dev_XX_XX_XX_XX_XX_XX
If this is on the client the object path does not necessarily need to
follow our object hierarchy.
> +
> +Properties Objects provided on this interface contain the same properties
> + as org.bluez.Battery1 interface.
> --
> 2.26.2
--
Luiz Augusto von Dentz
Hi Luiz,
On Tue, Nov 24, 2020 at 1:21 PM Luiz Augusto von Dentz
<[email protected]> wrote:
>
> Hi Sonny,
>
> On Fri, Nov 20, 2020 at 1:00 PM Sonny Sasaka <[email protected]> wrote:
> >
> > This patch add the documentation of the Battery Provider which lets
> > external clients feed battery information to BlueZ if they are able to
> > decode battery reporting via any profile. BlueZ UI clients can then use
> > the org.bluez.Battery1 API as a single source of battery information
> > coming from many different profiles.
> >
> > Reviewed-by: Miao-chen Chou <[email protected]>
> >
> > ---
> > Changes in v3:
> > * Remove doc duplication in BatteryProvider1 and mention that it's the
> > same as Battery1 instead.
> > * Suggest profile UUID in Source property.
> >
> > doc/battery-api.txt | 49 +++++++++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 49 insertions(+)
> >
> > diff --git a/doc/battery-api.txt b/doc/battery-api.txt
> > index dc7dbeda2..b5c9a7be1 100644
> > --- a/doc/battery-api.txt
> > +++ b/doc/battery-api.txt
> > @@ -12,3 +12,52 @@ Object path [variable prefix]/{hci0,hci1,...}/dev_XX_XX_XX_XX_XX_XX
> > Properties byte Percentage [readonly]
> >
> > The percentage of battery left as an unsigned 8-bit integer.
> > +
> > + string Source [readonly, optional, experimental]
> > +
> > + Describes where the battery information comes from
> > + This property is informational only and may be useful
> > + for debugging purposes.
> > + Providers from BatteryProvider1 may make use of this
> > + property to indicate where the battery report comes from
> > + (e.g. "HFP 1.7", "HID", or the profile UUID).
>
> We might need to remove the version number here since there is no
> equivalent on UUID, in fact friendly names may be a bad idea after all
> since for new profiles we may not have a friendly name to do the
> translation and since this is property that would be hard to notify
> the provider that we don't understand what is the Source while UUIDs,
> if well formatted, should not have this problem so Id just get rid of
> the use of friendly names altogether and expect the Source to be a
> 128bits UUID in string format.
Ack. Will change to 128bit UUID format.
>
> > +
> > +
> > +Battery Provider Manager hierarchy
> > +==================================
> > +A battery provider starts by registering itself as a battery provider with the
> > +RegisterBatteryProvider method passing an object path as the provider ID. Then,
> > +it can start exposing org.bluez.BatteryProvider1 objects having the path
> > +starting with the given provider ID. It can also remove objects at any time.
> > +The objects and their properties exposed by battery providers will be reflected
> > +on org.bluez.Battery1 interface.
> > +
> > +BlueZ will stop monitoring these exposed and removed objects after
> > +UnregisterBatteryProvider is called for that provider ID.
> > +
> > +Service org.bluez
> > +Interface org.bluez.BatteryProviderManager1 [experimental]
> > +Object path /org/bluez/{hci0,hci1,...}
> > +
> > +Methods void RegisterBatteryProvider(object provider)
> > +
> > + This registers a battery provider. A registered
> > + battery provider can then expose objects with
> > + org.bluez.BatteryProvider1 interface described below.
>
> We should probably mention this expects an object implementing
> ObjectManaged in order to list the Battery1 provider.
Ack. Will add more explanation.
>
> > + void UnregisterBatteryProvider(object provider)
> > +
> > + This unregisters a battery provider. After
> > + unregistration, the BatteryProvider1 objects provided
> > + by this client are ignored by BlueZ.
> > +
> > +
> > +Battery Provider hierarchy
> > +==========================
> > +
> > +Service <client D-Bus address>
> > +Interface org.bluez.BatteryProvider1 [experimental]
> > +Object path {provider_root}/org/bluez/{hci0,hci1,...}/dev_XX_XX_XX_XX_XX_XX
>
> If this is on the client the object path does not necessarily need to
> follow our object hierarchy.
We need a convention to match the exposed object by the battery
provider and BlueZ's device. I am suggesting that the simplest
convention is to use the same path of the BlueZ's device object, which
is easy to follow and implement by providers. Otherwise, we would
still need another convention to match them, but I think any other
convention is likely more complex to implement by battery providers.
Can you suggest an alternative convention to match the battery and the
device?
>
> > +
> > +Properties Objects provided on this interface contain the same properties
> > + as org.bluez.Battery1 interface.
> > --
> > 2.26.2
>
>
>
> --
> Luiz Augusto von Dentz
Hi Sonny,
On Tue, Nov 24, 2020 at 1:30 PM Sonny Sasaka <[email protected]> wrote:
>
> Hi Luiz,
>
>
> On Tue, Nov 24, 2020 at 1:21 PM Luiz Augusto von Dentz
> <[email protected]> wrote:
> >
> > Hi Sonny,
> >
> > On Fri, Nov 20, 2020 at 1:00 PM Sonny Sasaka <[email protected]> wrote:
> > >
> > > This patch add the documentation of the Battery Provider which lets
> > > external clients feed battery information to BlueZ if they are able to
> > > decode battery reporting via any profile. BlueZ UI clients can then use
> > > the org.bluez.Battery1 API as a single source of battery information
> > > coming from many different profiles.
> > >
> > > Reviewed-by: Miao-chen Chou <[email protected]>
> > >
> > > ---
> > > Changes in v3:
> > > * Remove doc duplication in BatteryProvider1 and mention that it's the
> > > same as Battery1 instead.
> > > * Suggest profile UUID in Source property.
> > >
> > > doc/battery-api.txt | 49 +++++++++++++++++++++++++++++++++++++++++++++
> > > 1 file changed, 49 insertions(+)
> > >
> > > diff --git a/doc/battery-api.txt b/doc/battery-api.txt
> > > index dc7dbeda2..b5c9a7be1 100644
> > > --- a/doc/battery-api.txt
> > > +++ b/doc/battery-api.txt
> > > @@ -12,3 +12,52 @@ Object path [variable prefix]/{hci0,hci1,...}/dev_XX_XX_XX_XX_XX_XX
> > > Properties byte Percentage [readonly]
> > >
> > > The percentage of battery left as an unsigned 8-bit integer.
> > > +
> > > + string Source [readonly, optional, experimental]
> > > +
> > > + Describes where the battery information comes from
> > > + This property is informational only and may be useful
> > > + for debugging purposes.
> > > + Providers from BatteryProvider1 may make use of this
> > > + property to indicate where the battery report comes from
> > > + (e.g. "HFP 1.7", "HID", or the profile UUID).
> >
> > We might need to remove the version number here since there is no
> > equivalent on UUID, in fact friendly names may be a bad idea after all
> > since for new profiles we may not have a friendly name to do the
> > translation and since this is property that would be hard to notify
> > the provider that we don't understand what is the Source while UUIDs,
> > if well formatted, should not have this problem so Id just get rid of
> > the use of friendly names altogether and expect the Source to be a
> > 128bits UUID in string format.
> Ack. Will change to 128bit UUID format.
> >
> > > +
> > > +
> > > +Battery Provider Manager hierarchy
> > > +==================================
> > > +A battery provider starts by registering itself as a battery provider with the
> > > +RegisterBatteryProvider method passing an object path as the provider ID. Then,
> > > +it can start exposing org.bluez.BatteryProvider1 objects having the path
> > > +starting with the given provider ID. It can also remove objects at any time.
> > > +The objects and their properties exposed by battery providers will be reflected
> > > +on org.bluez.Battery1 interface.
> > > +
> > > +BlueZ will stop monitoring these exposed and removed objects after
> > > +UnregisterBatteryProvider is called for that provider ID.
> > > +
> > > +Service org.bluez
> > > +Interface org.bluez.BatteryProviderManager1 [experimental]
> > > +Object path /org/bluez/{hci0,hci1,...}
> > > +
> > > +Methods void RegisterBatteryProvider(object provider)
> > > +
> > > + This registers a battery provider. A registered
> > > + battery provider can then expose objects with
> > > + org.bluez.BatteryProvider1 interface described below.
> >
> > We should probably mention this expects an object implementing
> > ObjectManaged in order to list the Battery1 provider.
> Ack. Will add more explanation.
> >
> > > + void UnregisterBatteryProvider(object provider)
> > > +
> > > + This unregisters a battery provider. After
> > > + unregistration, the BatteryProvider1 objects provided
> > > + by this client are ignored by BlueZ.
> > > +
> > > +
> > > +Battery Provider hierarchy
> > > +==========================
> > > +
> > > +Service <client D-Bus address>
> > > +Interface org.bluez.BatteryProvider1 [experimental]
> > > +Object path {provider_root}/org/bluez/{hci0,hci1,...}/dev_XX_XX_XX_XX_XX_XX
> >
> > If this is on the client the object path does not necessarily need to
> > follow our object hierarchy.
> We need a convention to match the exposed object by the battery
> provider and BlueZ's device. I am suggesting that the simplest
> convention is to use the same path of the BlueZ's device object, which
> is easy to follow and implement by providers. Otherwise, we would
> still need another convention to match them, but I think any other
> convention is likely more complex to implement by battery providers.
> Can you suggest an alternative convention to match the battery and the
> device?
I thought the objects would be registered directly on the device
object itself but it looks like it is on the adapter thus why you need
the device in the first place, but if you are using the object path it
is just a matter of moving BatteryProviderManager1 to the device
object.
> >
> > > +
> > > +Properties Objects provided on this interface contain the same properties
> > > + as org.bluez.Battery1 interface.
> > > --
> > > 2.26.2
> >
> >
> >
> > --
> > Luiz Augusto von Dentz
--
Luiz Augusto von Dentz
Hi Luiz,
On Tue, Nov 24, 2020 at 4:23 PM Luiz Augusto von Dentz
<[email protected]> wrote:
>
> Hi Sonny,
>
> On Tue, Nov 24, 2020 at 1:30 PM Sonny Sasaka <[email protected]> wrote:
> >
> > Hi Luiz,
> >
> >
> > On Tue, Nov 24, 2020 at 1:21 PM Luiz Augusto von Dentz
> > <[email protected]> wrote:
> > >
> > > Hi Sonny,
> > >
> > > On Fri, Nov 20, 2020 at 1:00 PM Sonny Sasaka <[email protected]> wrote:
> > > >
> > > > This patch add the documentation of the Battery Provider which lets
> > > > external clients feed battery information to BlueZ if they are able to
> > > > decode battery reporting via any profile. BlueZ UI clients can then use
> > > > the org.bluez.Battery1 API as a single source of battery information
> > > > coming from many different profiles.
> > > >
> > > > Reviewed-by: Miao-chen Chou <[email protected]>
> > > >
> > > > ---
> > > > Changes in v3:
> > > > * Remove doc duplication in BatteryProvider1 and mention that it's the
> > > > same as Battery1 instead.
> > > > * Suggest profile UUID in Source property.
> > > >
> > > > doc/battery-api.txt | 49 +++++++++++++++++++++++++++++++++++++++++++++
> > > > 1 file changed, 49 insertions(+)
> > > >
> > > > diff --git a/doc/battery-api.txt b/doc/battery-api.txt
> > > > index dc7dbeda2..b5c9a7be1 100644
> > > > --- a/doc/battery-api.txt
> > > > +++ b/doc/battery-api.txt
> > > > @@ -12,3 +12,52 @@ Object path [variable prefix]/{hci0,hci1,...}/dev_XX_XX_XX_XX_XX_XX
> > > > Properties byte Percentage [readonly]
> > > >
> > > > The percentage of battery left as an unsigned 8-bit integer.
> > > > +
> > > > + string Source [readonly, optional, experimental]
> > > > +
> > > > + Describes where the battery information comes from
> > > > + This property is informational only and may be useful
> > > > + for debugging purposes.
> > > > + Providers from BatteryProvider1 may make use of this
> > > > + property to indicate where the battery report comes from
> > > > + (e.g. "HFP 1.7", "HID", or the profile UUID).
> > >
> > > We might need to remove the version number here since there is no
> > > equivalent on UUID, in fact friendly names may be a bad idea after all
> > > since for new profiles we may not have a friendly name to do the
> > > translation and since this is property that would be hard to notify
> > > the provider that we don't understand what is the Source while UUIDs,
> > > if well formatted, should not have this problem so Id just get rid of
> > > the use of friendly names altogether and expect the Source to be a
> > > 128bits UUID in string format.
> > Ack. Will change to 128bit UUID format.
> > >
> > > > +
> > > > +
> > > > +Battery Provider Manager hierarchy
> > > > +==================================
> > > > +A battery provider starts by registering itself as a battery provider with the
> > > > +RegisterBatteryProvider method passing an object path as the provider ID. Then,
> > > > +it can start exposing org.bluez.BatteryProvider1 objects having the path
> > > > +starting with the given provider ID. It can also remove objects at any time.
> > > > +The objects and their properties exposed by battery providers will be reflected
> > > > +on org.bluez.Battery1 interface.
> > > > +
> > > > +BlueZ will stop monitoring these exposed and removed objects after
> > > > +UnregisterBatteryProvider is called for that provider ID.
> > > > +
> > > > +Service org.bluez
> > > > +Interface org.bluez.BatteryProviderManager1 [experimental]
> > > > +Object path /org/bluez/{hci0,hci1,...}
> > > > +
> > > > +Methods void RegisterBatteryProvider(object provider)
> > > > +
> > > > + This registers a battery provider. A registered
> > > > + battery provider can then expose objects with
> > > > + org.bluez.BatteryProvider1 interface described below.
> > >
> > > We should probably mention this expects an object implementing
> > > ObjectManaged in order to list the Battery1 provider.
> > Ack. Will add more explanation.
> > >
> > > > + void UnregisterBatteryProvider(object provider)
> > > > +
> > > > + This unregisters a battery provider. After
> > > > + unregistration, the BatteryProvider1 objects provided
> > > > + by this client are ignored by BlueZ.
> > > > +
> > > > +
> > > > +Battery Provider hierarchy
> > > > +==========================
> > > > +
> > > > +Service <client D-Bus address>
> > > > +Interface org.bluez.BatteryProvider1 [experimental]
> > > > +Object path {provider_root}/org/bluez/{hci0,hci1,...}/dev_XX_XX_XX_XX_XX_XX
> > >
> > > If this is on the client the object path does not necessarily need to
> > > follow our object hierarchy.
> > We need a convention to match the exposed object by the battery
> > provider and BlueZ's device. I am suggesting that the simplest
> > convention is to use the same path of the BlueZ's device object, which
> > is easy to follow and implement by providers. Otherwise, we would
> > still need another convention to match them, but I think any other
> > convention is likely more complex to implement by battery providers.
> > Can you suggest an alternative convention to match the battery and the
> > device?
>
> I thought the objects would be registered directly on the device
> object itself but it looks like it is on the adapter thus why you need
> the device in the first place, but if you are using the object path it
> is just a matter of moving BatteryProviderManager1 to the device
> object.
If we move BatteryProviderManager1 to the device object, that means we
can't use the object manager style and providers have to register each
battery once rather than registering once in the beginning and expose
several objects afterwards, so this would lose your suggestion to use
object manager in the first place. I prefer we stick to using object
manager style, it is simple, easy to understand and implement for
providers (refer to my python test app in one of the patches in this
series).
>
> > >
> > > > +
> > > > +Properties Objects provided on this interface contain the same properties
> > > > + as org.bluez.Battery1 interface.
> > > > --
> > > > 2.26.2
> > >
> > >
> > >
> > > --
> > > Luiz Augusto von Dentz
>
>
>
> --
> Luiz Augusto von Dentz
On Tue, 2020-11-24 at 13:29 -0800, Sonny Sasaka wrote:
>
> We need a convention to match the exposed object by the battery
> provider and BlueZ's device. I am suggesting that the simplest
> convention is to use the same path of the BlueZ's device object,
> which
> is easy to follow and implement by providers. Otherwise, we would
> still need another convention to match them, but I think any other
> convention is likely more complex to implement by battery providers.
> Can you suggest an alternative convention to match the battery and
> the
> device?
You should match on the interface being available, not the object
path.
UPower does that, it just watches for ObjectManager signals, and checks
whether the expected interface is available when a new object appears:
https://gitlab.freedesktop.org/upower/upower/-/blob/master/src/linux/up-backend.c#L314-357
There's no reason to care about the object path.
On Tue, 2020-11-24 at 17:20 -0800, Sonny Sasaka wrote:
>
> If we move BatteryProviderManager1 to the device object, that means
> we
> can't use the object manager style and providers have to register
> each
> battery once rather than registering once in the beginning and expose
> several objects afterwards, so this would lose your suggestion to use
> object manager in the first place. I prefer we stick to using object
> manager style, it is simple, easy to understand and implement for
> providers (refer to my python test app in one of the patches in this
> series).
org.freedesktop.DBus.ObjectManager.InterfacesAdded would show the
interface appearing. It's also what UPower expects bluez to do.
Hi Sonny,
On Tue, Nov 24, 2020 at 5:20 PM Sonny Sasaka <[email protected]> wrote:
>
> Hi Luiz,
>
> On Tue, Nov 24, 2020 at 4:23 PM Luiz Augusto von Dentz
> <[email protected]> wrote:
> >
> > Hi Sonny,
> >
> > On Tue, Nov 24, 2020 at 1:30 PM Sonny Sasaka <[email protected]> wrote:
> > >
> > > Hi Luiz,
> > >
> > >
> > > On Tue, Nov 24, 2020 at 1:21 PM Luiz Augusto von Dentz
> > > <[email protected]> wrote:
> > > >
> > > > Hi Sonny,
> > > >
> > > > On Fri, Nov 20, 2020 at 1:00 PM Sonny Sasaka <[email protected]> wrote:
> > > > >
> > > > > This patch add the documentation of the Battery Provider which lets
> > > > > external clients feed battery information to BlueZ if they are able to
> > > > > decode battery reporting via any profile. BlueZ UI clients can then use
> > > > > the org.bluez.Battery1 API as a single source of battery information
> > > > > coming from many different profiles.
> > > > >
> > > > > Reviewed-by: Miao-chen Chou <[email protected]>
> > > > >
> > > > > ---
> > > > > Changes in v3:
> > > > > * Remove doc duplication in BatteryProvider1 and mention that it's the
> > > > > same as Battery1 instead.
> > > > > * Suggest profile UUID in Source property.
> > > > >
> > > > > doc/battery-api.txt | 49 +++++++++++++++++++++++++++++++++++++++++++++
> > > > > 1 file changed, 49 insertions(+)
> > > > >
> > > > > diff --git a/doc/battery-api.txt b/doc/battery-api.txt
> > > > > index dc7dbeda2..b5c9a7be1 100644
> > > > > --- a/doc/battery-api.txt
> > > > > +++ b/doc/battery-api.txt
> > > > > @@ -12,3 +12,52 @@ Object path [variable prefix]/{hci0,hci1,...}/dev_XX_XX_XX_XX_XX_XX
> > > > > Properties byte Percentage [readonly]
> > > > >
> > > > > The percentage of battery left as an unsigned 8-bit integer.
> > > > > +
> > > > > + string Source [readonly, optional, experimental]
> > > > > +
> > > > > + Describes where the battery information comes from
> > > > > + This property is informational only and may be useful
> > > > > + for debugging purposes.
> > > > > + Providers from BatteryProvider1 may make use of this
> > > > > + property to indicate where the battery report comes from
> > > > > + (e.g. "HFP 1.7", "HID", or the profile UUID).
> > > >
> > > > We might need to remove the version number here since there is no
> > > > equivalent on UUID, in fact friendly names may be a bad idea after all
> > > > since for new profiles we may not have a friendly name to do the
> > > > translation and since this is property that would be hard to notify
> > > > the provider that we don't understand what is the Source while UUIDs,
> > > > if well formatted, should not have this problem so Id just get rid of
> > > > the use of friendly names altogether and expect the Source to be a
> > > > 128bits UUID in string format.
> > > Ack. Will change to 128bit UUID format.
> > > >
> > > > > +
> > > > > +
> > > > > +Battery Provider Manager hierarchy
> > > > > +==================================
> > > > > +A battery provider starts by registering itself as a battery provider with the
> > > > > +RegisterBatteryProvider method passing an object path as the provider ID. Then,
> > > > > +it can start exposing org.bluez.BatteryProvider1 objects having the path
> > > > > +starting with the given provider ID. It can also remove objects at any time.
> > > > > +The objects and their properties exposed by battery providers will be reflected
> > > > > +on org.bluez.Battery1 interface.
> > > > > +
> > > > > +BlueZ will stop monitoring these exposed and removed objects after
> > > > > +UnregisterBatteryProvider is called for that provider ID.
> > > > > +
> > > > > +Service org.bluez
> > > > > +Interface org.bluez.BatteryProviderManager1 [experimental]
> > > > > +Object path /org/bluez/{hci0,hci1,...}
> > > > > +
> > > > > +Methods void RegisterBatteryProvider(object provider)
> > > > > +
> > > > > + This registers a battery provider. A registered
> > > > > + battery provider can then expose objects with
> > > > > + org.bluez.BatteryProvider1 interface described below.
> > > >
> > > > We should probably mention this expects an object implementing
> > > > ObjectManaged in order to list the Battery1 provider.
> > > Ack. Will add more explanation.
> > > >
> > > > > + void UnregisterBatteryProvider(object provider)
> > > > > +
> > > > > + This unregisters a battery provider. After
> > > > > + unregistration, the BatteryProvider1 objects provided
> > > > > + by this client are ignored by BlueZ.
> > > > > +
> > > > > +
> > > > > +Battery Provider hierarchy
> > > > > +==========================
> > > > > +
> > > > > +Service <client D-Bus address>
> > > > > +Interface org.bluez.BatteryProvider1 [experimental]
> > > > > +Object path {provider_root}/org/bluez/{hci0,hci1,...}/dev_XX_XX_XX_XX_XX_XX
> > > >
> > > > If this is on the client the object path does not necessarily need to
> > > > follow our object hierarchy.
> > > We need a convention to match the exposed object by the battery
> > > provider and BlueZ's device. I am suggesting that the simplest
> > > convention is to use the same path of the BlueZ's device object, which
> > > is easy to follow and implement by providers. Otherwise, we would
> > > still need another convention to match them, but I think any other
> > > convention is likely more complex to implement by battery providers.
> > > Can you suggest an alternative convention to match the battery and the
> > > device?
> >
> > I thought the objects would be registered directly on the device
> > object itself but it looks like it is on the adapter thus why you need
> > the device in the first place, but if you are using the object path it
> > is just a matter of moving BatteryProviderManager1 to the device
> > object.
> If we move BatteryProviderManager1 to the device object, that means we
> can't use the object manager style and providers have to register each
> battery once rather than registering once in the beginning and expose
> several objects afterwards, so this would lose your suggestion to use
> object manager in the first place. I prefer we stick to using object
> manager style, it is simple, easy to understand and implement for
> providers (refer to my python test app in one of the patches in this
> series).
Well not really, you can still use the object manager style the only
difference is that you will register on a per-device basis instead of
per adapter, not every device is going to have battery providers but
some can have multiple providers from different profiles, but I guess
you were suggesting that one could register a single time per adapter
so we don't have round trips of registration per device in which case
then I would prefer to just have a property indicating the device
object path, but note that if there are different entities handling
different profiles each would have to register individually anyway.
> >
> > > >
> > > > > +
> > > > > +Properties Objects provided on this interface contain the same properties
> > > > > + as org.bluez.Battery1 interface.
> > > > > --
> > > > > 2.26.2
> > > >
> > > >
> > > >
> > > > --
> > > > Luiz Augusto von Dentz
> >
> >
> >
> > --
> > Luiz Augusto von Dentz
--
Luiz Augusto von Dentz
Hi Luiz,
On Sat, Nov 28, 2020 at 10:16 PM Luiz Augusto von Dentz
<[email protected]> wrote:
>
> Hi Sonny,
>
> On Tue, Nov 24, 2020 at 5:20 PM Sonny Sasaka <[email protected]> wrote:
> >
> > Hi Luiz,
> >
> > On Tue, Nov 24, 2020 at 4:23 PM Luiz Augusto von Dentz
> > <[email protected]> wrote:
> > >
> > > Hi Sonny,
> > >
> > > On Tue, Nov 24, 2020 at 1:30 PM Sonny Sasaka <[email protected]> wrote:
> > > >
> > > > Hi Luiz,
> > > >
> > > >
> > > > On Tue, Nov 24, 2020 at 1:21 PM Luiz Augusto von Dentz
> > > > <[email protected]> wrote:
> > > > >
> > > > > Hi Sonny,
> > > > >
> > > > > On Fri, Nov 20, 2020 at 1:00 PM Sonny Sasaka <[email protected]> wrote:
> > > > > >
> > > > > > This patch add the documentation of the Battery Provider which lets
> > > > > > external clients feed battery information to BlueZ if they are able to
> > > > > > decode battery reporting via any profile. BlueZ UI clients can then use
> > > > > > the org.bluez.Battery1 API as a single source of battery information
> > > > > > coming from many different profiles.
> > > > > >
> > > > > > Reviewed-by: Miao-chen Chou <[email protected]>
> > > > > >
> > > > > > ---
> > > > > > Changes in v3:
> > > > > > * Remove doc duplication in BatteryProvider1 and mention that it's the
> > > > > > same as Battery1 instead.
> > > > > > * Suggest profile UUID in Source property.
> > > > > >
> > > > > > doc/battery-api.txt | 49 +++++++++++++++++++++++++++++++++++++++++++++
> > > > > > 1 file changed, 49 insertions(+)
> > > > > >
> > > > > > diff --git a/doc/battery-api.txt b/doc/battery-api.txt
> > > > > > index dc7dbeda2..b5c9a7be1 100644
> > > > > > --- a/doc/battery-api.txt
> > > > > > +++ b/doc/battery-api.txt
> > > > > > @@ -12,3 +12,52 @@ Object path [variable prefix]/{hci0,hci1,...}/dev_XX_XX_XX_XX_XX_XX
> > > > > > Properties byte Percentage [readonly]
> > > > > >
> > > > > > The percentage of battery left as an unsigned 8-bit integer.
> > > > > > +
> > > > > > + string Source [readonly, optional, experimental]
> > > > > > +
> > > > > > + Describes where the battery information comes from
> > > > > > + This property is informational only and may be useful
> > > > > > + for debugging purposes.
> > > > > > + Providers from BatteryProvider1 may make use of this
> > > > > > + property to indicate where the battery report comes from
> > > > > > + (e.g. "HFP 1.7", "HID", or the profile UUID).
> > > > >
> > > > > We might need to remove the version number here since there is no
> > > > > equivalent on UUID, in fact friendly names may be a bad idea after all
> > > > > since for new profiles we may not have a friendly name to do the
> > > > > translation and since this is property that would be hard to notify
> > > > > the provider that we don't understand what is the Source while UUIDs,
> > > > > if well formatted, should not have this problem so Id just get rid of
> > > > > the use of friendly names altogether and expect the Source to be a
> > > > > 128bits UUID in string format.
> > > > Ack. Will change to 128bit UUID format.
> > > > >
> > > > > > +
> > > > > > +
> > > > > > +Battery Provider Manager hierarchy
> > > > > > +==================================
> > > > > > +A battery provider starts by registering itself as a battery provider with the
> > > > > > +RegisterBatteryProvider method passing an object path as the provider ID. Then,
> > > > > > +it can start exposing org.bluez.BatteryProvider1 objects having the path
> > > > > > +starting with the given provider ID. It can also remove objects at any time.
> > > > > > +The objects and their properties exposed by battery providers will be reflected
> > > > > > +on org.bluez.Battery1 interface.
> > > > > > +
> > > > > > +BlueZ will stop monitoring these exposed and removed objects after
> > > > > > +UnregisterBatteryProvider is called for that provider ID.
> > > > > > +
> > > > > > +Service org.bluez
> > > > > > +Interface org.bluez.BatteryProviderManager1 [experimental]
> > > > > > +Object path /org/bluez/{hci0,hci1,...}
> > > > > > +
> > > > > > +Methods void RegisterBatteryProvider(object provider)
> > > > > > +
> > > > > > + This registers a battery provider. A registered
> > > > > > + battery provider can then expose objects with
> > > > > > + org.bluez.BatteryProvider1 interface described below.
> > > > >
> > > > > We should probably mention this expects an object implementing
> > > > > ObjectManaged in order to list the Battery1 provider.
> > > > Ack. Will add more explanation.
> > > > >
> > > > > > + void UnregisterBatteryProvider(object provider)
> > > > > > +
> > > > > > + This unregisters a battery provider. After
> > > > > > + unregistration, the BatteryProvider1 objects provided
> > > > > > + by this client are ignored by BlueZ.
> > > > > > +
> > > > > > +
> > > > > > +Battery Provider hierarchy
> > > > > > +==========================
> > > > > > +
> > > > > > +Service <client D-Bus address>
> > > > > > +Interface org.bluez.BatteryProvider1 [experimental]
> > > > > > +Object path {provider_root}/org/bluez/{hci0,hci1,...}/dev_XX_XX_XX_XX_XX_XX
> > > > >
> > > > > If this is on the client the object path does not necessarily need to
> > > > > follow our object hierarchy.
> > > > We need a convention to match the exposed object by the battery
> > > > provider and BlueZ's device. I am suggesting that the simplest
> > > > convention is to use the same path of the BlueZ's device object, which
> > > > is easy to follow and implement by providers. Otherwise, we would
> > > > still need another convention to match them, but I think any other
> > > > convention is likely more complex to implement by battery providers.
> > > > Can you suggest an alternative convention to match the battery and the
> > > > device?
> > >
> > > I thought the objects would be registered directly on the device
> > > object itself but it looks like it is on the adapter thus why you need
> > > the device in the first place, but if you are using the object path it
> > > is just a matter of moving BatteryProviderManager1 to the device
> > > object.
> > If we move BatteryProviderManager1 to the device object, that means we
> > can't use the object manager style and providers have to register each
> > battery once rather than registering once in the beginning and expose
> > several objects afterwards, so this would lose your suggestion to use
> > object manager in the first place. I prefer we stick to using object
> > manager style, it is simple, easy to understand and implement for
> > providers (refer to my python test app in one of the patches in this
> > series).
>
> Well not really, you can still use the object manager style the only
> difference is that you will register on a per-device basis instead of
> per adapter, not every device is going to have battery providers but
> some can have multiple providers from different profiles, but I guess
> you were suggesting that one could register a single time per adapter
> so we don't have round trips of registration per device in which case
> then I would prefer to just have a property indicating the device
> object path, but note that if there are different entities handling
> different profiles each would have to register individually anyway.
I like your idea of explicitly specifying the device path for each
provided battery. I will send a v4 for this change. Please take another look.
>
> > >
> > > > >
> > > > > > +
> > > > > > +Properties Objects provided on this interface contain the same properties
> > > > > > + as org.bluez.Battery1 interface.
> > > > > > --
> > > > > > 2.26.2
> > > > >
> > > > >
> > > > >
> > > > > --
> > > > > Luiz Augusto von Dentz
> > >
> > >
> > >
> > > --
> > > Luiz Augusto von Dentz
>
>
>
> --
> Luiz Augusto von Dentz