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

Continuation of #7 #8

Closed Crozzers closed 3 years ago

Crozzers commented 3 years ago

7 Got a bit messy so this is a new issue for that same issue

Crozzers commented 3 years ago

@shadoWalker89 does this code do anything interesting?

import screen_brightness_control as s
from ctypes import windll
from ctypes.wintypes import DWORD, BYTE, HANDLE
for m in s.windows.VCP.iter_physical_monitors():
    print(windll.dxva2.SetVCPFeature(HANDLE(m), BYTE(0x10), DWORD(50)))

I was doing some thinking about the duplicate monitor showing up and I think that perhaps you have a laptop screen that can be addressed by both wmi and vcp. This could very well have confused the monitor/display kwarg filtering as it's not really designed to handle that kind of issue. It assumes a monitor is wmi or vcp only. If the code snippet above changes both monitor's brightness then that would confirm this theory. If not then it's back to the drawing board. Also what laptop are you using?

shadoWalker89 commented 3 years ago

To avoid any kind of confusion, the the master branch (0.7.0) and the dev branch at the current state (which is also the state after finishing from https://github.com/Crozzers/screen_brightness_control/issues/6) changing the brightness work flawlessly on both screens the laptop and the external one with or without the -d flag and on all the modes available from Windows + P shortcut.

The problem is not in changing the brightness it's only in the integrated monitor showing up twice in the list with -l flag The integrated monitor show up twice in the list only if i'm using the Extend mode or using the integrated monitor by it self without the external one.

And yes in the tests that i did in https://github.com/Crozzers/screen_brightness_control/issues/7 the integrated monitor works only with wmi and the external one works only with vcp

Also the code snippet that you provided in this issue confirms that even more as i execute it with both monitors on it prints

0
1

and the brightness changes to 50 on the external monitor only

When it comes to the laptop, i tested this on two laptops and i got the same results the first is Dell g5 the second is acer predator helios 300

As a closing note, if you keep the package the way it is now it will work without any problems at all from what i tested on both laptops with or without the -d flag.

Crozzers commented 3 years ago

I assume that "working with the -d flag" only doesn't work on the duplicate entry, it works on all the others, however, as stated in #7. If you use the -l flag with --verbose enabled, do both the integrated display and it's duplicate have the same EDID string? If you don't want to compare the EDID by eye you can probably run

import screen_brightness_control as s
laptop_screen = s.windows.WMI.get_display_info()[0]
duplicate_entry = s.windows.VCP.get_display_info()[0] # or whichever one is the duplicate entry
print(laptop_screen['edid'] == duplicate_entry['edid'])

I expect their serial numbers to be different and this is why they both show up because duplicates are filtered out using that serial.

shadoWalker89 commented 3 years ago

Yep as i mentioned here https://github.com/Crozzers/screen_brightness_control/issues/7#issuecomment-767868109 the -d flag does not work with the duplicate entry only. My initial message in issue 7 and my first reply go into the duplicate entry in detail

As for running with -l and --verbose this is the output

Display 0:
        Name: Asus AUO82ED
        Model: AUO82ED
        Manufacturer: Asus
        Manufacturer ID: AUO
        Serial: 4&14e39c04&0&UID265988_0
        Method: WMI
        EDID:
                HERE IT SHOWS THE EDID String i removed cuz i don't really now if i should not share it or not :D
Display 1:
        Name: Asus 82ED
        Model: 82ED
        Manufacturer: Asus
        Manufacturer ID: AUO
        Serial: 4&14e39c04&0&UID265988
        Method: VCP
        EDID: None
Display 2:
        Name: Samsung 0D2C
        Model: 0D2C
        Manufacturer: Samsung
        Manufacturer ID: SAM
        Serial: 5&6aabf8d&0&UID4354
        Method: VCP
        EDID: None

Looking at the output i think every thing is clear now The first entry is for the integrated monitor with wmi Second one is for the integrated but with vcp I think the problem is very clear now :D

Crozzers commented 3 years ago

It's interesting that none of them have a real serial number. They all use the windows UID. I don't think it's an issue but it certainly is curious. So this looks like a simple fix. Simply get rid of the '_0' on the end of all WMI serial numbers and we should be good

Crozzers commented 3 years ago

I tested commit 51eb40d on a laptop and there are no more duplicate entries when using the -l flag. The bad news is that s.get_brightness does return duplicates. I'll work on this but it seems like progress

Crozzers commented 3 years ago

After testing on my machine the duplicate entries have been resolved and the display kwarg works on the top-level (haven't tested lower level)

Crozzers commented 3 years ago

I have now tested and fixed the display kwarg on the low-level functions with commit 5458750. It should all work fine now. If you could test this that would be marvelous.

In issue #7 I think I said I was unable to replicate the duplicate display entries appearing and I do not remember having that issue before (I'm sure I would have noticed and fixed it). Well, I re-installed versions 0.5.1, 0.6.2 and 0.7.0 to test this (the usage of win32api started with v0.5.0) and the bug appeared. I saw duplicate entries in versions 0.5.1 and 0.6.2 but not in 0.7.0. As I said, I don't think these were present before and the only thing that has changed recently about my laptop is Windows was updated. While it is entirely possible that I completely missed that bug (as I seem to do often) it is also entirely possible that Microsoft changed something in their API. In any case, I will write some more unit tests sometime in the future to avoid this kind of event being an issue

shadoWalker89 commented 3 years ago

Hi, I can confirm that the latest changes on the dev branch have fixed the duplication of the integrated monitor without affecting the usage of the -d flag

This is the output of the list

image

Everything is ok now.

Crozzers commented 3 years ago

without affecting the usage of the -d flag

As in, without breaking it or without fixing it?

shadoWalker89 commented 3 years ago

Yeah without breaking

Crozzers commented 3 years ago

Wonderful. I'll release it then.

Thanks for helping to get to the bottom of this