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

Monitor randomly responds with 0 to get_*()-command and/or fails set_*()-command #288

Closed
XDA-Bam opened this issue Oct 4, 2023 · 10 comments · Fixed by #291
Closed

Monitor randomly responds with 0 to get_*()-command and/or fails set_*()-command #288

XDA-Bam opened this issue Oct 4, 2023 · 10 comments · Fixed by #291
Labels
bug Something isn't working

Comments

@XDA-Bam
Copy link

XDA-Bam commented Oct 4, 2023

  • Monitor manufacturer and model number:
    HP Omen 27u
  • Input source (HDMI, VGA, display port, ect.):
    DP (different cables and both DP ports on the GPU tested)
  • Output device (video card, discrete graphics, ect.):
    Discrete Radeon 6800 (at least two different driver versions tested)
  • Operating system:
    Win 10 Pro 64 bit
  • Python version:
    3.12.0 (also tested 3.11.3)
  • monitorcontrol version (monitorcontrol --version):
    3.0.3

Steps to Reproduce

Create a very basic Python script, which asks for monitor brightness and contrast and then tries to set the reported value for brightness (contrast ignored for now):

import time
from monitorcontrol import get_monitors

# Check monitors
while True:
    for monitor in get_monitors():
        with monitor:
            brightness = monitor.get_luminance()
            contrast   = monitor.get_contrast()
            print(f'Brightness: {brightness}') 
            print(f'Contrast:   {contrast}')

    time.sleep(1)

    # Set monitor backlight using monitorcontrol
    for monitor in get_monitors():
        with monitor:
            monitor.set_luminance(brightness)
            #monitor.set_contrast(contrast)
        
    time.sleep(1)

The script works erratically. Almost always, the first reported contrast value is 0, while the first reported brightness is correct. Sometimes, brightness is reported as 0 later on. If that is the case, the screen brightness is accordingly set to 0 in the following step, in case monitor.set_luminance() does not crash. If the script does crash, it is always at monitor.set_luminance() with the reported error that whatever brightness was defined is larger than the maximum allowed value of 0 (?).

Crashes occur with roughly 20% probability. A typical run of the script looks like this:

Python 3.12.0 (tags/v3.12.0:0fb18b0, Oct  2 2023, 13:03:39) [MSC v.1935 64 bit (AMD64)] on win32
Type "help", "copyright", "credits" or "license()" for more information.

