Unity 8

Merge lp:~mterry/unity8/display-proximity into lp:unity8

display-proximity
Merge into trunk

Proposed by Michael Terry on 2013-08-20

Status:

Merged

Approved by:

Michał Sawicz on 2013-08-21

Approved revision:

229

Merged at revision:

233

Proposed branch:

lp:~mterry/unity8/display-proximity

Merge into:

lp:unity8

Diff against target:

128 lines (+44/-11)

5 files modified

Shell.qml (+5/-3)
plugins/Powerd/Powerd.cpp (+2/-2)
plugins/Powerd/Powerd.h (+15/-1)
tests/mocks/Powerd/Powerd.h (+15/-2)
tests/qmltests/tst_Shell.qml (+7/-3)

To merge this branch:

bzr merge lp:~mterry/unity8/display-proximity

High

Fix Released

Link a bug report

Reviewer	Review Type	Date Requested	Status
Michał Sawicz		2013-08-20	Approve on 2013-08-21
PS Jenkins bot (community)	continuous-integration		Approve on 2013-08-21
Review via email: mp+181157@code.launchpad.net

Commit message

Listen to display-power-changed rather than system-power-changed signals when showing the greeter; ignore such signals when the proximity sensor is active

Description of the change

Listen to display-power-changed rather than system-power-changed signals when showing the greeter; ignore such signals when the proximity sensor is active.

We don't need to listen to power changes. The experience we really care about is whether the user sees the screen go black. That's when we want to turn on the greeter and unfocus apps. Plus, this gives us access to the flags that powerd sends with a display-power-changed signal that tell us whether the proximity sensors are enabled.

When proximity is on, we don't want to show the greeter. Because the user likely has the phone up to their ear for a call and want to resume what they were doing after pulling away from the ear.

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2013-08-20:

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

review: Needs Fixing (continuous-integration)

lp:~mterry/unity8/display-proximity updated on 2013-08-20

224. By Michael Terry on 2013-08-20: Update mock Powerd too

Revision history for this message

Michał Sawicz (saviq) wrote on 2013-08-20:

First of all - awesome! :)

Test needs fixing it seems (qmltestrunner.Shell::test_suspend: property showProgress Actual (): 0 Expected (): 1).

Can we have an enum for state... And actually, s/state/status/g so that we don't conflict with Item.state?

review: Needs Fixing

Revision history for this message

Michał Sawicz (saviq) wrote on 2013-08-20:

Ah, and I think we're missing Q_DECLARE_FLAGS() for completeness:

http://qt-project.org/doc/qt-5.0/qtcore/qobject.html#Q_FLAGS

review: Needs Information

lp:~mterry/unity8/display-proximity updated on 2013-08-20

225. By Michael Terry on 2013-08-20: And update suspend qmltest

Revision history for this message

Michael Terry (mterry) wrote on 2013-08-20:

Updated, thanks!

lp:~mterry/unity8/display-proximity updated on 2013-08-20

226. By Michael Terry on 2013-08-20: Add a Status enum
227. By Michael Terry on 2013-08-20: Use enums in test too

Revision history for this message

Michał Sawicz (saviq) wrote on 2013-08-20:

I get a failure in the test still:

FAIL! : qmltestrunner::Shell::test_suspend() property mainStageFocusedApplication
   Actual (): null
   Expected ():
   Loc: [/home/michal/dev/canonical/unity8/repo/tests/qmltests/tst_Shell.qml(139)]

The problem itself is fixed, though.

review: Needs Fixing

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2013-08-20:

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

