indicator-power

Merge lp:~albaguirre/indicator-power/use-new-brightness-dbus-interface into lp:indicator-power/14.10

  1. use-new-brightness-dbus-interface
  2. Merge into trunk.14.10
Proposed by Alberto Aguirre
Status: Merged
Approved by: Lars Karlitski
Approved revision: 252
Merged at revision: 247
Proposed branch: lp:~albaguirre/indicator-power/use-new-brightness-dbus-interface
Merge into: lp:indicator-power/14.10
Diff against target: 334 lines (+71/-50)
6 files modified
debian/control (+2/-0)
m4/gcov.m4 (+1/-1)
src/Makefile.am (+2/-2)
src/ib-brightness-uscreen-control.c (+41/-22)
src/ib-brightness-uscreen-control.h (+11/-11)
src/service.c (+14/-14)
To merge this branch: bzr merge lp:~albaguirre/indicator-power/use-new-brightness-dbus-interface
Reviewer Review Type Date Requested Status
Lars Karlitski (community) Approve
PS Jenkins bot (community) continuous-integration Approve
Iain Lane Approve
Review via email: mp+223339@code.launchpad.net

Commit message

Changes to address setBrightness interface moving from powerd to unity-system-compositor

To post a comment you must log in.
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:247
http://jenkins.qa.ubuntu.com/job/indicator-power-ci/105/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/indicator-power-utopic-amd64-ci/6/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/indicator-power-utopic-armhf-ci/6/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/indicator-power-ci/105/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
Iain Lane (laney) wrote :

Cheers. Since you asked me to review I've taken a look, but Charles I think usually takes care of i-power. One inline comment too.

The main problem is that you need to rename the 'powerd' references everywhere since that name is now inaccurate except for getBrightnessParams.

What provides this service? Please either add a dependency or (maybe this is a good idea anyway) keep the fallback to powerd if nothing has the new name.

review: Needs Fixing
Revision history for this message
Charles Kerr (charlesk) wrote :

alberto, laney, IMO we should remove the brightness code from indicator-power altogether. IMO it doesn't make sense for i-power to maintain a feature that it's not using.

Revision history for this message
Charles Kerr (charlesk) wrote :

In the 50 minutes since my last comment, it looks like Design has decided to restore the slider to indicator-power after all -- this MR brought the discussion to a head. :)

After this MR goes through as an interim step, I'll revert the previous code that disabled the slider.

lp:~albaguirre/indicator-power/use-new-brightness-dbus-interface updated
248. By Alberto Aguirre

Remove references to powerd since the brightness interface is now provided by Unity.Screen

249. By Alberto Aguirre

Remove powerd reference in message

250. By Alberto Aguirre

Add powerd and unity-system-compositor to Depends

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:248
http://jenkins.qa.ubuntu.com/job/indicator-power-ci/106/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/indicator-power-utopic-amd64-ci/7/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/indicator-power-utopic-armhf-ci/7/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/indicator-power-ci/106/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:250
http://jenkins.qa.ubuntu.com/job/indicator-power-ci/107/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/indicator-power-utopic-amd64-ci/8/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/indicator-power-utopic-armhf-ci/8/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/indicator-power-ci/107/rebuild

review: Needs Fixing (continuous-integration)
lp:~albaguirre/indicator-power/use-new-brightness-dbus-interface updated
251. By Alberto Aguirre

Add 1.11 as allowed version for lcov

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:251
http://jenkins.qa.ubuntu.com/job/indicator-power-ci/108/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/indicator-power-utopic-amd64-ci/9
    SUCCESS: http://jenkins.qa.ubuntu.com/job/indicator-power-utopic-armhf-ci/9
        deb: http://jenkins.qa.ubuntu.com/job/indicator-power-utopic-armhf-ci/9/artifact/work/output/*zip*/output.zip

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/indicator-power-ci/108/rebuild

review: Approve (continuous-integration)
Revision history for this message
Iain Lane (laney) wrote :

Cheers. It looks okay to me now (haven't tested though).

Just one thing. Could you version the u-s-c dependency to (>= the-one-which-introduced-this-interface)?

review: Approve
lp:~albaguirre/indicator-power/use-new-brightness-dbus-interface updated
252. By Alberto Aguirre

Specify required unity-system-compositor version

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:252
http://jenkins.qa.ubuntu.com/job/indicator-power-ci/109/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/indicator-power-utopic-amd64-ci/10
    SUCCESS: http://jenkins.qa.ubuntu.com/job/indicator-power-utopic-armhf-ci/10
        deb: http://jenkins.qa.ubuntu.com/job/indicator-power-utopic-armhf-ci/10/artifact/work/output/*zip*/output.zip

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/indicator-power-ci/109/rebuild

review: Approve (continuous-integration)
Revision history for this message
Lars Karlitski (larsu) wrote :

I would have preferred to rename the functions in a backend-agnostic way, but I won't block on that.

Thanks for the patch!

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'debian/control'
2--- debian/control 2014-04-15 13:16:44 +0000
3+++ debian/control 2014-06-20 19:57:33 +0000
4@@ -24,6 +24,8 @@
5 Depends: ${shlibs:Depends},
6 ${misc:Depends},
7 upower,
8+ powerd,
9+ unity-system-compositor (>= 0.0.4),
10 Recommends: unity-control-center | gnome-control-center (>= 3.1) | ubuntu-system-settings | xfce4-power-manager,
11 indicator-applet (>= 0.2) | indicator-renderer,
12 Description: Indicator showing power state.
13
14=== modified file 'm4/gcov.m4'
15--- m4/gcov.m4 2013-12-03 14:00:14 +0000
16+++ m4/gcov.m4 2014-06-20 19:57:33 +0000
17@@ -30,7 +30,7 @@
18 AC_MSG_ERROR([ccache must be disabled when --enable-gcov option is used. You can disable ccache by setting environment variable CCACHE_DISABLE=1.])
19 fi
20
21- lcov_version_list="1.6 1.7 1.8 1.9 1.10"
22+ lcov_version_list="1.6 1.7 1.8 1.9 1.10 1.11"
23 AC_CHECK_PROG(LCOV, lcov, lcov)
24 AC_CHECK_PROG(GENHTML, genhtml, genhtml)
25
26
27=== modified file 'src/Makefile.am'
28--- src/Makefile.am 2014-03-03 09:18:05 +0000
29+++ src/Makefile.am 2014-06-20 19:57:33 +0000
30@@ -46,8 +46,8 @@
31 libindicatorpower_service_a_SOURCES = \
32 ib-brightness-control.c \
33 ib-brightness-control.h \
34- ib-brightness-powerd-control.c \
35- ib-brightness-powerd-control.h \
36+ ib-brightness-uscreen-control.c \
37+ ib-brightness-uscreen-control.h \
38 device-provider.c \
39 device-provider.h \
40 device.c \
41
42=== renamed file 'src/ib-brightness-powerd-control.c' => 'src/ib-brightness-uscreen-control.c'
43--- src/ib-brightness-powerd-control.c 2014-04-28 23:15:18 +0000
44+++ src/ib-brightness-uscreen-control.c 2014-06-20 19:57:33 +0000
45@@ -17,19 +17,20 @@
46 * Yuan-Chen Cheng <yc.cheng@canonical.com>
47 */
48
49-#include "ib-brightness-powerd-control.h"
50+#include "ib-brightness-uscreen-control.h"
51
52 static gboolean getBrightnessParams(GDBusProxy* powerd_proxy, int *dim, int *min,
53 int *max, int *dflt, gboolean *ab_supported);
54
55 GDBusProxy*
56-powerd_get_proxy(brightness_params_t *params)
57+uscreen_get_proxy(brightness_params_t *params)
58 {
59 GError *error = NULL;
60 gboolean ret;
61
62 g_return_val_if_fail (params != NULL, NULL);
63
64+ /* For now we still need to obtain the brigthness params from powerd */
65 GDBusProxy* powerd_proxy = g_dbus_proxy_new_for_bus_sync(G_BUS_TYPE_SYSTEM,
66 G_DBUS_PROXY_FLAGS_NONE,
67 NULL,
68@@ -55,8 +56,26 @@
69 g_object_unref (powerd_proxy);
70 return NULL;
71 }
72-
73- return powerd_proxy;
74+
75+ g_clear_object (&powerd_proxy);
76+
77+ GDBusProxy* uscreen_proxy = g_dbus_proxy_new_for_bus_sync(G_BUS_TYPE_SYSTEM,
78+ G_DBUS_PROXY_FLAGS_NONE,
79+ NULL,
80+ "com.canonical.Unity.Screen",
81+ "/com/canonical/Unity/Screen",
82+ "com.canonical.Unity.Screen",
83+ NULL,
84+ &error);
85+
86+ if (error != NULL)
87+ {
88+ g_debug ("could not connect to unity screen: %s", error->message);
89+ g_error_free (error);
90+ return NULL;
91+ }
92+
93+ return uscreen_proxy;
94 }
95
96
97@@ -89,18 +108,18 @@
98 return TRUE;
99 }
100
101-static gboolean setUserBrightness(GDBusProxy* powerd_proxy, GCancellable *gcancel, int brightness)
102+static gboolean setUserBrightness(GDBusProxy* uscreen_proxy, GCancellable *gcancel, int brightness)
103 {
104 GVariant *ret = NULL;
105 GError *error = NULL;
106
107- ret = g_dbus_proxy_call_sync(powerd_proxy,
108+ ret = g_dbus_proxy_call_sync(uscreen_proxy,
109 "setUserBrightness",
110 g_variant_new("(i)", brightness),
111 G_DBUS_CALL_FLAGS_NONE,
112 -1, gcancel, &error);
113 if (!ret) {
114- g_warning("setUserBrightness via powerd failed: %s", error->message);
115+ g_warning("setUserBrightness via unity.screen failed: %s", error->message);
116 g_error_free(error);
117 return FALSE;
118 } else {
119@@ -109,9 +128,9 @@
120 }
121 }
122
123-struct _IbBrightnessPowerdControl
124+struct _IbBrightnessUScreenControl
125 {
126- GDBusProxy *powerd_proxy;
127+ GDBusProxy *uscreen_proxy;
128 GCancellable *gcancel;
129
130 int dim;
131@@ -123,13 +142,13 @@
132 int current;
133 };
134
135-IbBrightnessPowerdControl*
136-ib_brightness_powerd_control_new (GDBusProxy* powerd_proxy, brightness_params_t params)
137+IbBrightnessUscreenControl*
138+ib_brightness_uscreen_control_new (GDBusProxy* uscreen_proxy, brightness_params_t params)
139 {
140- IbBrightnessPowerdControl *control;
141+ IbBrightnessUscreenControl *control;
142
143- control = g_new0 (IbBrightnessPowerdControl, 1);
144- control->powerd_proxy = powerd_proxy;
145+ control = g_new0 (IbBrightnessUscreenControl, 1);
146+ control->uscreen_proxy = uscreen_proxy;
147 control->gcancel = g_cancellable_new();
148
149 control->dim = params.dim;
150@@ -139,21 +158,21 @@
151 control->ab_supported = params.ab_supported;
152
153 // XXX: set the brightness value is the only way to sync the brightness value with
154- // powerd, and we should set the user prefered / last set brightness value upon startup.
155+ // unity.screen, and we should set the user prefered / last set brightness value upon startup.
156 // Before we have code to store last set brightness value or other mechanism, we set
157 // it to default brightness that powerd proposed.
158- ib_brightness_powerd_control_set_value(control, control->dflt);
159+ ib_brightness_uscreen_control_set_value(control, control->dflt);
160
161 return control;
162 }
163
164 void
165-ib_brightness_powerd_control_set_value (IbBrightnessPowerdControl* self, gint value)
166+ib_brightness_uscreen_control_set_value (IbBrightnessUscreenControl* self, gint value)
167 {
168 gboolean ret;
169
170 value = CLAMP(value, self->min, self->max);
171- ret = setUserBrightness(self->powerd_proxy, self->gcancel, value);
172+ ret = setUserBrightness(self->uscreen_proxy, self->gcancel, value);
173 if (ret)
174 {
175 self->current = value;
176@@ -161,23 +180,23 @@
177 }
178
179 gint
180-ib_brightness_powerd_control_get_value (IbBrightnessPowerdControl* self)
181+ib_brightness_uscreen_control_get_value (IbBrightnessUscreenControl* self)
182 {
183 return self->current;
184 }
185
186 gint
187-ib_brightness_powerd_control_get_max_value (IbBrightnessPowerdControl* self)
188+ib_brightness_uscreen_control_get_max_value (IbBrightnessUscreenControl* self)
189 {
190 return self->max;
191 }
192
193 void
194-ib_brightness_powerd_control_free (IbBrightnessPowerdControl *self)
195+ib_brightness_uscreen_control_free (IbBrightnessUscreenControl *self)
196 {
197 g_cancellable_cancel (self->gcancel);
198 g_object_unref (self->gcancel);
199- g_object_unref (self->powerd_proxy);
200+ g_object_unref (self->uscreen_proxy);
201 g_free (self);
202 }
203
204
205=== renamed file 'src/ib-brightness-powerd-control.h' => 'src/ib-brightness-uscreen-control.h'
206--- src/ib-brightness-powerd-control.h 2014-04-28 23:15:18 +0000
207+++ src/ib-brightness-uscreen-control.h 2014-06-20 19:57:33 +0000
208@@ -17,8 +17,8 @@
209 * Y.C Cheng <yc.cheng@canonical.com>
210 */
211
212-#ifndef __IB_BRIGHTNESS_POWERD_CONTROL_H__
213-#define __IB_BRIGHTNESS_POWERD_CONTROL_H__
214+#ifndef __IB_BRIGHTNESS_USCREEN_CONTROL_H__
215+#define __IB_BRIGHTNESS_USCREEN_CONTROL_H__
216
217 #include <gio/gio.h>
218
219@@ -30,14 +30,14 @@
220 gboolean ab_supported;
221 } brightness_params_t;
222
223-GDBusProxy* powerd_get_proxy(brightness_params_t *);
224-
225-typedef struct _IbBrightnessPowerdControl IbBrightnessPowerdControl;
226-
227-IbBrightnessPowerdControl* ib_brightness_powerd_control_new (GDBusProxy* powerd_proxy, brightness_params_t params);
228-void ib_brightness_powerd_control_set_value (IbBrightnessPowerdControl* self, gint value);
229-gint ib_brightness_powerd_control_get_value (IbBrightnessPowerdControl* self);
230-gint ib_brightness_powerd_control_get_max_value (IbBrightnessPowerdControl* self);
231-void ib_brightness_powerd_control_free (IbBrightnessPowerdControl *self);
232+GDBusProxy* uscreen_get_proxy(brightness_params_t *);
233+
234+typedef struct _IbBrightnessUScreenControl IbBrightnessUscreenControl;
235+
236+IbBrightnessUscreenControl* ib_brightness_uscreen_control_new (GDBusProxy* uscreen_proxy, brightness_params_t params);
237+void ib_brightness_uscreen_control_set_value (IbBrightnessUscreenControl* self, gint value);
238+gint ib_brightness_uscreen_control_get_value (IbBrightnessUscreenControl* self);
239+gint ib_brightness_uscreen_control_get_max_value (IbBrightnessUscreenControl* self);
240+void ib_brightness_uscreen_control_free (IbBrightnessUscreenControl *self);
241
242 #endif
243
244=== modified file 'src/service.c'
245--- src/service.c 2014-04-15 13:08:08 +0000
246+++ src/service.c 2014-06-20 19:57:33 +0000
247@@ -27,7 +27,7 @@
248 #include "device.h"
249 #include "device-provider.h"
250 #include "ib-brightness-control.h"
251-#include "ib-brightness-powerd-control.h"
252+#include "ib-brightness-uscreen-control.h"
253 #include "service.h"
254
255 #define BUS_NAME "com.canonical.indicator.power"
256@@ -105,7 +105,7 @@
257 GSettings * settings;
258
259 IbBrightnessControl * brightness_control;
260- IbBrightnessPowerdControl * brightness_powerd_control;
261+ IbBrightnessUscreenControl * brightness_uscreen_control;
262
263 guint own_id;
264 guint actions_export_id;
265@@ -467,9 +467,9 @@
266 {
267 max = ib_brightness_control_get_max_value (self->priv->brightness_control);
268 }
269- else if (p->brightness_powerd_control)
270+ else if (p->brightness_uscreen_control)
271 {
272- max = ib_brightness_powerd_control_get_max_value (self->priv->brightness_powerd_control);
273+ max = ib_brightness_uscreen_control_get_max_value (self->priv->brightness_uscreen_control);
274 }
275 *low = max * 0.05; /* 5% minimum -- don't let the screen go completely dark */
276 *high = max;
277@@ -500,9 +500,9 @@
278 {
279 brightness = ib_brightness_control_get_value (p->brightness_control);
280 }
281- else if (p->brightness_powerd_control)
282+ else if (p->brightness_uscreen_control)
283 {
284- brightness = ib_brightness_powerd_control_get_value (p->brightness_powerd_control);
285+ brightness = ib_brightness_uscreen_control_get_value (p->brightness_uscreen_control);
286 }
287 return g_variant_new_double (brightness_to_percentage (self, brightness));
288 }
289@@ -527,9 +527,9 @@
290 {
291 ib_brightness_control_set_value (self->priv->brightness_control, brightness);
292 }
293- else if (self->priv->brightness_powerd_control)
294+ else if (self->priv->brightness_uscreen_control)
295 {
296- ib_brightness_powerd_control_set_value (self->priv->brightness_powerd_control, brightness);
297+ ib_brightness_uscreen_control_set_value (self->priv->brightness_uscreen_control, brightness);
298 }
299
300 update_brightness_action_state (self);
301@@ -1024,7 +1024,7 @@
302
303 // g_clear_pointer has NULL check inside.
304 g_clear_pointer (&p->brightness_control, ib_brightness_control_free);
305- g_clear_pointer (&p->brightness_powerd_control, ib_brightness_powerd_control_free);
306+ g_clear_pointer (&p->brightness_uscreen_control, ib_brightness_uscreen_control_free);
307
308 indicator_power_service_set_device_provider (self, NULL);
309
310@@ -1038,8 +1038,8 @@
311 static void
312 indicator_power_service_init (IndicatorPowerService * self)
313 {
314- GDBusProxy *powerd_proxy;
315- brightness_params_t powerd_brightness_params;
316+ GDBusProxy *uscreen_proxy;
317+ brightness_params_t brightness_params;
318 priv_t * p = G_TYPE_INSTANCE_GET_PRIVATE (self,
319 INDICATOR_TYPE_POWER_SERVICE,
320 IndicatorPowerServicePrivate);
321@@ -1049,10 +1049,10 @@
322
323 p->settings = g_settings_new ("com.canonical.indicator.power");
324
325- powerd_proxy = powerd_get_proxy(&powerd_brightness_params);
326- if (powerd_proxy != NULL)
327+ uscreen_proxy = uscreen_get_proxy(&brightness_params);
328+ if (uscreen_proxy != NULL)
329 {
330- p->brightness_powerd_control = ib_brightness_powerd_control_new(powerd_proxy, powerd_brightness_params);
331+ p->brightness_uscreen_control = ib_brightness_uscreen_control_new(uscreen_proxy, brightness_params);
332 }
333 else
334 {

Subscribers

People subscribed via source and target branches