search

Crozzers / screen_brightness_control

A Python tool for controlling the brightness of your monitor
https://crozzers.github.io/screen_brightness_control
MIT License
50 stars 9 forks source link

Errors caused if monitor is turned off and on again #3

Closed Crozzers closed 3 years ago

Crozzers commented 3 years ago

I just came across something that might need some preliminary thinking. At least on the VCP side, when you switch off the monitor either using the power button or by unplugging it, the handles are invalidated and the functions provided by the windows submodule don't work anymore. In ipython I did the following to reload the VCP state. I also could see that it takes a bit of time (about 1 second). 

import screen_brightness_control as sbc
...
sbc.set_brightness(0,20)#works
...switch off monitor..
sbc.set_brightness(0,20)#does not work 
...
sbc.windows.methods[1]=sbc.windows.VCP() #restart VCP, this takes about 1 second

sbc.set_brightness(0,20) #works again

Maybe a more granular approach might be needed.

_Originally posted by @lcharles in https://github.com/Crozzers/screen_brightness_control/issues/2#issuecomment-729715648_

Crozzers commented 3 years ago

One issue I have found with that commit is that, while the issue is suppressed, the list_monitors function still returns a monitors name even if the monitor has been turned off

lcharles commented 3 years ago

One issue I have found with that commit is that, while the issue is suppressed, the list_monitors function still returns a monitors name even if the monitor has been turned off

I think it might be caused by the fact that it reuses the handles. I'm not 100% sure but it looks like the monitor handles should be created and destroyed at every request. In the big picture I think listing the monitors should be based on their brand name and some EDID number, the handles should probably be used only for the lifespan of the request. What I am 100% sure though is that if I switch off my monitor, the handles are not valid anymore.

If you want, I can create an ipython notebook using nothing but the win32 API so that you could replay the critical bits of code. Also if you don't want to go insane with these corner cases I think it is more convenient if you avoid any kind of internal state in the library. This would mean returning to the client a list of monitors and their characteristics but nothing related to the windows API. The matching would just be done internally between some unique ID that the client passes and the results of new calls to the win32 API.

Crozzers commented 3 years ago

So what you're suggesting is that we don't initialize a VCP, WMI or Monitor class with internal caches, but we re-request the info as needed every time a method is called? This sounds like it would slow down each individual request (and speed up the import) but I do think that this is the way forward. I'll look into this concept tonight.

As for creating an ipython notebook, don't worry about it, I don't use ipython. Cheers for the offer though

lcharles commented 3 years ago

So what you're suggesting is that we don't initialize a VCP, WMI or Monitor class with internal caches, but we re-request the info as needed every time a method is called? This sounds like it would slow down each individual request (and speed up the import) but I do think that this is the way forward. I'll look into this concept tonight.

Yes, I'll try to benchmark some requests to see which are really critical.

As for creating an ipython notebook, don't worry about it, I don't use ipython. Cheers for the offer though

I can also create a test folder where I write some tests that I find useful. You just need to be careful to exclude them in setuptools.

Crozzers commented 3 years ago

So, the dev branch has been updated. The WMI and VCP classes are not initialized upon import and cannot be initialized, but the methods within can still be called. So instead of doing

import screen_brightness_control as sbc
vcp = sbc.windows.VCP()
vcp.set_brightness(50)

you can now do

import screen_brightness_control as sbc
sbc.windows.VCP.set_brightness(50)

This should completely fix the monitor errors as the monitor handles are opened and closed upon request, with nothing cached. The only issue is that the function that retrieves the monitors model name takes ~1.5 seconds to run and would end up being called ~7 times, which is unacceptable. I have made it so that this function is only called if you use the Monitor class like so:

import screen_brightness_control as sbc
monitor = sbc.windows.Monitor(0)
print(vars(monitor))
> {'serial': '5&d382771&1&UID37122', 'name': 'BenQ BNQ78A7', 'method': <class 'screen_brightness_control.windows.VCP'>, 'manufacturer': 'BenQ', 'manufacturer_id': 'BNQ', 'model': 'BNQ78A7', 'model_name': None, 'index': 0}
print(monitor.model_name) # is then loaded on-command
> 'GL2450HM'

I intend to merge the dev branch after I have tested everything and (hopefully) after VCP support has been added to the Linux side of things but the latter is not guaranteed

Crozzers commented 3 years ago

v0.5.0 has been merged to the master branch and will be published on PyPi later today

lcharles commented 3 years ago

Great !! Do you have a plan or a set of features that you want to provide for 1.0 ?

Crozzers commented 3 years ago
  1. DDC/CI command support for Linux
  2. Rock solid reliability
  3. Full documentation
  4. Any support for MAC

Those are the main ones that I can think of. I will probably add these as I go and then, once the last one is added, bump the version to 1.0 but that seems a long way off at the moment.

lcharles commented 3 years ago
  1. DDC/CI command support for Linux
  2. Rock solid reliability
  3. Full documentation
  4. Any support for MAC

Those are the main ones that I can think of. I will probably add these as I go and then, once the last one is added, bump the version to 1.0 but that seems a long way off at the moment.

Great, so with regard to point 2 I will probably be able to contribute, my goal is to develop a little app that would stay in the notification area and registers two keyboard shortcuts to change the brightness. I have to evaluate the possibility to do this in Qt to make it cross-platform.

Crozzers commented 3 years ago

That sounds interesting. I cannot offer any advice on that as I haven't used Qt but it would definitely be a useful tool for Linux users

lcharles commented 3 years ago

The only thing is that the author of ddcutil for Linux is also developing ddcui in Qt/C++.

Crozzers commented 3 years ago

That is true but that only covers DDC/CI monitors (I think), not laptop screens. Those can be covered by the programs that this module calls (Light, XRandr, XBacklight) so it could be a good 'one glove fits all' type approach. If you only want to cover the Windows side I would be happy to submit a PR to add support for Linux once I have added DDC/CI linux support on this module

lcharles commented 3 years ago

That is true but that only covers DDC/CI monitors (I think), not laptop screens. Those can be covered by the programs that this module calls (Light, XRandr, XBacklight) so it could be a good 'one glove fits all' type approach. If you only want to cover the Windows side I would be happy to submit a PR to add support for Linux once I have added DDC/CI linux support on this module

Yeah sorry about that I completely forgot about the laptop part. I will probably try to make to make it cross platform then. The thing is that I don't really have a linux box right now, the other issue I have to dig is that global keyboard shortcuts can be done on X but not on Wayland. And in my opinion it is more the job of the DE to dispatch the key events. I don't think this means it should be written once with KDE framework and one in GTK3 though, but I need to be careful with how I split the code.

Crozzers commented 3 years ago

That's pretty much Linux in a nutshell. Great for privacy, fast and lots of choice but no two programs ever work the same way. Theres no easy to access Linux API like there is on Windows, in the form of WMI and windll. This is the reason it takes so long to bring anything to Linux, trying to find the best and most widely supported way of doing things (like DDC commands or global keyboard shortcuts).