review: Needs Fixing (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2013-08-21:

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

review: Needs Fixing (continuous-integration)

lp:~mterry/unity8/display-proximity updated on 2013-08-21

228. By Michael Terry on 2013-08-21: Whoops, fix argument name to match the Qml plugin

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2013-08-21:

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

review: Approve (continuous-integration)

Revision history for this message

Michał Sawicz (saviq) wrote on 2013-08-21:

The greeter does not come in on suspend now. It might be a powerd bug, but still problematic for us.

review: Needs Fixing

Revision history for this message

Michał Sawicz (saviq) wrote on 2013-08-21:

Humm... Did you ever see onDisplayPowerStateChange actually being called? I can't seem to, whether on power button, timeout suspend or proximity sensor...

Feels like we need an autopilot test for this...

Also, I seem to lose input to the phone app sometimes with this (not with trunk), I wonder what happens there...

review: Needs Information

Revision history for this message

Michael Terry (mterry) wrote on 2013-08-21:

For posterity: I mentioned on IRC that this is working for me, both the signal being called and input in phone app...

lp:~mterry/unity8/display-proximity updated on 2013-08-21

229. By Michael Terry on 2013-08-21: Fix indentation on Status enum

Revision history for this message

Michał Sawicz (saviq) wrote on 2013-08-21:

Yup, working now - not sure what was the issue. Input was not restarting phone-app between unity8 restarts.

review: Approve

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:

Michael Terry

Mohammad Rakibul Hasan

Unity Team

 === modified file 'Shell.qml'
 --- Shell.qml	2013-08-15 11:21:26 +0000
 +++ Shell.qml	2013-08-21 20:04:45 +0000
@@ -497,11 +497,13 @@
+             }
+         }
--        onPowerStateChange: {
--            if (state == 0) { // suspend
++        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) {
                  powerConnection.setFocused(false);
                  greeter.show();
--            } else if (state == 1) { // active
++            } else if (status == Powerd.On) {
                  powerConnection.setFocused(true);
+             }
+         }
 === modified file 'plugins/Powerd/Powerd.cpp'
 --- plugins/Powerd/Powerd.cpp	2013-08-02 16:18:28 +0000
 +++ plugins/Powerd/Powerd.cpp	2013-08-21 20:04:45 +0000
@@ -30,7 +30,7 @@
      powerd->connection().connect("com.canonical.powerd",
                                   "/com/canonical/powerd",
                                   "com.canonical.powerd",
--                                 "SysPowerStateChange",
++                                 "DisplayPowerStateChange",
                                   this,
--                                 SIGNAL(powerStateChange(int)));
++                                 SIGNAL(displayPowerStateChange(int, unsigned int)));
+ }
 === modified file 'plugins/Powerd/Powerd.h'
 --- plugins/Powerd/Powerd.h	2013-08-02 16:18:28 +0000
 +++ plugins/Powerd/Powerd.h	2013-08-21 20:04:45 +0000
@@ -26,12 +26,26 @@
  class Powerd: public QObject
+ {
      Q_OBJECT
++    Q_ENUMS(Status)
++    Q_FLAGS(DisplayFlag DisplayFlags)
  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
++    };
++    Q_DECLARE_FLAGS(DisplayFlags, DisplayFlag)
++
++    enum Status {
++        Off,
++        On,
++    };
++
      explicit Powerd(QObject *parent = 0);
  Q_SIGNALS:
--    void powerStateChange(int state);
++    void displayPowerStateChange(int status, unsigned int flags);
  private:
      QDBusInterface *powerd;
 === modified file 'tests/mocks/Powerd/Powerd.h'
 --- tests/mocks/Powerd/Powerd.h	2013-08-09 14:35:36 +0000
 +++ tests/mocks/Powerd/Powerd.h	2013-08-21 20:04:45 +0000
@@ -20,17 +20,30 @@
  #define UNITY_MOCK_POWERD_H
  #include <QtCore/QObject>
--#include <QtDBus/QDBusInterface>
  class Powerd: public QObject
+ {
      Q_OBJECT
++    Q_ENUMS(Status)
++    Q_FLAGS(DisplayFlag DisplayFlags)
  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
++    };
++    Q_DECLARE_FLAGS(DisplayFlags, DisplayFlag)
++
++    enum Status {
++        Off,
++        On,
++    };
++
      explicit Powerd(QObject *parent = 0);
  Q_SIGNALS:
--    void powerStateChange(int state);
++    void displayPowerStateChange(int status, unsigned int flags);
  };
  #endif
 === modified file 'tests/qmltests/tst_Shell.qml'
 --- tests/qmltests/tst_Shell.qml	2013-08-15 08:09:39 +0000
 +++ tests/qmltests/tst_Shell.qml	2013-08-21 20:04:45 +0000
@@ -125,13 +125,17 @@
              var mainApp = ApplicationManager.mainStageFocusedApplication;
              tryCompareFunction(function() { return mainApp != null; }, true);
--            // Now suspend
--            Powerd.powerStateChange(0);
++            // Try to suspend while proximity is engaged...
++            Powerd.displayPowerStateChange(Powerd.Off, Powerd.UseProximity);
++            tryCompare(greeter, "showProgress", 0);
++
++            // Now really suspend
++            Powerd.displayPowerStateChange(Powerd.Off, 0);
              tryCompare(greeter, "showProgress", 1);
              tryCompare(ApplicationManager, "mainStageFocusedApplication", null);
              // And wake up
--            Powerd.powerStateChange(1);
++            Powerd.displayPowerStateChange(Powerd.On, 0);
              tryCompare(ApplicationManager, "mainStageFocusedApplication", mainApp);
              tryCompare(greeter, "showProgress", 1);
+         }