Merge lp:~charlesk/indicator-bluetooth/lp-1236249 into lp:indicator-bluetooth/13.10

[Charles Kerr]

When the user chooses to toggle bluetooth on or off, if /dev/rfkill isn't available, fall back to toggling org.bluez.Adapter's Powered property.

* Use org.bluez.Adapter's Powered property to toggle bluetooth on and off. Skip /dev/rfkill entirely for 13.10, there are issues there that can't be resolved in time for 13.10.

* Leave the killswitch /dev/rfkill code in the codebase, though disabled for now, because it's likely to return post-13.10

* Do a better job of monitoring the bluez Manager and Adapter for changes s.t. the indicator can update its own state appropriately.

See also https://bugs.launchpad.net/ubuntu/+source/unity8/+bug/1236249/comments/8

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:73
http://jenkins.qa.ubuntu.com/job/indicator-bluetooth-ci/21/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/indicator-bluetooth-saucy-amd64-ci/20
    SUCCESS: http://jenkins.qa.ubuntu.com/job/indicator-bluetooth-saucy-armhf-ci/20
        deb: http://jenkins.qa.ubuntu.com/job/indicator-bluetooth-saucy-armhf-ci/20/artifact/work/output/*zip*/output.zip

Click here to trigger a rebuild:
http://10.97.0.26:8080/job/indicator-bluetooth-ci/21/rebuild

review: Approve (continuous-integration)

[Ted Gould (ted)]

Don't see anything objectionable (a lot of code though). Works for me on my Galaxy Nexus.

review: Approve

=== modified file 'src/bluetooth.vala'
--- src/bluetooth.vala	2013-10-10 19:58:46 +0000
+++ src/bluetooth.vala	2013-10-14 23:34:48 +0000
@@ -27,22 +27,19 @@
      This work as a proxy for "does this hardware support bluetooth?" */
   public abstract bool supported { get; protected set; }
 
-  /* True if there are any bluetooth adapters powered up on the system.
-     In short, whether or not this system's bluetooth is "on". */
-  public abstract bool powered { get; protected set; }
+  /* True if bluetooth's enabled on this system.
+     Bluetooth can be soft-blocked by software and hard-blocked physically,
+     eg by a laptop's network killswitch */
+  public abstract bool enabled { get; protected set; }
+
+  /* Try to enable/disable bluetooth. This can fail if it's overridden
+     by the system, eg by a laptop's network killswitch */
+  public abstract void try_set_enabled (bool b);
 
   /* True if our system can be seen by other bluetooth devices */
   public abstract bool discoverable { get; protected set; }
   public abstract void try_set_discoverable (bool discoverable);
 
-  /* True if bluetooth's blocked. This can be soft-blocked by software and
-   * hard-blocked physically, eg by a laptop's network killswitch */
-  public abstract bool blocked { get; protected set; }
-
-  /* Try to block/unblock bluetooth. This can fail if it's overridden
-     by the system, eg by a laptop's network killswitch */
-  public abstract void try_set_blocked (bool b);
-
   /* Get a list of the Device structs that we know about */
   public abstract List<unowned Device> get_devices ();
 
@@ -53,33 +50,3 @@
      The device_key argument comes from the Device struct */
   public abstract void set_device_connected (uint device_key, bool connected);
 }
-
-
-
-/**
- * Base class for Bluetooth objects that use a killswitch to implement
- * the 'discoverable' property.
- */
-public abstract class KillswitchBluetooth: Object, Bluetooth
-{
-  private KillSwitch killswitch;
-
-  public KillswitchBluetooth (KillSwitch killswitch)
-  {
-    // always sync our 'blocked' property with the one in killswitch
-    this.killswitch = killswitch;
-    blocked = killswitch.blocked;
-    killswitch.notify["blocked"].connect (() => blocked = killswitch.blocked );
-  }
-
-  public bool supported { get; protected set; default = false; }
-  public bool powered { get; protected set; default = false; }
-  public bool discoverable { get; protected set; default = false; }
-  public bool blocked { get; protected set; default = true; }
-  public void try_set_blocked (bool b) { killswitch.try_set_blocked (b); }
-
-  // empty implementations
-  public abstract void try_set_discoverable (bool b);
-  public abstract List<unowned Device> get_devices ();
-  public abstract void set_device_connected (uint device_key, bool connected);
-}
 
=== modified file 'src/bluez.vala'
--- src/bluez.vala	2013-10-10 19:58:46 +0000
+++ src/bluez.vala	2013-10-14 23:34:48 +0000
@@ -22,12 +22,25 @@
 /**
  * Bluetooth implementaion which uses org.bluez on DBus 
  */
-public class Bluez: KillswitchBluetooth
+public class Bluez: Bluetooth, Object
 {
   uint next_device_id = 1;
   org.bluez.Manager manager;
   org.bluez.Adapter default_adapter;
 
+  private bool _powered = false;
+
+  private bool powered {
+    get { return _powered; }
+    set { _powered = value; update_enabled(); }
+  }
+
+  private KillSwitch killswitch = null;
+
+  private string adapter_path = null;
+
+  private DBusConnection bus = null;
+
   /* maps an org.bluez.Device's object_path to the org.bluez.Device proxy */
   HashTable<string,org.bluez.Device> path_to_proxy;
 
@@ -40,44 +53,87 @@
   /* maps our arbitrary unique id to a Bluetooth.Device struct for public consumption */
   HashTable<uint,Device> id_to_device;
 
-  public Bluez (KillSwitch killswitch)
+  public Bluez (KillSwitch? killswitch)
   {
-    base (killswitch);
+    try
+      {
+        bus = Bus.get_sync (BusType.SYSTEM);
+      }
+    catch (Error e)
+      {
+        critical (@"$(e.message)");
+      }
 
-    string adapter_path = null;
+    if ((killswitch != null) && (killswitch.is_valid()))
+      {
+        this.killswitch = killswitch;
+        killswitch.notify["blocked"].connect (() => update_enabled());
+        update_enabled ();
+      }
 
     id_to_path    = new HashTable<uint,string> (direct_hash, direct_equal);
     id_to_device  = new HashTable<uint,Device> (direct_hash, direct_equal);
     path_to_id    = new HashTable<string,uint> (str_hash, str_equal);
     path_to_proxy = new HashTable<string,org.bluez.Device> (str_hash, str_equal);
 
+    reset_manager ();
+  }
+
+  private void reset_manager ()
+  {
+    string new_adapter_path = null;
     try
       {
-        manager = Bus.get_proxy_sync (BusType.SYSTEM, "org.bluez", "/");
+        manager = bus.get_proxy_sync ("org.bluez", "/");
 
-        // get the current default adapter & watch for future default adapters
-        adapter_path = manager.default_adapter ();
+        // if the default adapter changes, update our connections
         manager.default_adapter_changed.connect ((object_path)
             => on_default_adapter_changed (object_path));
+
+        // if the current adapter disappears, call clear_adapter()
+        manager.adapter_removed.connect ((object_path) => { 
+            if (object_path == adapter_path)
+              clear_adapter ();
+        });
+
+        // get the current default adapter & watch for future default adapters
+        new_adapter_path = manager.default_adapter ();
       }
     catch (Error e)
       {
         critical (@"$(e.message)");
       }
 
-    on_default_adapter_changed (adapter_path);
-  }
-
-  private void on_default_adapter_changed (string? object_path)
-  {
-    supported = object_path != null;
+    on_default_adapter_changed (new_adapter_path);
+  }
+
+  private void clear_adapter ()
+  {
+    if (adapter_path != null)
+      debug (@"clearing adapter; was using $adapter_path");
+
+    path_to_proxy.remove_all ();
+    path_to_id.remove_all ();
+    id_to_path.remove_all ();
+    id_to_device.remove_all ();
+
+    default_adapter = null;
+    adapter_path = null;
+
+    discoverable = false;
+    powered = false;
+  }
+
+  void on_default_adapter_changed (string? object_path)
+  {
+    clear_adapter ();
 
     if (object_path != null) try
       {
-        debug (@"using default adapter at $object_path");
+        adapter_path = object_path;
         default_adapter = Bus.get_proxy_sync (BusType.SYSTEM,
                                               "org.bluez",
-                                              object_path);
+                                              adapter_path);
 
         default_adapter.property_changed.connect (()
             => on_default_adapter_properties_changed ());
@@ -104,6 +160,8 @@
     on_default_adapter_properties_changed ();
   }
 
+  /* When the default adapter's properties change,
+     update our own properties "powered" and "discoverable" */
   private void on_default_adapter_properties_changed ()
   {
     bool is_discoverable = false;
@@ -217,7 +275,6 @@
   private void device_connect_on_interface (DBusProxy proxy,
                                             string interface_name)
   {
-    var bus = proxy.get_connection ();
     var object_path = proxy.get_object_path ();
 
     debug (@"trying to connect to $object_path: $(interface_name)");
@@ -359,11 +416,21 @@
     devices_changed ();
   }
 
+  /* update the 'enabled' property by looking at the killswitch state
+     and the 'powered' property state */
+  void update_enabled ()
+  {
+    var blocked = (killswitch != null) && killswitch.blocked;
+    debug (@"in upate_enabled, powered is $powered, blocked is $blocked");
+    enabled = powered && !blocked;
+  }
+    
+
   ////
   ////  Public API
   ////
 
-  public override void set_device_connected (uint id, bool connected)
+  public void set_device_connected (uint id, bool connected)
   {
     var device = id_to_device.lookup (id);
     var path = id_to_path.lookup (id);
@@ -380,20 +447,35 @@
       }
   }
 
-  public override void try_set_discoverable (bool b)
+  public void try_set_discoverable (bool b)
   {
-    if (discoverable != b) try
-      {
-        default_adapter.set_property ("Discoverable", new Variant.boolean (b));
-      }
-    catch (Error e)
-      {
-        critical (@"$(e.message)");
+    if (discoverable != b)
+      {
+        default_adapter.set_property.begin ("Discoverable", new Variant.boolean (b));
       }
   }
 
-  public override List<unowned Device> get_devices ()
+  public List<unowned Device> get_devices ()
   {
     return id_to_device.get_values();
   }
+
+  public bool supported { get; protected set; default = false; }
+  public bool discoverable { get; protected set; default = false; }
+  public bool enabled { get; protected set; default = false; }
+
+  public void try_set_enabled (bool b)
+  {
+    if (killswitch != null)
+      {
+        debug (@"setting killswitch blocked to $(!b)");
+        killswitch.try_set_blocked (!b);
+      }
+    else if (default_adapter != null)
+      {
+        debug (@"setting bluez Adapter's Powered property to $b");
+        default_adapter.set_property.begin ("Powered", new Variant.boolean (b));
+        powered = b;
+      }
+  }
 }
 
=== modified file 'src/desktop.vala'
--- src/desktop.vala	2013-10-10 19:58:46 +0000
+++ src/desktop.vala	2013-10-14 23:34:48 +0000
@@ -66,7 +66,7 @@
 
     // know when to show the indicator & when to hide it
     settings.changed["visible"].connect (()=> update_visibility());
-    bluetooth.notify.connect (() => update_visibility());
+    bluetooth.notify["enabled"].connect (() => update_visibility());
     update_visibility ();
 
     // when devices change, rebuild our device section
@@ -82,7 +82,7 @@
 
   void update_visibility ()
   {
-    visible = bluetooth.powered && !bluetooth.blocked && settings.get_boolean("visible");
+    visible = bluetooth.enabled && settings.get_boolean("visible");
   }
 
   ///
@@ -229,10 +229,6 @@
     bluetooth.notify["discoverable"].connect (()
         => action.set_state (bluetooth.discoverable));
 
-    action.set_enabled (bluetooth.powered);
-    bluetooth.notify["powered"].connect (()
-        => action.set_enabled (bluetooth.powered));
-
     return action;
   }
 
 
