WSL Method Implementation. #190
Conversation
|
|
||
| from wakepy.core import DbusAdapter, Method, ModeName, PlatformName | ||
|
|
||
| class Flags(enum.StrEnum): |
There was a problem hiding this comment.
The enum.StrEnum is unfortunately available only for python 3.11 and above. I've created my own backport which can be imported from wakepy.core. Let's use that instead so we can keep wakepy supported from python 3.7 onwards.
|
|
||
|
|
||
| class WslSetThreadExecutionState(Method, ABC): | ||
| """This is a method which calls the SetThreadExecutionState function from |
There was a problem hiding this comment.
Thanks for keeping a good level of docstrings / comments!
I've been using a strict line limits of 79 for comments and dosctrings, and 88 (from black) for formatting the code. I guess I should write those down to some contribution guide, and probably add some tests for the comments line length.
If you're using vs code, you could add this to your workspace settings:
"editor.rulers": [
{
"column": 79,
"color": "#664766"
},
{
"column": 88,
"color": "#7e4343"
},
],
I have a laptop with very small screen and these line lengths make it possible to see two tabs of code side-by-side :)
| self._powershell = Popen(["powershell.exe"], stdout=PIPE, stderr=None, stdin=PIPE, universal_newlines=True) | ||
|
|
||
| # C# Code to allow PowerShell to call SetThreadExecutionState | ||
| code = """$code = @' |
There was a problem hiding this comment.
I wonder if we could use textwrap.dedent to make this indented, too?
We could also include the explanation for the non-obvious SetLastError. Perhaps
SetLastError makes it possible to look up the result of the function (currently not used, though).
| # Write a blank line to complete the code block. | ||
| self._write("") | ||
|
|
||
| # Add the code above to the PowerShell Context. |
There was a problem hiding this comment.
This was the line I had question in the initial version, so I suggest we add your answer as comment :D something like:
Some C# code to add the `code` above to the PowerShell Context. Add-Type call sets it up as something we can call within PowerShell.
?
|
|
||
| # Tests the result of the SetThreadExecutionState. | ||
| # Close PowerShell if we failed. This is easy to test for. | ||
| self._write("([System.Runtime.InteropServices.Marshal]::GetLastWin32Error() -eq 0) | Where-Object { $_ } | Foreach-Object { exit -1 }") |
There was a problem hiding this comment.
This goes a bit above my head but nice that there's a check for the success and RuntimeError if it did not succeed!
|
Hi @ccrutchf , thanks for the PR! Very nice work! :) I commented on the code about few small details. re: Unit testsYeah this is the same pain in all of the wakepy.Methods, as they all need to call some other software to work. I think it's enough that the automatic tests test that the external software gets the calls we expect. I've been just mocking or faking the required parts and checking that they're called in correct order and with correct arguments. Then, in manual tests (done once per Method when creating it), I check that the mode is actually activated and it works as it should. I just ran Manual testsI could boot to Windows to test this manually. Did you already test this on WSL? Windows support?I was wondering if this would actually work also on Windows? If so, this could supersede the old |
|
Hi @ccrutchf , have you had time to think about unit tests? Would you still like to provide them? If not, I could take this as is, merge to dev (after a rebase), and write the unit tests :) |
|
Hi, I removed the |
This is an initial pass to fix #36, #161. It is missing unit tests. As it calls out to PowerShell, I don't know how to best write unit tests testing the functionality.
I figured that as I try to come up with unit tests, I could get your feedback on this PR.
This could also be adapted to fix #167.