Unity 8

Merge lp:~albaguirre/unity8/use-new-display-power-state-interface into lp:unity8

use-new-display-power-state-interface
Merge into trunk

Proposed by kevin gunn on 2014-05-14

Status:	Merged
Approved by:	Michael Terry on 2014-06-24
Approved revision:	897
Merged at revision:	985
Proposed branch:	lp:~albaguirre/unity8/use-new-display-power-state-interface
Merge into:	lp:unity8
Diff against target:	155 lines (+33/-33) 5 files modified plugins/Powerd/Powerd.cpp (+16/-16) plugins/Powerd/Powerd.h (+8/-8) qml/Shell.qml (+1/-1) tests/mocks/Powerd/Powerd.h (+7/-7) tests/qmltests/tst_Shell.qml (+1/-1)
To merge this branch:	bzr merge lp:~albaguirre/unity8/use-new-display-power-state-interface
Related bugs:	Link a bug report

Reviewer	Review Type	Date Requested	Status
PS Jenkins bot (community)	continuous-integration		Needs Fixing on 2014-06-24
Michael Terry			Approve on 2014-06-24
Albert Astals Cid (community)		2014-05-14	Needs Fixing on 2014-06-02
Review via email: mp+219552@code.launchpad.net

Commit message

Update Powerd plugin and Shell.qml to accommodate changes in the display power state notification.

Description of the change

There are some changes being done to powerd and unity-system-compositor in regards to display power state notifications - the interface name and method has changed slightly.

* Are there any related MPs required for this MP to build/function as expected?
Yes:
The dependent changes in powerd and USC are here:
https://code.launchpad.net/~albaguirre/unity-system-compositor/screen-power-state-handling/+merge/213957

https://code.launchpad.net/~albaguirre/powerd/remove-input-inactivity-handling/+merge/219449

* Did you perform an exploratory manual test run of your code change and any related functionality?
Yes

* Did you make sure that your branch does not contain spurious tags?
Yes

* If you changed the packaging (debian), did you subscribe the ubuntu-unity team to this MP?
N/A

* If you changed the UI, has there been a design review?
N/A

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-05-14:

FAILED: Continuous integration, rev:887
No commit message was specified in the merge proposal. Click on the following link and set the commit message (if you want a jenkins rebuild you need to trigger it yourself):
https://code.launchpad.net/~albaguirre/unity8/use-new-display-power-state-interface/+merge/219552/+edit-commit-message

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity8-ci/2963/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

Albert Astals Cid (aacid) wrote on 2014-05-15:

Kevin/Alberto can you please set the commit message and then set as description the "MP Submission Checklist Template" of https://wiki.ubuntu.com/Process/Merges/Checklists/Unity8 ?

review: Needs Fixing

Revision history for this message

kevin gunn (kgunn72) wrote on 2014-05-15:

> Kevin/Alberto can you please set the commit message and then set as
> description the "MP Submission Checklist Template" of
> https://wiki.ubuntu.com/Process/Merges/Checklists/Unity8 ?

absolutely, we were just getting a silo up first for some pre-flight testing.

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-05-20:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity8-ci/3000/rebuild

review: Needs Fixing (continuous-integration)

lp:~albaguirre/unity8/use-new-display-power-state-interface updated on 2014-05-21

888. By Alberto Aguirre on 2014-05-21

Update to use display power state change reason

889. By Alberto Aguirre on 2014-05-21

merge trunk

890. By Alberto Aguirre on 2014-05-21

Keep hook to powerd auto-brightness

Update mock powerd.h to reflect changes in real powerd.h

Revision history for this message

Alberto Aguirre (albaguirre) wrote on 2014-05-21:

* Are there any related MPs required for this MP to build/function as expected? Please list.
Yes there are:

https://code.launchpad.net/~albaguirre/unity-system-compositor/screen-power-state-handling/+merge/213957

https://code.launchpad.net/~albaguirre/powerd/remove-input-inactivity-handling/+merge/219449

* Did you perform an exploratory manual test run of your code change and any related functionality?
Yes. I tested that the greeter is still shown when powering on/off the screen through the power key and that the greeter is not shown when screen is turned off due to proximity events.