=== modified file 'src/killswitch.vala'
--- src/killswitch.vala	2013-08-06 20:33:26 +0000
+++ src/killswitch.vala	2013-10-14 23:34:48 +0000
@@ -26,6 +26,8 @@
  */
 public interface KillSwitch: Object
 {
+  public abstract bool is_valid ();
+
   public abstract bool blocked { get; protected set; }
 
   /* Try to block/unblock bluetooth.
@@ -109,6 +111,11 @@
       }
   }
 
+  public bool is_valid ()
+  {
+    return fd != -1;
+  }
+
   private bool on_channel_event (IOChannel source, IOCondition condition)
   {
     read_event ();
 
=== modified file 'src/main.vala'
--- src/main.vala	2013-08-06 00:43:18 +0000
+++ src/main.vala	2013-10-14 23:34:48 +0000
@@ -26,7 +26,7 @@
   Intl.textdomain (Config.GETTEXT_PACKAGE);
 
   // create the backend
-  var bluetooth = new Bluez (new RfKillSwitch ());
+  var bluetooth = new Bluez (null);
  
   // start the service
   var service = new Service (bluetooth);
 
=== modified file 'src/org-bluez.vala'
--- src/org-bluez.vala	2013-08-01 23:21:05 +0000
+++ src/org-bluez.vala	2013-10-14 23:34:48 +0000
@@ -69,7 +69,7 @@
 			public abstract GLib.HashTable<string, GLib.Variant> get_properties() throws DBusError, IOError;
 
 			[DBus (name = "SetProperty")]
-			public abstract void set_property(string name, GLib.Variant value) throws DBusError, IOError;
+			public abstract async void set_property(string name, GLib.Variant value) throws DBusError, IOError;
 
 			[DBus (name = "RequestSession")]
 			public abstract void request_session() throws DBusError, IOError;
 
=== modified file 'src/phone.vala'
--- src/phone.vala	2013-10-10 19:58:46 +0000
+++ src/phone.vala	2013-10-14 23:34:48 +0000
@@ -53,7 +53,7 @@
 
   void update_visibility ()
   {
-    visible = bluetooth.powered && !bluetooth.blocked;
+    visible = bluetooth.enabled;
   }
 
   ///
 
=== modified file 'src/profile.vala'
--- src/profile.vala	2013-10-10 19:58:46 +0000
+++ src/profile.vala	2013-10-14 23:34:48 +0000
@@ -123,16 +123,16 @@
   {
     var action = new SimpleAction.stateful ("bluetooth-enabled",
                                             null,
-                                            !bluetooth.blocked);
+                                            bluetooth.enabled);
 
     action.activate.connect (()
         => action.change_state (!action.get_state().get_boolean()));
 
     action.change_state.connect ((action, requestedValue)
-        => bluetooth.try_set_blocked (!requestedValue.get_boolean()));
+        => bluetooth.try_set_enabled (requestedValue.get_boolean()));
 
-    bluetooth.notify["blocked"].connect (()
-        => action.set_state (!bluetooth.blocked));
+    bluetooth.notify["enabled"].connect (()
+        => action.set_state (bluetooth.enabled));
 
     return action;
   }
@@ -144,12 +144,9 @@
 
   protected Variant action_state_for_root ()
   {
-    var blocked = bluetooth.blocked;
-    var powered = bluetooth.powered;
-
     string a11y;
     string icon_name;
-    if (powered && !blocked)
+    if (bluetooth.enabled)
       {
         a11y = "Bluetooth (on)";
         icon_name = "bluetooth-active";