Merge lp:~charlesk/indicator-bluetooth/lp-1530807-watch-bluez-owner-on-bus into lp:indicator-bluetooth/15.10

Proposed by Charles Kerr on 2016-02-14
Status: Merged
Approved by: Charles Kerr on 2016-02-15
Approved revision: 98
Merged at revision: 94
Proposed branch: lp:~charlesk/indicator-bluetooth/lp-1530807-watch-bluez-owner-on-bus
Merge into: lp:indicator-bluetooth/15.10
Diff against target: 142 lines (+56/-18)
1 file modified
src/bluez.vala (+56/-18)
To merge this branch: bzr merge lp:~charlesk/indicator-bluetooth/lp-1530807-watch-bluez-owner-on-bus
Reviewer Review Type Date Requested Status
Xavi Garcia 2016-02-14 Approve on 2016-02-15
PS Jenkins bot (community) continuous-integration Approve on 2016-02-14
Review via email: mp+286003@code.launchpad.net

Commit message

Change the indicator's Bluez class to watch for 'org.bluez' to appear/vanish on the system bus.

Description of the change

Several people experiencing bug #1530807 have stated that restarting indicator-bluetooth solves the issue, so the bug appears to be a timing issue with the indicator's startup.

Change the indicator's Bluez class to watch for 'org.bluez' to appear/vanish on the system bus and update accordingly. The class was previously brittle in that it assumed org.bluez would be up and running before the indicator started.

To post a comment you must log in.
Xavi Garcia (xavi-garcia-mena) wrote :

just one question:
Why does it call init_bluez_state_vars () from the constructor and also from on_bluez_appeared?

do we need the call from on_bluez_appeared?

review: Needs Information
Xavi Garcia (xavi-garcia-mena) wrote :

Approving after discussion with alecu.
If we are not restarting the indicator it makes sense to re-initialize everything

review: Approve
Charles Kerr (charlesk) wrote :

Xavi, the idea was to have the indicator reset its state whenever org.bluez appeared on the bus, not just for the bootstrap case but also to handle bluez being stopped and restarted.

I agree that reinit'ing is probably not necessary in on_bluez_appeared() because the bootstrap case is handled in the ctor and the restart case is handled in on_bluez_vanished().

Since this MP's siloed and tested, though, let's leave this wart in for now and I'll make sure to clean it up when I come back and add bluez5 mock tests after the emergency landing.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/bluez.vala'
2--- src/bluez.vala 2015-09-28 04:06:29 +0000
3+++ src/bluez.vala 2016-02-14 21:10:44 +0000
4@@ -20,12 +20,14 @@
5
6
7 /**
8- * Bluetooth implementaion which uses org.bluez on DBus
9+ * Bluetooth implementaion which uses org.bluez on DBus
10 */
11 public class Bluez: Bluetooth, Object
12 {
13+ uint name_watch_id = 0;
14 uint next_device_id = 1;
15 ObjectManager manager;
16+ static const string BLUEZ_BUSNAME = "org.bluez";
17
18 private bool _powered = false;
19
20@@ -55,14 +57,7 @@
21
22 public Bluez (KillSwitch? killswitch)
23 {
24- try
25- {
26- bus = Bus.get_sync (BusType.SYSTEM);
27- }
28- catch (Error e)
29- {
30- critical (@"$(e.message)");
31- }
32+ init_bluez_state_vars ();
33
34 if ((killswitch != null) && (killswitch.is_valid()))
35 {
36@@ -71,20 +66,59 @@
37 update_enabled ();
38 }
39
40- reset_manager ();
41- }
42-
43- private void reset_manager ()
44+ name_watch_id = Bus.watch_name(BusType.SYSTEM,
45+ BLUEZ_BUSNAME,
46+ BusNameWatcherFlags.AUTO_START,
47+ on_bluez_appeared,
48+ on_bluez_vanished);
49+ }
50+
51+ ~Bluez()
52+ {
53+ Bus.unwatch_name(name_watch_id);
54+ }
55+
56+ private void on_bluez_appeared (DBusConnection connection, string name, string name_owner)
57+ {
58+ debug(@"$name owned by $name_owner, setting up bluez proxies");
59+
60+ bus = connection;
61+
62+ init_bluez_state_vars();
63+ reset_manager();
64+ }
65+
66+ private void on_bluez_vanished (DBusConnection connection, string name)
67+ {
68+ debug(@"$name vanished from the bus");
69+
70+ reset_bluez();
71+ }
72+
73+ private void init_bluez_state_vars ()
74 {
75 id_to_path = new HashTable<uint,ObjectPath> (direct_hash, direct_equal);
76 id_to_device = new HashTable<uint,Device> (direct_hash, direct_equal);
77 path_to_id = new HashTable<ObjectPath,uint> (str_hash, str_equal);
78 path_to_adapter_proxy = new HashTable<ObjectPath,BluezAdapter> (str_hash, str_equal);
79 path_to_device_proxy = new HashTable<ObjectPath,BluezDevice> (str_hash, str_equal);
80-
81+ }
82+
83+ private void reset_bluez ()
84+ {
85+ init_bluez_state_vars ();
86+
87+ devices_changed ();
88+ update_combined_adapter_state ();
89+ update_connected ();
90+ update_enabled ();
91+ }
92+
93+ private void reset_manager()
94+ {
95 try
96 {
97- manager = bus.get_proxy_sync ("org.bluez", "/");
98+ manager = bus.get_proxy_sync (BLUEZ_BUSNAME, "/");
99
100 // Find the adapters and watch for changes
101 manager.interfaces_added.connect ((object_path, interfaces_and_properties) => {
102@@ -134,12 +168,14 @@
103
104 private void update_adapter (ObjectPath object_path)
105 {
106+ debug(@"bluez5 calling update_adapter for $object_path");
107+
108 // Create a proxy if we don't have one
109 var adapter_proxy = path_to_adapter_proxy.lookup (object_path);
110 if (adapter_proxy == null)
111 {
112 try {
113- adapter_proxy = bus.get_proxy_sync ("org.bluez", object_path);
114+ adapter_proxy = bus.get_proxy_sync (BLUEZ_BUSNAME, object_path);
115 } catch (Error e) {
116 critical (@"$(e.message)");
117 return;
118@@ -232,12 +268,14 @@
119 */
120 private void update_device (ObjectPath object_path)
121 {
122+ debug(@"bluez5 calling update_device for $object_path");
123+
124 // Create a proxy if we don't have one
125 var device_proxy = path_to_device_proxy.lookup (object_path);
126 if (device_proxy == null)
127 {
128 try {
129- device_proxy = bus.get_proxy_sync ("org.bluez", object_path);
130+ device_proxy = bus.get_proxy_sync (BLUEZ_BUSNAME, object_path);
131 } catch (Error e) {
132 critical (@"$(e.message)");
133 return;
134@@ -262,7 +300,7 @@
135 var v = device_proxy.get_cached_property ("Class");
136 if (v == null)
137 type = Device.Type.OTHER;
138- else
139+ else
140 type = Device.class_to_device_type (v.get_uint32());
141
142 // look up the device's human-readable name

Subscribers

People subscribed via source and target branches