* Did you make sure that your branch does not contain spurious tags?
Yes

* If you changed the packaging (debian), did you subscribe the ubuntu-unity team to this MP?
N/A

* If you changed the UI, has there been a design review?
N/A

lp:~albaguirre/unity8/use-new-display-power-state-interface updated on 2014-05-21

891. By Alberto Aguirre on 2014-05-21: Fix missing #endif

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-05-21:

FAILED: Continuous integration, rev:891
http://jenkins.qa.ubuntu.com/job/unity8-ci/3010/
Executed test runs:
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-utopic-touch/366
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-qmluitests-utopic/45
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-utopic-amd64-ci/104
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-utopic-armhf-ci/104
        deb: http://jenkins.qa.ubuntu.com/job/unity8-utopic-armhf-ci/104/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-utopic-i386-ci/104
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-mako/838
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/716
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/716/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/7427

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity8-ci/3010/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-05-21:

FAILED: Continuous integration, rev:891
http://jenkins.qa.ubuntu.com/job/unity8-ci/3012/
Executed test runs:
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-utopic-touch/370
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-qmluitests-utopic/47
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-utopic-amd64-ci/106
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-utopic-armhf-ci/106
        deb: http://jenkins.qa.ubuntu.com/job/unity8-utopic-armhf-ci/106/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-utopic-i386-ci/106
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-mako/841
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/727
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/727/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/7437

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity8-ci/3012/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

Albert Astals Cid (aacid) wrote on 2014-05-23:

Just for curiosity, do we need the three branches at the same time or can we land the u-s-c and powerd ones and then this one? In case they need to land at the same time is there a silo available for testing?

review: Needs Information

Revision history for this message

Michał Sawicz (saviq) wrote on 2014-05-23:

It's in silo 001, but I'm not sure this is actually ready for review? Alberto, could you please mark this branch Work In Progress if that's what it is?

Revision history for this message

Alberto Aguirre (albaguirre) wrote on 2014-05-28:

@Saviq,

It's ready for review but not landing.

@Albert
They need to land at the same time.

Revision history for this message

Albert Astals Cid (aacid) wrote on 2014-06-02:

Text conflict in qml/Shell.qml
1 conflicts encountered.

review: Needs Fixing

lp:~albaguirre/unity8/use-new-display-power-state-interface updated on 2014-06-17

892. By Alberto Aguirre on 2014-06-16: merge trunk
893. By Alberto Aguirre on 2014-06-17: Hook auto brightness to unity screen instead of powerd
894. By Alberto Aguirre on 2014-06-17: tests: fix Powerd constant name
895. By Alberto Aguirre on 2014-06-17: merge trunk

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-06-17:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity8-ci/3154/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

Alberto Aguirre (albaguirre) wrote on 2014-06-18:

"
(2 failures / -12)
unity8.application_lifecycle.tests.test_application_lifecycle.ApplicationLifecycleTests.test_greeter_hides_on_app_focus
unity8.application_lifecycle.tests.test_application_lifecycle.ApplicationLifecycleTests.test_greeter_hides_on_app_open
"
^-- Expected since it needs the powerd/USC dependencies listed in description

Revision history for this message

Michał Sawicz (saviq) wrote on 2014-06-18:

On 18.06.2014 20:36, Alberto Aguirre wrote:
> ^-- Expected since it needs the powerd/USC dependencies listed in description

Shouldn't there be package/dependency version bumps then?

Revision history for this message

Alberto Aguirre (albaguirre) wrote on 2014-06-19:

> On 18.06.2014 20:36, Alberto Aguirre wrote:
> > ^-- Expected since it needs the powerd/USC dependencies listed in
> description
>
> Shouldn't there be package/dependency version bumps then?

I don't see a powerd dependency on the current debian/control...Should I add a line for USC?

Revision history for this message

Michael Terry (mterry) wrote on 2014-06-19:

With your new branches, yeah maybe USC could get a Recommends for powerd. Depends feels too strong. And Recommends would still pull in powerd by default.

Revision history for this message

Alberto Aguirre (albaguirre) wrote on 2014-06-19:

@mterry: I meant should I add a unity-system-compositor to Depends in unity8 Package?

