[ddcutil] Invalid Display should be return by list_monitors_info("ddcutil")

[goldyfruit]

Hi,

I'm working with monitor and TV, monitor mostly supports ddc procotl but TV doesn´t.

When list_monitors_info() function is used with ddcutil, it should return both valid and invalid monitor/TV as ddcutil does.

Supports DDC protocol

$ ddcutil detect
Display 1
   I2C bus:             /dev/i2c-20
   EDID synopsis:
      Mfg id:           RTK
      Model:            
      Serial number:    demoset-1
      Manufacture year: 2019
      EDID version:     1.3
   VCP version:         2.2

>>> import screen_brightness_control as sbc
>>> sbc.list_monitors_info("ddcutil")
[{'method': <class 'screen_brightness_control.linux.DDCUtil'>, 'index': 0, 'model': None, 'serial': 'demoset-1', 'bin_serial': None, 'manufacturer': None, 'manufacturer_id': None, 'edid': '00ffffffffffff004a8b201901010101321d0103800000783eee91a3544c99260f5054230800d1c0b3009500810061404540814081c0023a801871382d40582c250058c31000001e000000fc00000a2020202020202020202020000000ff0064656d6f7365742d310a203020000000fd00383f545413000a2020202020200186', 'i2c_bus': '/dev/i2c-20', 'bus_number': 20, 'name': ''}]

Unsupported DDC protocol

$ ddcutil detect
Invalid display
   I2C bus:             /dev/i2c-20
   EDID synopsis:
      Mfg id:           SAM
      Model:            SAMSUNG
      Serial number:    
      Manufacture year: 2018
      EDID version:     1.3
   DDC communication failed

>>> import screen_brightness_control as sbc
>>> sbc.list_monitors_info("ddcutil")
[]

[Crozzers]

If ddcutil says that a display does not support DDC then it will not show up in list_monitors_info. This is by design as I don't see a reason for a screen_brightness_control library to list monitors that it knows it cannot control the brightness of.

Is there a particular reason you want the monitor to be listed?

[goldyfruit]

The reason is that DDC returns information (supported or not) only if the monitor/TV is powered on, compare to xrandr that returns a monitor/TV even if not powered on.

It could be useful as well for debugging, people may not now that the monitor/TV doesn't suppor DDC, having an argument like supported in the array returned by list_monitors_info() could be helpful.

[Crozzers]

I suppose it could indeed be useful for debugging. However I think adding a supported: True/False key in the returned monitor information would not be a great way to do it. Alot of the library is built on the assumption that all the monitors coming out of list_monitors_info are valid and supported. Implementing checks to filter unsupported monitors all over the place would not be great.

I'm thinking something like this:

import screen_brightness_control as sbc

sbc.list_monitors_info()                  # list all supported and valid monitors
sbc.list_monitors_info(unsupported=True)  # list ALL monitors, valid or not

This way, it doesn't break the existing infastructure.

The pain will be refactoring the get_display_info method of each class to possibly optionally return unsupported monitors. I'm thinking about adding a parse_monitors function to return ALL monitors and then changing get_display_info to just filter the output of parse_monitors

[goldyfruit]

I suppose it could indeed be useful for debugging. However I think adding a supported: True/False key in the returned monitor information would not be a great way to do it. Alot of the library is built on the assumption that all the monitors coming out of list_monitors_info are valid and supported. Implementing checks to filter unsupported monitors all over the place would not be great.

I'm thinking something like this:

import screen_brightness_control as sbc

sbc.list_monitors_info()                  # list all supported and valid monitors
sbc.list_monitors_info(unsupported=True)  # list ALL monitors, valid or not

This way, it doesn't break the existing infastructure.

You're right, your approach is better than mine.

The pain will be refactoring the get_display_info method of each class to possibly optionally return unsupported monitors. I'm thinking about adding a parse_monitors function to return ALL monitors and then changing get_display_info to just filter the output of parse_monitors

I feel you, sorry!

[Crozzers]

Hello. Apologies for the late update.

So in the main branch at the moment, you can list unsupported monitors for DDCUtil and XRandr using the unsupported=True kwarg in list_monitors_info. These are the only methods at the moment that can detect unsupported displays.

Its best to use the unsupported kwarg like so:

import screen_brightness_control as sbc
sbc.list_monitors_info(allow_duplicates=True, unsupported=True)

To make sure that unsupported displays aren't filtered out in favour of supported ones. I've added a section to the FAQ that states this. That will go live when I publish the release.

For DDCUtil, any display that the executable flags as invalid will show up as unsupported. For xrandr, any Wayland display will be unsupported, since xrandr does not work on wayland. I have heard of wayland ports of xrandr so perhaps support could be added in the future.

Unfortunately, these are the only two methods that will report unsupported displays. The I2C method could potentially, in the future, report unsupported displays but the only way to check if a display is unsupported is to either:

1. Fetch the VCP capabilities string for the monitor and check if brightness support is listed
   • Slow as all hell, requiring multiple I2C bus reads and writes, very error prone and sucks to implement
   • Display might not even list brightness as supported, even if it is supported (see ddcutil docs)
2. Try to get the current brightness
   • This works well but makes I2C 30% slower when attempting to get/set the brightness, although the brightness value could be cached, making only setting the brightness 30% slower

For Windows, the unsupported=True kwarg has no effect as there is not currently a way to detect unsupported displays. I will do further experimentation on this in the future.

I'll put out a release within the next week with these changes

[Crozzers]

v0.18.0 has been released with these changes

[goldyfruit]

Thanks @Crozzers !