Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement listener for macOS #25

Closed
albertosottile opened this issue Jul 18, 2022 · 6 comments
Closed

Implement listener for macOS #25

albertosottile opened this issue Jul 18, 2022 · 6 comments
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@albertosottile
Copy link
Owner

albertosottile commented Jul 18, 2022

A listener function for macOS is currently missing. As stated in the README, calling this function on this platform raises NotImplementedError.

EDIT:

Implementation requirements

The listener function must:

  • accept a callback function (Callable[[str], None]) as its only argument
  • call the passed callback function, with the new theme name as argument, whenever there is a theme update on the system
  • be able to run from the main thread
  • be able to run from a background thread (see the example in the README)
  • not continuously poll the system (e.g. run darkdetect.theme() in a loop) as that wastes resources

The listener function should, if possible:

  • do not depend on external C extensions that have to be compiled (we would like to have pure Python wheels)
  • do not depend on packages not included in the Python standard library
    • if additional dependencies from PyPI are required, those must be only included in a setuptools extra (e.g. pip install darkdetect[macos-listener])

Related: #14

@albertosottile albertosottile added enhancement New feature or request help wanted Extra attention is needed labels Jul 18, 2022
@albertosottile albertosottile pinned this issue Jul 18, 2022
@albertosottile albertosottile changed the title Listener for macOS Implement listener for macOS Jul 18, 2022
@albertosottile
Copy link
Owner Author

I invested some time into trying to implement this, but so far I had lo luck.

If anyone is willing to pick up this task, further information on the most severe issues can be found here: https://stackoverflow.com/questions/72982210/swift-and-python-get-a-runloop-that-can-run-in-threading and here: https://developer.apple.com/forums/thread/710135?login=true&page=1&r_s_legacy=true#720551022

I pushed the library implementations that I tried in the macos_listener_attempts branch, but none of them is functional from a secondary thread (a fundamental requirement for this package).

@zwimer
Copy link
Contributor

zwimer commented Nov 30, 2022

Is there a reason something akin to this would not work?

def listener(callback):
  old = isDark()
  while True:
    x = isDark()
    if x != old:
      old = x
      callback(x)
    time.sleep(.01)

def listen(callback):
  threading.Thread(target=listener, args=(callback,)).start()

This seemed to work for me, though maybe you'd want theme() instead of isDark?

I realize this might not be an ideal listener, but given the current lack of any, might this be acceptable in the meantime?

@zwimer
Copy link
Contributor

zwimer commented Nov 30, 2022

A PR of the above: #27

@albertosottile
Copy link
Owner Author

@zwimer Thanks for your contribution. I apologize if I did not reply sooner, but I think this approach is not truly viable. This thread will constantly call sleep and the ctypes reading code, which will have a CPU impact on the host machine and an impact on the other threads due to the constant GIL lock/release. I did not test this, but I assume the effect on e.g. laptops battery life would be significant.

Therefore, I would be against integrating this in the darkdetect codebase, as any programmer would expect that the listener function would not behave like this. The general, and correct, expectation would be that the spawned thread does not consume any constant resources and only fires the callback when needed. The approach proposed here does not allow that.

As you mentioned, though, as a last resort measure any developer can implement a similar approach in their own codebase, where they have full control on the secondary effects. But I would rather not add this in the codebase, especially hidden under a listener API that, as mentioned before, carries a different set of expectations..

@zwimer
Copy link
Contributor

zwimer commented Dec 6, 2022

@albertosottile I implemented a proper listener here: #30

@albertosottile
Copy link
Owner Author

Closed in #30, thanks to @zwimer.

@albertosottile albertosottile unpinned this issue Dec 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

2 participants