lp:~albaguirre/unity8/use-new-display-power-state-interface updated on 2014-06-20

896. By Alberto Aguirre on 2014-06-20: merge trunk

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-06-20:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity8-ci/3189/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

Michael Terry (mterry) wrote on 2014-06-24:

> @mterry: I meant should I add a unity-system-compositor to Depends in unity8 Package?

Naw, I don't think so. But we're just listening to a DBus interface here. If USC isn't running, it's no big deal, right?

Code looks fine, let me test with silo.

lp:~albaguirre/unity8/use-new-display-power-state-interface updated on 2014-06-24

897. By Alberto Aguirre on 2014-06-24: merge trunk again...

Revision history for this message

Michael Terry (mterry) wrote on 2014-06-24:

Testing with the silo, it doesn't seem like the greeter comes up when I press the power button...

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-06-24:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity8-ci/3206/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

Michael Terry (mterry) wrote on 2014-06-24:

* Did you perform an exploratory manual test run of the code change and any related functionality?
- Yes, worked fine after a rebuild of the silo.

* Did CI run pass? If not, please explain why.
- No, but only because of existing qmluitest errors and the autopilot failure is because AP needs the rest of the silo. I tested myself and they passed.

review: Approve

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-06-24:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity8-ci/3208/rebuild

review: Needs Fixing (continuous-integration)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk

Subscribers

People subscribed via source and target branches

to all changes:

Alberto Aguirre

Mohammad Rakibul Hasan

Unity Team

 === modified file 'plugins/Powerd/Powerd.cpp'
 --- plugins/Powerd/Powerd.cpp	2014-03-26 17:54:40 +0000
 +++ plugins/Powerd/Powerd.cpp	2014-06-24 16:27:39 +0000
@@ -19,35 +19,35 @@
  #include "Powerd.h"
  #include <QDBusPendingCall>
--void autoBrightnessChanged(GSettings *settings, const gchar *key, QDBusInterface *powerd)
++void autoBrightnessChanged(GSettings *settings, const gchar *key, QDBusInterface *unityScreen)
+ {
      bool value = g_settings_get_boolean(settings, key);
--    powerd->asyncCall("userAutobrightnessEnable", QVariant(value));
++    unityScreen->asyncCall("userAutobrightnessEnable", QVariant(value));
+ }
  Powerd::Powerd(QObject* parent)
    : QObject(parent),
--    powerd(NULL)
++    unityScreen(NULL)
+ {
--    powerd = new QDBusInterface("com.canonical.powerd",
--                                "/com/canonical/powerd",
--                                "com.canonical.powerd",
--                                QDBusConnection::SM_BUSNAME(), this);
++    unityScreen = new QDBusInterface("com.canonical.Unity.Screen",
++                                     "/com/canonical/Unity/Screen",
++                                     "com.canonical.Unity.Screen",
++                                     QDBusConnection::SM_BUSNAME(), this);
--    powerd->connection().connect("com.canonical.powerd",
--                                 "/com/canonical/powerd",
--                                 "com.canonical.powerd",
--                                 "DisplayPowerStateChange",
--                                 this,
--                                 SIGNAL(displayPowerStateChange(int, unsigned int)));
++    unityScreen->connection().connect("com.canonical.Unity.Screen",
++                                      "/com/canonical/Unity/Screen",
++                                      "com.canonical.Unity.Screen",
++                                      "DisplayPowerStateChange",
++                                      this,
++                                      SIGNAL(displayPowerStateChange(int, int)));
      systemSettings = g_settings_new("com.ubuntu.touch.system");
--    g_signal_connect(systemSettings, "changed::auto-brightness", G_CALLBACK(autoBrightnessChanged), powerd);
--    autoBrightnessChanged(systemSettings, "auto-brightness", powerd);
++    g_signal_connect(systemSettings, "changed::auto-brightness", G_CALLBACK(autoBrightnessChanged), unityScreen);
++    autoBrightnessChanged(systemSettings, "auto-brightness", unityScreen);
+ }
  Powerd::~Powerd()