= RESTART: C:\Users\MyName\Downloads\Monitor brightness\Test_Set brightness_monitorcontrol.py
Brightness: 16
Contrast:   0
Brightness: 16
Contrast:   80
Brightness: 16
Contrast:   80
Traceback (most recent call last):
  File "C:\Users\MyName\Downloads\Monitor brightness\Test_Set brightness_monitorcontrol.py", line 22, in <module>
    monitor.set_luminance(brightness)
  File "C:\Users\MyName\AppData\Local\Programs\Python\Python312\Lib\site-packages\monitorcontrol\monitorcontrol.py", line 254, in set_luminance
    self._set_vcp_feature(code, value)
  File "C:\Users\MyName\AppData\Local\Programs\Python\Python312\Lib\site-packages\monitorcontrol\monitorcontrol.py", line 149, in _set_vcp_feature
    raise ValueError(
ValueError: value of 16 exceeds code maximum of 0
@newAM newAM added the bug Something isn't working label Oct 5, 2023
@newAM
Copy link
Owner

newAM commented Oct 5, 2023

This looks a lot like what happens if the VCP timeout is set too low in Linux, but Windows does not expose this control.

I booted into Windows 10 and tried this on my own system (Acer XF270HU, 4090, displayport) and was unable to reproduce it after running the script for 10 minutes.

You could try it on Linux to see if it responds differently. This would narrow it down to a hardware or software problem, but booting into Linux is a pain to do if you're not already dual booting. I sadly don't have a lot of experience with debugging Windows APIs and I'm not sure if there are any tools available to see the data on the bus.

@XDA-Bam
Copy link
Author

XDA-Bam commented Oct 5, 2023

Thanks for looking into this. I assume I could boot a live distro and set up Python, but in the end, we still couldn't fix it even if it was the timeout. I have to admit, I'm not overly motivated to go down that route :/

Essentially, my main problem is not that monitorcontrol crashes during set-commands - I could try/catch that and repeat until the command succeeds. That would be "dirty", but this is a small private project and it just has to work. My main problem is, that monitor.get_luminance() and monitor.get_contrast() report no errors and return a seemingly valid value of 0 if they fail. I can ignore the contrast, but I can't rule out setting my brightness to zero and therefore have no chance of catching that error and getting stuff to work correctly.

Could you somehow check, why there is no error reported in case monitor.get_luminance() fails? Does Windows drop the ball here, or is this maybe something in monitorcontrol, where we can fix it?

Let me know if I can help with any more logs or stuff.

@newAM
Copy link
Owner

newAM commented Oct 6, 2023

Could you somehow check, why there is no error reported in case monitor.get_luminance() fails? Does Windows drop the ball here, or is this maybe something in monitorcontrol, where we can fix it?

There is a CRC in Linux, I double checked the Windows code, and sure enough, I misunderstood the python interface to the Windows APIs. I thought the return status bools automatically got turned into WindowsErrors, but that is not the case!

I made a branch windows-oops, I tried to wrangle my monitors to get a negative status but was unsuccessful, are you able to test that and let me know what happens on your setup? With poetry it should just be poetry install then poetry run python path_to_your_script.py.

@XDA-Bam
Copy link
Author

XDA-Bam commented Oct 6, 2023

Yep, definitely different. No 0 reported back anymore and "correctly" crashes instead:

Traceback (most recent call last):
  File "C:\Users\MyName\AppData\Local\Programs\Python\Python312\Lib\site-packages\monitorcontrol\vcp\vcp_windows.py", line 138, in get_vcp_feature
    raise VCPError(
monitorcontrol.vcp.vcp_abc.VCPError: failed to get VCP feature: An operation failed because a DDC/CI message had an invalid value in its command field.

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "C:\Users\MyName\Downloads\Monitor brightness\Test_Set brightness_monitorcontrol.py", line 8, in <module>
    brightness = monitor.get_luminance()
  File "C:\Users\MyName\AppData\Local\Programs\Python\Python312\Lib\site-packages\monitorcontrol\monitorcontrol.py", line 248, in get_luminance
    return self._get_vcp_feature(code)
  File "C:\Users\MyName\AppData\Local\Programs\Python\Python312\Lib\site-packages\monitorcontrol\monitorcontrol.py", line 192, in _get_vcp_feature
    current, maximum = self.vcp.get_vcp_feature(code.value)
  File "C:\Users\MyName\AppData\Local\Programs\Python\Python312\Lib\site-packages\monitorcontrol\vcp\vcp_windows.py", line 141, in get_vcp_feature
    except ctypes.WinError as e:
TypeError: catching classes that do not inherit from BaseException is not allowed

This affects both, monitor.get_luminance() and monitor.get_contrast() in the same ~20% of calls as before.

@newAM
Copy link
Owner

newAM commented Oct 9, 2023

An operation failed because a DDC/CI message had an invalid value in its command field

Hmm that seems like another issue, but the only inputs to GetVCPFeatureAndVCPFeatureReply are the monitor handle and the VCP code.

TypeError: catching classes that do not inherit from BaseException is not allowed

this function is probably the worst-named thing in ctypes

I must have missed that line in the ctypes docs :(

I pushed another commit to fix that second exception. Can you try again? Also, with this fix does it now "work" for you, in that you can catch the exception and retry?

@XDA-Bam
Copy link
Author

XDA-Bam commented Oct 10, 2023

Updated the package and ran again. The error now looks like this:

Traceback (most recent call last):
  File "C:\Users\MyName\Downloads\Monitor brightness\Test_Set brightness_monitorcontrol.py", line 22, in <module>
    monitor.set_luminance(brightness)
  File "C:\Users\MyName\AppData\Local\Programs\Python\Python312\Lib\site-packages\monitorcontrol\monitorcontrol.py", line 271, in set_luminance
    self._set_vcp_feature(code, value)
  File "C:\Users\MyName\AppData\Local\Programs\Python\Python312\Lib\site-packages\monitorcontrol\monitorcontrol.py", line 164, in _set_vcp_feature
    maximum = self._get_code_maximum(code)
  File "C:\Users\MyName\AppData\Local\Programs\Python\Python312\Lib\site-packages\monitorcontrol\monitorcontrol.py", line 141, in _get_code_maximum
    _, maximum = self.vcp.get_vcp_feature(code.value)
  File "C:\Users\MyName\AppData\Local\Programs\Python\Python312\Lib\site-packages\monitorcontrol\vcp\vcp_windows.py", line 138, in get_vcp_feature
    raise VCPError(
monitorcontrol.vcp.vcp_abc.VCPError: failed to get VCP feature: An operation failed because a DDC/CI message had an invalid value in its command field.

Using try/except works and I think I can now work around the errors. Thanks!

It looks a bit odd to me that the error still is about an "invalid value in its command field". I would intuitively expect something different in case of a timeout, but I'm also 100% not familiar with low-level windows VCP code 😄

@newAM
Copy link
Owner

newAM commented Oct 11, 2023

It looks a bit odd to me that the error still is about an "invalid value in its command field"

Yeah, it looks odd to me too 🤔
There's nothing that I can think of to tweak in that API call though.

@newAM
Copy link
Owner

newAM commented Oct 11, 2023

I made a release with the current "fix": https://github.com/newAM/monitorcontrol/releases/tag/3.1.0

Feel free to reopen the issue anytime :)

@gabortim
Copy link

@XDA-Bam I had a similar or same(?) issue. The workaround for me is to retry until it works again. The application code part looks as follows:

try:
    def _set():
        monitor.set_luminance(value)
        logger.info(f"Set brightness value {value} on monitor#{i}")

    _set()
except VCPError as e:
    logger.exception(e)
    time.sleep(5)
    _set()

@XDA-Bam
Copy link
Author

XDA-Bam commented Aug 23, 2024

@gabortim Thanks for posting. Yes, I implemented a workaround, too. Currently using retry2-package, which seemed like a clean solution to me:

from retry import retry

retry_delay = 0.25
retry_count = 10

@retry(delay=retry_delay, tries=retry_count)
def get_brightness(monitor):
    brightness = monitor.get_luminance()
    if brightness > 100 or brightness  < 0:
        raise ValueError('Reported brightness value outside of valid range [0, 100].')
    return brightness

I should probably define an additional error in case the code ever happens to fail after 10 tries, but for my tiny project, it doesn't really matter (and I never saw it reach that threshold).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants