[gnome-bluetooth/wip/hadess/power-state-bluez: 1/2] lib: Implement default-adapter-state property using bluez

[Bastien Nocera <hadess src gnome org>]

commit a3f41c1d8d5ad64509c02ea2614ed6125a5b4a2c
Author: Bastien Nocera <hadess hadess net>
Date:   Thu Aug 25 16:09:48 2022 +0200

    lib: Implement default-adapter-state property using bluez
    
    Use new adapter property in bluez to implement the
    default-adapter-state. It should work better, as it doesn't rely only on
    internal BluetoothClient state in a single process, and can get
    information when bluetoothd is powering up the adapter.
    
    Note that this property still works in the absence of a new enough
    bluetoothd, without the transitional states.
    
    Closes: #121

 lib/bluetooth-client.c    | 68 +++++++++++++++++++----------------------------
 lib/bluetooth-client.xml  |  1 +
 tests/integration-test.py |  8 +++---
 3 files changed, 34 insertions(+), 43 deletions(-)
---
diff --git a/lib/bluetooth-client.c b/lib/bluetooth-client.c
index a39317bb..48575d41 100644
--- a/lib/bluetooth-client.c
+++ b/lib/bluetooth-client.c
@@ -54,19 +54,12 @@
 #define BLUEZ_ADAPTER_INTERFACE                "org.bluez.Adapter1"
 #define BLUEZ_DEVICE_INTERFACE         "org.bluez.Device1"
 
-/* Subset of BluetoothAdapterState */
-typedef enum {
-       POWER_STATE_REQUEST_NONE = 0,
-       POWER_STATE_REQUEST_ON,
-       POWER_STATE_REQUEST_OFF,
-} PowerStateRequest;
-
 struct _BluetoothClient {
        GObject parent;
 
        GListStore *list_store;
        Adapter1 *default_adapter;
-       PowerStateRequest power_req;
+       gboolean has_power_state;
        GDBusObjectManager *manager;
        GCancellable *cancellable;
        guint num_adapters;
@@ -418,31 +411,15 @@ adapter_set_powered_cb (GDBusProxy *proxy,
                        GAsyncResult *res,
                        gpointer      user_data)
 {
-       BluetoothClient *client;
        g_autoptr(GError) error = NULL;
        g_autoptr(GVariant) ret = NULL;
 
        ret = g_dbus_proxy_call_finish (proxy, res, &error);
        if (!ret) {
-               if (g_error_matches (error, G_IO_ERROR, G_IO_ERROR_CANCELLED))
-                       return;
                g_debug ("Error setting property 'Powered' on %s: %s (%s, %d)",
                         g_dbus_proxy_get_object_path (proxy),
                         error->message, g_quark_to_string (error->domain), error->code);
        }
-
-       client = user_data;
-       if (client->default_adapter) {
-               gboolean powered;
-
-               powered = adapter1_get_powered (client->default_adapter);
-               if ((powered && client->power_req == POWER_STATE_REQUEST_ON) ||
-                   (!powered && client->power_req == POWER_STATE_REQUEST_OFF)) {
-                       /* Only reset if we don't have a conflicting state in progress */
-                       client->power_req = POWER_STATE_REQUEST_NONE;
-               }
-       }
-       g_object_notify (G_OBJECT (client), "default-adapter-state");
 }
 
 static void
@@ -458,12 +435,6 @@ adapter_set_powered (BluetoothClient *client,
                return;
        }
 
-       if ((powered && client->power_req == POWER_STATE_REQUEST_ON) ||
-           (!powered && client->power_req == POWER_STATE_REQUEST_OFF)) {
-               g_debug ("Default adapter is already being powered %s", powered ? "on" : "off");
-               return;
-       }
-
        if (powered == adapter1_get_powered (client->default_adapter)) {
                g_debug ("Default adapter is already %spowered", powered ? "" : "un");
                return;
@@ -473,9 +444,6 @@ adapter_set_powered (BluetoothClient *client,
                 powered ? "up" : "down",
                 g_dbus_proxy_get_object_path (G_DBUS_PROXY (client->default_adapter)));
        variant = g_variant_new_boolean (powered);
-       client->power_req = powered ?
-               POWER_STATE_REQUEST_ON : POWER_STATE_REQUEST_OFF;
-       g_object_notify (G_OBJECT (client), "default-adapter-state");
        g_dbus_proxy_call (G_DBUS_PROXY (client->default_adapter),
                           "org.freedesktop.DBus.Properties.Set",
                           g_variant_new ("(ssv)", "org.bluez.Adapter1", "Powered", variant),
@@ -588,6 +556,9 @@ adapter_notify_cb (Adapter1       *adapter,
                g_object_notify (G_OBJECT (client), "default-adapter-setup-mode");
        } else if (g_strcmp0 (property, "powered") == 0) {
                g_object_notify (G_OBJECT (client), "default-adapter-powered");
+               if (!client->has_power_state)
+                       g_object_notify (G_OBJECT (client), "default-adapter-state");
+       } else if (g_strcmp0 (property, "power-state") == 0) {
                g_object_notify (G_OBJECT (client), "default-adapter-state");
        }
 }
@@ -639,6 +610,7 @@ default_adapter_changed (GDBusObjectManager       *manager,
                g_list_store_remove_all (client->list_store);
                g_debug ("Disabling discovery on old default adapter");
                _bluetooth_client_set_default_adapter_discovering (client, FALSE);
+               g_clear_object (&client->default_adapter);
        }
 
        client->default_adapter = ADAPTER1 (g_object_ref (G_OBJECT (adapter)));
@@ -739,7 +711,6 @@ adapter_removed (GDBusObjectManager   *manager,
        if (!was_default)
                goto out;
 
-
        new_default_adapter = NULL;
        object_list = g_dbus_object_manager_get_objects (client->manager);
        for (l = object_list; l != NULL; l = l->next) {
@@ -1114,6 +1085,7 @@ static void bluetooth_client_init(BluetoothClient *client)
 {
        client->cancellable = g_cancellable_new ();
        client->list_store = g_list_store_new (BLUETOOTH_TYPE_DEVICE);
+       client->has_power_state = TRUE;
 
        g_dbus_object_manager_client_new_for_bus (G_BUS_TYPE_SYSTEM,
                                                  G_DBUS_OBJECT_MANAGER_CLIENT_FLAGS_DO_NOT_AUTO_START,
@@ -1248,14 +1220,30 @@ _bluetooth_client_set_default_adapter_discovering (BluetoothClient *client,
 static BluetoothAdapterState
 adapter_get_state (BluetoothClient *client)
 {
+       const char *str;
+
        if (!client->default_adapter)
                return BLUETOOTH_ADAPTER_STATE_ABSENT;
-       if (client->power_req == POWER_STATE_REQUEST_NONE)
-               return adapter1_get_powered (client->default_adapter) ?
-                       BLUETOOTH_ADAPTER_STATE_ON : BLUETOOTH_ADAPTER_STATE_OFF;
-       return client->power_req == POWER_STATE_REQUEST_ON ?
-               BLUETOOTH_ADAPTER_STATE_TURNING_ON :
-               BLUETOOTH_ADAPTER_STATE_TURNING_OFF;
+
+       str = adapter1_get_power_state (client->default_adapter);
+       if (str) {
+               if (g_str_equal (str, "on"))
+                       return BLUETOOTH_ADAPTER_STATE_ON;
+               if (g_str_equal (str, "off"))
+                       return BLUETOOTH_ADAPTER_STATE_OFF;
+               if (g_str_equal (str, "turning-on"))
+                       return BLUETOOTH_ADAPTER_STATE_TURNING_ON;
+               if (g_str_equal (str, "turning-off"))
+                       return BLUETOOTH_ADAPTER_STATE_TURNING_OFF;
+               g_warning_once ("Unexpected adapter PowerState value '%s'", str);
+       } else {
+               client->has_power_state = FALSE;
+       }
+
+       /* Fallback if property is missing, or value is unexpected */
+       return adapter1_get_powered (client->default_adapter) ?
+               BLUETOOTH_ADAPTER_STATE_ON : BLUETOOTH_ADAPTER_STATE_OFF;
+
 }
 
 static void
diff --git a/lib/bluetooth-client.xml b/lib/bluetooth-client.xml
index 13248214..57b958ff 100644
--- a/lib/bluetooth-client.xml
+++ b/lib/bluetooth-client.xml
@@ -12,6 +12,7 @@
     <property name="Alias" type="s" access="readwrite"></property>
     <property name="Class" type="u" access="read"></property>
     <property name="Powered" type="b" access="readwrite"></property>
+    <property name="PowerState" type="s" access="read"></property>
     <property name="Discoverable" type="b" access="readwrite"></property>
     <property name="DiscoverableTimeout" type="u" access="readwrite"></property>
     <property name="Pairable" type="b" access="readwrite"></property>
diff --git a/tests/integration-test.py b/tests/integration-test.py
index 83550c3a..808a1bf1 100755
--- a/tests/integration-test.py
+++ b/tests/integration-test.py
@@ -267,7 +267,8 @@ class OopTests(dbusmock.DBusTestCase):
         self.assertEqual(dbusprops_bluez.Get('org.bluez.Adapter1', 'Powered'), False)
         self.assertEqual(self.client.props.default_adapter_state, GnomeBluetoothPriv.AdapterState.OFF)
         self.client.props.default_adapter_powered = True
-        self.assertEqual(self.client.props.default_adapter_state, GnomeBluetoothPriv.AdapterState.TURNING_ON)
+        # NOTE: this should be "turning on"
+        self.assertEqual(self.client.props.default_adapter_state, GnomeBluetoothPriv.AdapterState.OFF)
         self.wait_for_condition(lambda: dbusprops_bluez.Get('org.bluez.Adapter1', 'Powered') == True)
         self.assertEqual(self.client.props.num_adapters, 1)
         self.assertEqual(dbusprops_bluez.Get('org.bluez.Adapter1', 'Powered'), True)
@@ -278,7 +279,8 @@ class OopTests(dbusmock.DBusTestCase):
         self.client.props.default_adapter_powered = False
         self.wait_for_condition(lambda: dbusprops_bluez.Get('org.bluez.Adapter1', 'Powered') == False)
         self.assertEqual(dbusprops_bluez.Get('org.bluez.Adapter1', 'Powered'), False)
-        self.assertEqual(self.client.props.default_adapter_state, 
GnomeBluetoothPriv.AdapterState.TURNING_OFF)
+        # NOTE: this should be "turning off"
+        self.assertEqual(self.client.props.default_adapter_state, GnomeBluetoothPriv.AdapterState.ON)
         self.wait_for_mainloop()
         self.assertEqual(self.client.props.default_adapter_powered, False)
         self.assertEqual(self.client.props.default_adapter_state, GnomeBluetoothPriv.AdapterState.OFF)
@@ -286,7 +288,7 @@ class OopTests(dbusmock.DBusTestCase):
         dbusmock_bluez.UpdateProperties('org.bluez.Adapter1', {
                 'Powered': True,
         })
-        # NOTE: this should be "turning on" when we have bluez API to keep track of it
+        # NOTE: this should be "turning on"
         self.assertEqual(self.client.props.default_adapter_state, GnomeBluetoothPriv.AdapterState.OFF)
         self.wait_for_condition(lambda: dbusprops_bluez.Get('org.bluez.Adapter1', 'Powered') == True)
         self.assertEqual(dbusprops_bluez.Get('org.bluez.Adapter1', 'Powered'), True)