diff mbox series

[BlueZ,v5,1/7] battery: Add the internal Battery API

Message ID 20201130215602.386545-1-sonnysasaka@chromium.org (mailing list archive)
State Accepted
Delegated to: Luiz Von Dentz
Headers show
Series [BlueZ,v5,1/7] battery: Add the internal Battery API | expand

Commit Message

Sonny Sasaka Nov. 30, 2020, 9:55 p.m. UTC 
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 <danielwinkler@google.com>

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

Comments

bluez.test.bot@gmail.com Nov. 30, 2020, 10:34 p.m. UTC | #1 
This is automated email and please do not reply to this email!

Dear submitter,

Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=393559

---Test result---

##############################
Test: CheckPatch - FAIL
Output:
battery: Add the internal Battery API
ERROR:INITIALISED_STATIC: do not initialise statics to NULL
#71: FILE: src/battery.c:38:
+static struct queue *batteries = NULL;

- total: 1 errors, 0 warnings, 215 lines checked

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

"[PATCH] battery: Add the internal Battery API" has style problems, please review.

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

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

test: Add test app for Battery Provider API
ERROR:EXECUTE_PERMISSIONS: do not set execute permissions for source files
#12: FILE: test/example-battery-provider

- total: 1 errors, 0 warnings, 232 lines checked

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

"[PATCH] test: Add test app for Battery Provider API" has style problems, please review.

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

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


##############################
Test: CheckGitLint - PASS

##############################
Test: CheckBuild - PASS

##############################
Test: MakeCheck - PASS



---
Regards,
Linux Bluetooth
Luiz Augusto von Dentz Dec. 1, 2020, 12:27 a.m. UTC | #2 
Hi Sonny,

On Mon, Nov 30, 2020 at 3:13 PM <bluez.test.bot@gmail.com> wrote:
>
> This is automated email and please do not reply to this email!
>
> Dear submitter,
>
> Thank you for submitting the patches to the linux bluetooth mailing list.
> This is a CI test results with your patch series:
> PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=393559
>
> ---Test result---
>
> ##############################
> Test: CheckPatch - FAIL
> Output:
> battery: Add the internal Battery API
> ERROR:INITIALISED_STATIC: do not initialise statics to NULL
> #71: FILE: src/battery.c:38:
> +static struct queue *batteries = NULL;
>
> - total: 1 errors, 0 warnings, 215 lines checked
>
> NOTE: For some of the reported defects, checkpatch may be able to
>       mechanically convert to the typical style using --fix or --fix-inplace.
>
> "[PATCH] battery: Add the internal Battery API" has style problems, please review.
>
> NOTE: Ignored message types: COMMIT_MESSAGE COMPLEX_MACRO CONST_STRUCT FILE_PATH_CHANGES MISSING_SIGN_OFF PREFER_PACKED SPLIT_STRING SSCANF_TO_KSTRTO
>
> NOTE: If any of the errors are false positives, please report
>       them to the maintainer, see CHECKPATCH in MAINTAINERS.
>
> test: Add test app for Battery Provider API
> ERROR:EXECUTE_PERMISSIONS: do not set execute permissions for source files
> #12: FILE: test/example-battery-provider
>
> - total: 1 errors, 0 warnings, 232 lines checked
>
> NOTE: For some of the reported defects, checkpatch may be able to
>       mechanically convert to the typical style using --fix or --fix-inplace.
>
> "[PATCH] test: Add test app for Battery Provider API" has style problems, please review.
>
> NOTE: Ignored message types: COMMIT_MESSAGE COMPLEX_MACRO CONST_STRUCT FILE_PATH_CHANGES MISSING_SIGN_OFF PREFER_PACKED SPLIT_STRING SSCANF_TO_KSTRTO
>
> NOTE: If any of the errors are false positives, please report
>       them to the maintainer, see CHECKPATCH in MAINTAINERS.
>
>
> ##############################
> Test: CheckGitLint - PASS
>
> ##############################
> Test: CheckBuild - PASS
>
> ##############################
> Test: MakeCheck - PASS

Applied, thanks.
Bastien Nocera Dec. 1, 2020, 4:29 p.m. UTC | #3 
On Mon, 2020-11-30 at 16:27 -0800, Luiz Augusto von Dentz wrote:
> Hi Sonny,
> <snip>
> Applied, thanks.

Missed it :/
Any chance you could update the org.bluez.BatteryProvider1 docs as per
my review?

Cheers
Sonny Sasaka Dec. 1, 2020, 7:15 p.m. UTC | #4 
Hi Bastien,

I will send a separate patch to update the doc.

On Tue, Dec 1, 2020 at 8:29 AM Bastien Nocera <hadess@hadess.net> wrote:
>
> On Mon, 2020-11-30 at 16:27 -0800, Luiz Augusto von Dentz wrote:
> > Hi Sonny,
> > <snip>
> > Applied, thanks.
>
> Missed it :/
> Any chance you could update the org.bluez.BatteryProvider1 docs as per
> my review?
>
> Cheers
>
diff mbox series

Patch

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);