Merge lp:~yuningdodo/unity-control-center/unity-control-center.lp1248720-block-power-callback-unless-its-triggered-by-user into lp:unity-control-center

Proposed by Yu Ning on 2015-04-02
Status: Merged
Approved by: Sebastien Bacher on 2015-04-09
Approved revision: 12812
Merged at revision: 12815
Proposed branch: lp:~yuningdodo/unity-control-center/unity-control-center.lp1248720-block-power-callback-unless-its-triggered-by-user
Merge into: lp:unity-control-center
Diff against target: 72 lines (+19/-0)
1 file modified
panels/bluetooth/cc-bluetooth-panel.c (+19/-0)
To merge this branch: bzr merge lp:~yuningdodo/unity-control-center/unity-control-center.lp1248720-block-power-callback-unless-its-triggered-by-user
Reviewer Review Type Date Requested Status
Lars Karlitski (community) Approve on 2015-04-09
Sebastien Bacher 2015-04-02 Approve on 2015-04-09
Mathieu Trudel-Lapierre Abstain on 2015-04-08
Review via email: mp+255041@code.launchpad.net

Commit message

Block power callback unless it's triggered by the user.

Description of the change

Block power callback unless it's triggered by the user.

To post a comment you must log in.
Sebastien Bacher (seb128) wrote :

Thank you for your work, could you give some context/explain why you are doing that? Is that resolving an user visible issue?

review: Needs Information
Yu Ning (yuningdodo) wrote :

Oh sorry, forgot to mention that this patch is for bug #1248720.

Sebastien Bacher (seb128) wrote :

do you know if that's an issue impacting upstream as well and if it could be sent to GNOME for gnome-control-center?

Mathieu Trudel-Lapierre (cyphermox) wrote :

Looks fine to me. Avoiding to represent the bluetooth powered changes initiated by the app is logical. That said, I am not convinced it will indeed fix the issue completely, but the code changes seem correct.

Please address seb's concerns with upstreaming the code though.

review: Abstain
Yu Ning (yuningdodo) wrote :

Thanks for the review, let me install a gnome desktop and have a try. Will update later.

Yu Ning (yuningdodo) wrote :

I installed gnome desktop on ubuntu trusty, and can also reproduce the issue in gnome-control-center.

The package version is:
gnome-control-center 1:3.6.3-0ubuntu56.1

$ gnome-control-center --version
gnome-control-center 3.6.3

Sebastien Bacher (seb128) wrote :

@Yu, thanks, any chance you could try on something newer like vivid? 3.6 is really outdated and doesn't reflect current upstream state

Yu Ning (yuningdodo) wrote :

@Sebastien, more details are tested on bug #1248720.

Sebastien Bacher (seb128) wrote :

thanks, it's not the most elegant but it should work ;-)

review: Approve
Lars Karlitski (larsu) wrote :

Blocking signals is quite ugly. GtkSwitch differentiates between "state" and "active" for cases like this. I've fixed this in my bluez5 branch[1].

It'll work though. I'm fine with adding it for V and getting the proper fix next cycle. Thanks for working on this!

[1] http://bazaar.launchpad.net/~larsu/unity-control-center/bluez/revision/12812

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'panels/bluetooth/cc-bluetooth-panel.c'
2--- panels/bluetooth/cc-bluetooth-panel.c 2014-02-20 22:57:03 +0000
3+++ panels/bluetooth/cc-bluetooth-panel.c 2015-04-02 05:06:38 +0000
4@@ -58,6 +58,8 @@
5 GHashTable *connecting_devices;
6 GCancellable *cancellable;
7 GSettings *indicator_settings;
8+
9+ gulong power_callback_handler_id;
10 };
11
12 static void cc_bluetooth_panel_finalize (GObject *object);
13@@ -420,8 +422,12 @@
14
15 state = gtk_switch_get_active (GTK_SWITCH (WID ("switch_bluetooth")));
16 g_debug ("Power switched to %s", state ? "on" : "off");
17+ g_signal_handler_block (G_OBJECT (WID ("switch_bluetooth")),
18+ self->priv->power_callback_handler_id);
19 bluetooth_killswitch_set_state (self->priv->killswitch,
20 state ? BLUETOOTH_KILLSWITCH_STATE_UNBLOCKED : BLUETOOTH_KILLSWITCH_STATE_SOFT_BLOCKED);
21+ g_signal_handler_unblock (G_OBJECT (WID ("switch_bluetooth")),
22+ self->priv->power_callback_handler_id);
23 }
24
25 static void
26@@ -776,7 +782,11 @@
27 CcBluetoothPanel *self)
28 {
29 g_debug ("Default adapter power changed");
30+ g_signal_handler_block (G_OBJECT (WID ("switch_bluetooth")),
31+ self->priv->power_callback_handler_id);
32 cc_bluetooth_panel_update_powered_state (self);
33+ g_signal_handler_unblock (G_OBJECT (WID ("switch_bluetooth")),
34+ self->priv->power_callback_handler_id);
35 }
36
37 static void
38@@ -785,9 +795,13 @@
39 CcBluetoothPanel *self)
40 {
41 g_debug ("Default adapter changed");
42+ g_signal_handler_block (G_OBJECT (WID ("switch_bluetooth")),
43+ self->priv->power_callback_handler_id);
44 cc_bluetooth_panel_update_state (self);
45 cc_bluetooth_panel_update_power (self);
46 cc_bluetooth_panel_update_powered_state (self);
47+ g_signal_handler_unblock (G_OBJECT (WID ("switch_bluetooth")),
48+ self->priv->power_callback_handler_id);
49 }
50
51 static void
52@@ -796,8 +810,12 @@
53 CcBluetoothPanel *self)
54 {
55 g_debug ("Killswitch changed to state '%s' (%d)", bluetooth_killswitch_state_to_string (state) , state);
56+ g_signal_handler_block (G_OBJECT (WID ("switch_bluetooth")),
57+ self->priv->power_callback_handler_id);
58 cc_bluetooth_panel_update_state (self);
59 cc_bluetooth_panel_update_power (self);
60+ g_signal_handler_unblock (G_OBJECT (WID ("switch_bluetooth")),
61+ self->priv->power_callback_handler_id);
62 }
63
64 static void
65@@ -904,6 +922,7 @@
66 G_CALLBACK (switch_connected_active_changed), self);
67
68 /* Set the initial state of power */
69+ self->priv->power_callback_handler_id =
70 g_signal_connect (G_OBJECT (WID ("switch_bluetooth")), "notify::active",
71 G_CALLBACK (power_callback), self);
72 g_signal_connect (G_OBJECT (self->priv->killswitch), "state-changed",

Subscribers

People subscribed via source and target branches