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
Status: Merged
Approved by: Sebastien Bacher
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
Sebastien Bacher Approve
Mathieu Trudel-Lapierre Abstain
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.
Revision history for this message
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
Revision history for this message
Yu Ning (yuningdodo) wrote :

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

Revision history for this message
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?

Revision history for this message
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
Revision history for this message
Yu Ning (yuningdodo) wrote :

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

Revision history for this message
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

Revision history for this message
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

Revision history for this message
Yu Ning (yuningdodo) wrote :

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

Revision history for this message
Sebastien Bacher (seb128) wrote :

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

review: Approve
Revision history for this message
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
=== modified file 'panels/bluetooth/cc-bluetooth-panel.c'
--- panels/bluetooth/cc-bluetooth-panel.c 2014-02-20 22:57:03 +0000
+++ panels/bluetooth/cc-bluetooth-panel.c 2015-04-02 05:06:38 +0000
@@ -58,6 +58,8 @@
58 GHashTable *connecting_devices;58 GHashTable *connecting_devices;
59 GCancellable *cancellable;59 GCancellable *cancellable;
60 GSettings *indicator_settings;60 GSettings *indicator_settings;
61
62 gulong power_callback_handler_id;
61};63};
6264
63static void cc_bluetooth_panel_finalize (GObject *object);65static void cc_bluetooth_panel_finalize (GObject *object);
@@ -420,8 +422,12 @@
420422
421 state = gtk_switch_get_active (GTK_SWITCH (WID ("switch_bluetooth")));423 state = gtk_switch_get_active (GTK_SWITCH (WID ("switch_bluetooth")));
422 g_debug ("Power switched to %s", state ? "on" : "off");424 g_debug ("Power switched to %s", state ? "on" : "off");
425 g_signal_handler_block (G_OBJECT (WID ("switch_bluetooth")),
426 self->priv->power_callback_handler_id);
423 bluetooth_killswitch_set_state (self->priv->killswitch,427 bluetooth_killswitch_set_state (self->priv->killswitch,
424 state ? BLUETOOTH_KILLSWITCH_STATE_UNBLOCKED : BLUETOOTH_KILLSWITCH_STATE_SOFT_BLOCKED);428 state ? BLUETOOTH_KILLSWITCH_STATE_UNBLOCKED : BLUETOOTH_KILLSWITCH_STATE_SOFT_BLOCKED);
429 g_signal_handler_unblock (G_OBJECT (WID ("switch_bluetooth")),
430 self->priv->power_callback_handler_id);
425}431}
426432
427static void433static void
@@ -776,7 +782,11 @@
776 CcBluetoothPanel *self)782 CcBluetoothPanel *self)
777{783{
778 g_debug ("Default adapter power changed");784 g_debug ("Default adapter power changed");
785 g_signal_handler_block (G_OBJECT (WID ("switch_bluetooth")),
786 self->priv->power_callback_handler_id);
779 cc_bluetooth_panel_update_powered_state (self);787 cc_bluetooth_panel_update_powered_state (self);
788 g_signal_handler_unblock (G_OBJECT (WID ("switch_bluetooth")),
789 self->priv->power_callback_handler_id);
780}790}
781791
782static void792static void
@@ -785,9 +795,13 @@
785 CcBluetoothPanel *self)795 CcBluetoothPanel *self)
786{796{
787 g_debug ("Default adapter changed");797 g_debug ("Default adapter changed");
798 g_signal_handler_block (G_OBJECT (WID ("switch_bluetooth")),
799 self->priv->power_callback_handler_id);
788 cc_bluetooth_panel_update_state (self);800 cc_bluetooth_panel_update_state (self);
789 cc_bluetooth_panel_update_power (self);801 cc_bluetooth_panel_update_power (self);
790 cc_bluetooth_panel_update_powered_state (self);802 cc_bluetooth_panel_update_powered_state (self);
803 g_signal_handler_unblock (G_OBJECT (WID ("switch_bluetooth")),
804 self->priv->power_callback_handler_id);
791}805}
792806
793static void807static void
@@ -796,8 +810,12 @@
796 CcBluetoothPanel *self)810 CcBluetoothPanel *self)
797{811{
798 g_debug ("Killswitch changed to state '%s' (%d)", bluetooth_killswitch_state_to_string (state) , state);812 g_debug ("Killswitch changed to state '%s' (%d)", bluetooth_killswitch_state_to_string (state) , state);
813 g_signal_handler_block (G_OBJECT (WID ("switch_bluetooth")),
814 self->priv->power_callback_handler_id);
799 cc_bluetooth_panel_update_state (self);815 cc_bluetooth_panel_update_state (self);
800 cc_bluetooth_panel_update_power (self);816 cc_bluetooth_panel_update_power (self);
817 g_signal_handler_unblock (G_OBJECT (WID ("switch_bluetooth")),
818 self->priv->power_callback_handler_id);
801}819}
802820
803static void821static void
@@ -904,6 +922,7 @@
904 G_CALLBACK (switch_connected_active_changed), self);922 G_CALLBACK (switch_connected_active_changed), self);
905923
906 /* Set the initial state of power */924 /* Set the initial state of power */
925 self->priv->power_callback_handler_id =
907 g_signal_connect (G_OBJECT (WID ("switch_bluetooth")), "notify::active",926 g_signal_connect (G_OBJECT (WID ("switch_bluetooth")), "notify::active",
908 G_CALLBACK (power_callback), self);927 G_CALLBACK (power_callback), self);
909 g_signal_connect (G_OBJECT (self->priv->killswitch), "state-changed",928 g_signal_connect (G_OBJECT (self->priv->killswitch), "state-changed",

Subscribers

People subscribed via source and target branches