Skip to content

Conversation

@logiclrd
Copy link

This pull request is being created as a draft because I have no way to test the code. :-)

  • I don't have a Linux or Mac build environment. (I do have a Linux system but it doesn't have a working Qt installation.)
  • I tried to configure a Windows build environment but messed up somewhere along the way. It's trying to use CL to build the C code, but it's using GNU extensions so that's not working.

I have executed the Windows and Linux specific inhibitor code in isolated test projects and it builds and seems to work. But, I don't have any way to tell if the other code even compiles. I'm hoping there will be build pipelines that'll take care of at least part of that.

Someone with a Mac will need to test the Mac code.

This PR is a direct response to a poor user experience I had with the tool. Per issue #1188, I set up an operation to download & write an image to a Micro SD card, and then I left the system doing its thing. When I returned, I found out that the system had deemed itself idle and gone to sleep after 15 minutes, long before the download completed.

This PR aims to address this by leveraging OS-specific functionality to inhibit autosuspend.

  • On Windows, it sets the thread execution state to include the flag ES_SYSTEM_REQUIRED.
  • On OS X, it creates an IOPM assertion of type PreventUserIdleSystemSleep.
  • On Linux, it tries three strategies:
    • It attempts to connect to the DBus endpoint org.gnome.SessionManager. If this succeeds, it makes a call to the Inhibit method.
    • It attempts to launch system utility systemd-inhibit.
    • It attempts to launch system utility kde-inhibit.

These -inhibit utilities in the Linux-specific side work by taking a command and executing it in a scope that inhibits whatever action is to be inhibited. The inhibition remains in effect until the process exits. In order to invert this control mechanism, the code here creates a FIFO and then has the command in the -inhibit scope read from it. It can then terminate the read at any time by closing its end of the pipe.

Any errors are silently ignored; a best effort was made but if it can't explicitly inhibit suspending, we still want to perform the download & write operation.

…asses WindowsSuspendInhibitor, LinuxSuspendInhibitor and MacSuspendInhibitor in the per-platform directories.
…m-specific SuspendInhibitor implementation for its lifetime.
@logiclrd
Copy link
Author

Well shucks 🙂 There's no C.I., it seems. Hmm.

@tdewey-rpi
Copy link
Collaborator

Conceptually approved, but I can't merge it until I've put it through a round of testing.

This feature is sufficiently impactful that it should block 2.0 release.

I expect to come back to this PR on Thursday afternoon at the earliest.

@logiclrd
Copy link
Author

a round of testing

Indeed :-) The main reason it was over a month between the issue and the PR is that I hate putting forth code I haven't tested myself. But, ultimately, I struggled with getting a development environment working and still wanted to contribute this feature, so here we are, a draft PR with no end-to-end testing yet. My apologies that that's the best I could muster.

@@ -1,5 +1,7 @@
# Windows platform-specific sources and link settings

set(CMAKE_DLLTOOL "C:\\msys64\\mingw64\\bin\\dlltool.exe")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is fairly dangerous - can we obtain it from the mingw64_root variable?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh shoot, I don't think I meant to commit that. My local build was failing because some crucial link wasn't in place, so I hardcoded the path just to make it run for me. (Ultimately I still didn't get it working overall, though.)

@tdewey-rpi
Copy link
Collaborator

Broadly happy after initial review.

catch (...) patterns make me exceptionally nervous, but in this case the code you're guarding is relatively constrained, and not something where differentiated handling would offer an advantage, so I'm prepared to let it pass by.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants