Merge ~peat-new/repowerd:synchronous-unity-dbus-call into repowerd:master

[Ratchanan Srirattanamet]

adaptors: make the screen-on call to u-s-c synchronous

Normally, repowerd calls the unity-screen-compositor to unblank the
screen via DBus asynchronously before incrementally increase the
brightness. The problem is that, on some device (such as Fairphone 2),
the brightness is ignored if the screen is blanked. So, there is a race
condition between the brightness is incremented to the max and the
screen being actually unblanked.

This commit makes this call synchronous, making sure that the screen will
be unblanked completely before the brightness is set.

adaptors: make the screen-on call to u-s-c synchronous

Normally, repowerd calls the unity-screen-compositor to unblank the
screen via DBus asynchronously before incrementally increase the
brightness. The problem is that, on some device (such as Fairphone 2),
the brightness is ignored if the screen is blanked. So, there is a race
condition between the brightness is incremented to the max and the
screen being actually unblanked.

This commit makes this call synchronous, making sure that the screen will
be unblanked completely before the brightness is set.

[Alexandros Frantzis (afrantzis)]

Thanks for the patch.

Making the call synchronous as you propose has the side effect that if the system compositor blocks for whatever reason, it will also block repowerd for the default dbus timeout which is rather long (25 seconds).

I am considering making the call synchronous with a shorter, 1 second timeout, so even if the compositor has entered a problematic state, it won't block repowerd for more than 1 second.

Unfortunately I don't have a fp2 to try this out. Could you check if a 1 second timeout is enough for the fp2?

review: Needs Information

a8f4e85... by Ratchanan Srirattanamet on 2017-03-18

adaptors: make the screen-on call to u-s-c synchronous

Normally, repowerd calls the unity-screen-compositor to unblank the
screen via DBus asynchronously before incrementally increase the
brightness. The problem is that, on some device (such as Fairphone 2),
the brightness is ignored if the screen is blanked. So, there is a race
condition between the brightness is incremented to the max and the
screen being actually unblanked.

This commit makes this call synchronous, making sure that the screen will
be unblanked completely before the brightness is set. A small timeout of
1 second is added to prevent unity-screen-compositor from blocking
repowerd for too long.

[Ratchanan Srirattanamet (peat-new)]

> I am considering making the call synchronous with a shorter, 1 second timeout,
> so even if the compositor has entered a problematic state, it won't block
> repowerd for more than 1 second.
>
> Unfortunately I don't have a fp2 to try this out. Could you check if a 1
> second timeout is enough for the fp2?

Sorry for the delay. I've tested 1-second timeout on FP2 and it seems to work fine. So, I've updated the patch for that timeout and also rebase it on current master.

[Alexandros Frantzis (afrantzis)]

Thanks for the updated patch. I have merged it into repowerd master.

review: Approve

diff --git a/src/adapters/unity_display.cpp b/src/adapters/unity_display.cpp
index 057c0d9..e89435f 100644
--- a/src/adapters/unity_display.cpp
+++ b/src/adapters/unity_display.cpp
@@ -82,7 +82,7 @@ void repowerd::UnityDisplay::turn_on(DisplayPowerControlFilter filter)
 
     log->log(log_tag, "turn_on(%s)", filter_str.c_str());
 
-    g_dbus_connection_call(
+    g_dbus_connection_call_sync(
         dbus_connection,
         unity_display_bus_name,
         unity_display_object_path,
@@ -91,8 +91,7 @@ void repowerd::UnityDisplay::turn_on(DisplayPowerControlFilter filter)
         g_variant_new("(s)", filter_str.c_str()),
         nullptr,
         G_DBUS_CALL_FLAGS_NONE,
-        -1,
-        nullptr,
+        /* timeout_msec */ 1000,
         nullptr,
         nullptr);
 }