+ {
--    g_signal_handlers_disconnect_by_data(systemSettings, powerd);
++    g_signal_handlers_disconnect_by_data(systemSettings, unityScreen);
      g_object_unref(systemSettings);
+ }
 === modified file 'plugins/Powerd/Powerd.h'
 --- plugins/Powerd/Powerd.h	2014-03-26 17:54:40 +0000
 +++ plugins/Powerd/Powerd.h	2014-06-24 16:27:39 +0000
@@ -28,15 +28,15 @@
+ {
      Q_OBJECT
      Q_ENUMS(Status)
--    Q_FLAGS(DisplayFlag DisplayFlags)
++    Q_ENUMS(DisplayStateChangeReason)
  public:
--    enum DisplayFlag {
--        UseProximity          = 1, // Use proximity sensor to override screen state
--        DisableAutoBrightness = 2, // Force autobrightness to be disabled
--        Bright                = 4, // Request the screen to stay bright
++    enum DisplayStateChangeReason {
++        Unknown         = 0,
++        Inactivity      = 1, // Display changed state due to inactivity
++        PowerKey        = 2, // Display changed state due to user pressing power key
++        Proximity       = 3, // Display changed state due to proximity events
      };
--    Q_DECLARE_FLAGS(DisplayFlags, DisplayFlag)
      enum Status {
          Off,
@@ -47,10 +47,10 @@
      ~Powerd();
  Q_SIGNALS:
--    void displayPowerStateChange(int status, unsigned int flags);
++    void displayPowerStateChange(int status, int reason);
  private:
--    QDBusInterface *powerd;
++    QDBusInterface *unityScreen;
      GSettings *systemSettings;
  };
 === modified file 'qml/Shell.qml'
 --- qml/Shell.qml	2014-06-13 12:13:51 +0000
 +++ qml/Shell.qml	2014-06-24 16:27:39 +0000
@@ -452,7 +452,7 @@
          onDisplayPowerStateChange: {
              // We ignore any display-off signals when the proximity sensor
              // is active.  This usually indicates something like a phone call.
--            if (status == Powerd.Off && (flags & Powerd.UseProximity) == 0) {
++            if (status == Powerd.Off && reason != Powerd.Proximity) {
                  greeter.showNow();
+             }
 === modified file 'tests/mocks/Powerd/Powerd.h'
 --- tests/mocks/Powerd/Powerd.h	2013-08-21 20:04:24 +0000
 +++ tests/mocks/Powerd/Powerd.h	2014-06-24 16:27:39 +0000
@@ -25,15 +25,15 @@
+ {
      Q_OBJECT
      Q_ENUMS(Status)
--    Q_FLAGS(DisplayFlag DisplayFlags)
++    Q_ENUMS(DisplayStateChangeReason)
  public:
--    enum DisplayFlag {
--        UseProximity          = 1, // Use proximity sensor to override screen state
--        DisableAutoBrightness = 2, // Force autobrightness to be disabled
--        Bright                = 4, // Request the screen to stay bright
++    enum DisplayStateChangeReason {
++        Unknown         = 0,
++        Inactivity      = 1, // Display changed state due to inactivity
++        PowerKey        = 2, // Display changed state due to user pressing power key
++        Proximity       = 3, // Display changed state due to proximity events
      };
--    Q_DECLARE_FLAGS(DisplayFlags, DisplayFlag)
      enum Status {
          Off,
@@ -43,7 +43,7 @@
      explicit Powerd(QObject *parent = 0);
  Q_SIGNALS:
--    void displayPowerStateChange(int status, unsigned int flags);
++    void displayPowerStateChange(int status, int reason);
  };
  #endif
 === modified file 'tests/qmltests/tst_Shell.qml'
 --- tests/qmltests/tst_Shell.qml	2014-06-11 15:36:51 +0000
 +++ tests/qmltests/tst_Shell.qml	2014-06-24 16:27:39 +0000
@@ -195,7 +195,7 @@
              tryCompare(mainApp, "state", ApplicationInfo.Running);
              // Try to suspend while proximity is engaged...
--            Powerd.displayPowerStateChange(Powerd.Off, Powerd.UseProximity);
++            Powerd.displayPowerStateChange(Powerd.Off, Powerd.Proximity);
              tryCompare(greeter, "showProgress", 0);
              // Now really suspend