Closed denilsonsa closed 1 year ago
digitaltrails
commented
1 year ago Easy - just enable VCP code 14 - see below:
They're not all available by default, because some monitors have very dodgy implementations - so I want folk to read the documentation before enabling some of them.
denilsonsa
commented
1 year ago I tried that. It crashes.
ERROR:
Traceback (most recent call last):
File "/usr/bin/vdu_controls", line 2861, in initialise_control_panels
controller = VduController(vdu_id, vdu_model_name, vdu_serial, manufacturer, main_config,
File "/usr/bin/vdu_controls", line 1600, in __init__
self.config.restrict_to_actual_capabilities(self.capabilities)
File "/usr/bin/vdu_controls", line 1333, in restrict_to_actual_capabilities
and VDU_SUPPORTED_CONTROLS.by_arg_name[option].vcp_code not in vdu_capabilities:
KeyError: 'unsupported-14'
During handling of the above exception, another exception occurred:
Traceback (most recent call last):
File "/usr/bin/vdu_controls", line 5195, in <module>
main()
File "/usr/bin/vdu_controls", line 5025, in main
main_window = MainWindow(main_config, app, session_startup=is_logging_in())
File "/usr/bin/vdu_controls", line 4484, in __init__
create_main_control_panel()
File "/usr/bin/vdu_controls", line 4478, in create_main_control_panel
self.main_control_panel.initialise_control_panels(self.app_context_menu, main_config, session_startup)
File "/usr/bin/vdu_controls", line 2876, in initialise_control_panels
controller = VduController(vdu_id, vdu_model_name, vdu_serial, manufacturer, main_config,
File "/usr/bin/vdu_controls", line 1600, in __init__
self.config.restrict_to_actual_capabilities(self.capabilities)
File "/usr/bin/vdu_controls", line 1333, in restrict_to_actual_capabilities
and VDU_SUPPORTED_CONTROLS.by_arg_name[option].vcp_code not in vdu_capabilities:
KeyError: 'unsupported-14'It works for me. It's my HP ZR24w monitor that supports code 14. So I go to the HP-ZR24w tab (not the vdu_controls tab) and add 14 to it's enable vcp codes. Having saved that, it works.
Can you confirm you're enabling it for only the one monitor that supports the code and not all of them?
I should add, that I will look into not having it fall over, it should error gracefully - perhaps just ignore an enabled code if a monitor does not support it.
denilsonsa
commented
1 year ago I have two identical displays. :)
Anyway, setting that either in the global configuration or in the per-display configuration have the same issue: crash.
I can confirm that using ddcui is able to switch the color preset of the displays, which means the display has support for that. But the ddcui UI is more debug-friendly and less user-friendly.
digitaltrails
commented
1 year ago Perhaps attach the config files for the two monitors. The error could happen if the capability output from ddcutil doesn't include VCP 14, or doesn't include it in a way that vdu_control recognises. In which case, thes answer would be to edit the config and put in a reasonable looking entry for 14. Your included example looked fine though.
If I have the config files I can use them with a fake-ddcutil shell-script which fakes monitors, I'll see if I can duplicate the problem. Censor the included serial numbers if that is a concern.
It's still not good that vdu_controls falls over - but that's a side issue to it not working.
denilsonsa
commented
1 year ago If I have the config files I can use them with a fake-ddcutil shell-script which fakes monitors, I'll see if I can duplicate the problem. Censor the included serial numbers if that is a concern.
Do you need anything from me, or is the output that I copy-pasted into the first comment enough? (You have to click on the arrow to expand it.)
digitaltrails
commented
1 year ago I found a bug in the capabilities validation for codes from "enable vcp codes". I suspect no one has been using this capability. It is now possible to enable code 14 in the vdu_controls settings or individually for each monitor.
If you're comfortable with downloading the github vdu_controls.py, you can use that until I release 1.8.4 (which I will do in a few days, after looking more into these other issues).
Some background in case enable code 14 creates issues. I could easily promote code 14 to being supported (enabled via a checkbox). The reason I haven't is that I have an LG monitor that reports via DDC that code 14 is supported. It reports it has three values. But the actual monitor lacks any controls for code 14, which is a bit suspicious. If I use vdu_controls to change the code 14 to one of the three values, only one of them works. There is no way to unset the one that works except by resetting the monitor. So I'm not sure it's a code that should be available out of the box.
So mind how you go, if the code 14 values correspond to the ones on the monitor's actual controls, I suspect it will work just fine.
denilsonsa
commented
1 year ago Using the latest version from the repository, it doesn't crash anymore. It is correctly displaying the color preset drop-down. Thanks!
Today I tested it on a different laptop, with a different set of displays, and I got the following result when trying to change the preset:
Not a number: "08"Invalid VCP value: 08 Invalid VCP value: 08 INFO: Retry set DELL_P2419H:BLBL3Q2 vcp_code 14 to 08. Not a number: "08"Invalid VCP value: 08 Invalid VCP value: 08
Meanwhile, using ddcui I could change the color preset without any problem.
EDIT: For this first issue, could it be that 08 is read as octal number? Sometimes the leading zero is understood as octal prefix, which would make 08 invalid. I don't know, this is just a guess.
EDIT 2: I think my guess is correct… https://github.com/rockowitz/ddcutil/blob/2.0.0-dev/src/cmdline/cmd_parser_aux.c#L253-L258 Fixing this could potentially fix a bunch of other stuff.
In fact, the OSD of this display has more than three color presets, and I figured out the additional presets actually change the 0xDC "Display Mode" feature.
However, when trying to add 14, dc to "enable vcp codes", it doesn't work. (it shows many error messages) Only "color preset" shows up in vdu_controls. Again, ddcui seems to work fine.
EDIT: for this second issue, the reason is because vdu_controls doesn't like the lower-case dc value. If I replace it with DC, then the new drop-down shows up.
The solution should be simple enough: in get_all_enabled_vcp_codes(), change the line inside the loop to code = vcp_code.strip().upper().
denilsonsa
commented
1 year ago a edited 6 hours ago EDIT: For this first issue, could it be that 08 is read as octal number? Sometimes the leading zero is understood as octal prefix, which would make 08 invalid. I don't know, this is just a guess.
EDIT 2: I think my guess is correct… https://github.com/rockowitz/ddcutil/blob/2.0.0-dev/src/cmdline/cmd_parser_aux.c#L253-L258 Fixing this could potentially fix a bunch of other stuff.
I've looked at the C source-code, and this is indeed the case.
app_set_vcp_value() function will sometimes use hhs_to_byte_array() (which seems to only accept hexadecimal strings, without prefixes) and other times use parse_vcp_value(), which calls str_to_int() with base=0 which calls strtol() with base=0, which considers 0 as a prefix for octal numbers. I still don't understand what's DDCA_TABLE.
TL;DR: setvcp parameter should either be decimal without any leading zeros, or hexadecimal with 0x prefix.
EDIT: Accidentally edited this - digitaltrails - have reversed the edit.
digitaltrails
commented
1 year ago Using the latest version from the repository, it doesn't crash anymore. It is correctly displaying the color preset drop-down. Thanks!
Today I tested it on a different laptop, with a different set of displays, and I got the following result when trying to change the preset:
Not a number: "08"Invalid VCP value: 08 Invalid VCP value: 08 INFO: Retry set DELL_P2419H:BLBL3Q2 vcp_code 14 to 08. Not a number: "08"Invalid VCP value: 08 Invalid VCP value: 08
Meanwhile, using ddcui I could change the color preset without any problem.
It's interesting that VCP 14 does seem to trigger special handling and send 0x08 (for my HP monitor):
DEBUG: subprocess run - ddcutil ('--brief', '--display', '1', 'getvcp', '14')
DEBUG: subprocess result - CompletedProcess(args=['ddcutil', '--sleep-multiplier', '0.51', '--force', '--brief', '--display', '1', 'getvcp', '14'], returncode=0, stdout=b'VCP 14 CNC x00 x0b x00 x0b\n')
DEBUG: subprocess run - ddcutil ('--display', '1', 'setvcp', '14', 'x08')
DEBUG: subprocess result - CompletedProcess(args=['ddcutil', '--sleep-multiplier', '0.7', '--force', '--display', '1', 'setvcp', '14', 'x08'], returncode=0, stdout=b'')So why is it different for your DELL monitor? Could it be the capabilities text is different? Note that in the above the value returned is CNC (complex non-continuous, two bytes). I think this triggers output in 0x format for 08:
elif type_indicator == COMPLEX_NON_CONTINUOUS_TYPE:
cnc_match = cnc_pattern.match(value_match.group(2))
if cnc_match is not None:
return '{:02x}'.format(int(cnc_match.group(3), 16) << 8 | int(cnc_match.group(4), 16)), '0'If you enable debugging (in settings), you will be able to see what is being sent/received. I'm not sure, but from what I read in the past in the ddcutil docs, I think Rockowitz might program around some monitor quirks. I treat all monitors the same, perhaps that might explain the difference.
You can see why I'm reluctant to support codes beyond the basics.
One thing to note in the above example, in the capabilities output from ddcutil, I'm not passed the specific type info. So vdu_controls does not know the type of a VCP code until it retrieves a value, at which point it picks the type out of b'VCP 14 CNC x00 x0b x00 x0b\n'. The capabilities output from ddcutil (including in each monitors settings), does not include whether a value is simple (one byte int value), simple-non-continuous (one byte, set of values), or complex-non-continuous (two bytes, set of values). So it may also be that CNC isn't being returned by getvcp - hopefully not, because there is no workaround for that.
denilsonsa
commented
1 year ago As seen here, that Dell monitor returns:
VCP 0C C 1 255
VCP 10 C 66 100
VCP 12 C 75 100
VCP 14 SNC X05
VCP 62 C 5 255
VCP 8D SNC X05
VCP 91 ERR
VCP CC SNC X02
VCP DC SNC X00Specifically: VCP 14 SNC X05
The capabilities text is in another comment.
For my AOC monitor, I get:
VCP 0C ERR
VCP 10 C 0 100
VCP 12 C 25 100
VCP 14 SNC x05
VCP 62 C 100 100
VCP 8D ERR
VCP 91 ERR
VCP CC SNC x05
VCP DC ERRWhich is the same output for that code: VCP 14 SNC x05.
denilsonsa
commented
1 year ago For completeness sake, here's some data while trying it on my AOC monitor:
05: 6500 K works fine.06: 7500 K works fine.08: 9300 K fails, as described a few comments above.0b: User 1 fails due to the same reason.01: sRGB fails but works. Let me explain:DEBUG: subprocess run - ddcutil ('--brief', '--display', '2', 'getvcp', '14') DEBUG: subprocess result - CompletedProcess(args=['ddcutil', '--sleep-multiplier', '0.25', '--brief', '--display', '2', 'getvcp', '14'], returncode=0, stdout=b'VCP 14 SNC x05\n') DEBUG: subprocess run - ddcutil ('--display', '2', 'setvcp', '14', '01') Setting value failed for feature 14, rc=DDCRC_NULL_RESPONSE(-3002): received DDC null response
It complains that setting it failed. However, I can see that my monitor changed the color preset. Okay, I try clicking "Retry":
INFO: Retry set Q27P1B:GNXKCHA123704 vcp_code 14 to 01. DEBUG: subprocess run - ddcutil ('--brief', '--display', '2', 'getvcp', '14') DEBUG: subprocess result - CompletedProcess(args=['ddcutil', '--sleep-multiplier', '0.25', '--brief', '--display', '2', 'getvcp', '14'], returncode=0, stdout=b'VCP 14 SNC x01\n')
Now there are no complaints. Okay, seems to be working. Let me try setting back to the other value:
DEBUG: subprocess run - ddcutil ('--brief', '--display', '2', 'getvcp', '14') DEBUG: subprocess result - CompletedProcess(args=['ddcutil', '--sleep-multiplier', '0.25', '--brief', '--display', '2', 'getvcp', '14'], returncode=0, stdout=b'VCP 14 SNC x01\n') DEBUG: subprocess run - ddcutil ('--display', '2', 'setvcp', '14', '05') DEBUG: subprocess result - CompletedProcess(args=['ddcutil', '--sleep-multiplier', '0.25', '--display', '2', 'setvcp', '14', '05'], returncode=0, stdout=b'')
Also works. This seems to be a quirk with this monitor, because ddcui also complains with the same error, despite the color preset clearly changing in front of me.
https://user-images.githubusercontent.com/121676/212217627-0a1034a8-c56e-45f9-9290-617024b727a9.mp4
As a side node: the OSD language is listing Italian, and the drop-down doesn't have any other option than that; but it's clearly wrong.
denilsonsa
commented
1 year ago (looks like you accidentally edited my comment, look at the edit history of it) :laughing:
digitaltrails
commented
1 year ago Inconsistent responses can sometimes be due to the monitor being slow to respond, it may be worth upping the sleep multiplier. I see you used 0.25, I'm surprised that works that well. I've had to move away from 0.5 toward 1.0. DDcutil also supports a bunch of other timing related parameters that might be worth exploring.
I don't think the manufacturers of these monitors have much of an incentive to properly implement DCC. Especially the rare codes. I imagine they only intended many of the codes to be used by their own custom Windows software that they would provide on CDROM, or they are just playing lip service to standards. Some cheap monitors, such as my noname aliexpress usb-powered LCD, do not support DDC at all - apparently these cheap monitors may just use LCD-TV firmware.
I also suspect they manufacturers are using the same code in monitors with different features, so ddcutil may be reporting features that a monitor does not actually support. For example, my LG monitor reports a Display Port input that doesn't exist - I bet more expensive models have a second port.
At least one person has bricked a monitor trying undocumented vcp codes using ddcutil. I don't think anyone has done any damage using the documented calls. I believe Samsung was able to remotely brick a batch of stolen TV's, so there may well be codes that support that kind of functionality.
Fear of consequences is this is primarily why vdu_controls first calls getvcp to see if a value actually needs to change and then setvcp only if necessary. Perhaps this is over cautious.
Sorry about the edit (I put back the original text).
digitaltrails
commented
1 year ago As seen here, that Dell monitor returns:
VCP 0C C 1 255 VCP 10 C 66 100 VCP 12 C 75 100 VCP 14 SNC X05 VCP 62 C 5 255 VCP 8D SNC X05 VCP 91 ERR VCP CC SNC X02 VCP DC SNC X00Specifically:
VCP 14 SNC X05The capabilities text is in another comment.
For my AOC monitor, I get:
VCP 0C ERR VCP 10 C 0 100 VCP 12 C 25 100 VCP 14 SNC x05 VCP 62 C 100 100 VCP 8D ERR VCP 91 ERR VCP CC SNC x05 VCP DC ERRWhich is the same output for that code:
VCP 14 SNC x05.
It's interesting that your monitors return an SNC for VCP 14, where as my HP returns a CNC. I thought it was weird that DDC needed two bytes for this value. I guess this proves that a capability isn't restricted to a specific type.
Vdu_controls is coded to pass CNC with an leading x and SNC with a leading zero (but I've only seen values less the 0F actually being passed, not sure what happens to 10). That was what ddcutil seemed to expect.
denilsonsa
commented
1 year ago Vdu_controls is coded to pass CNC with an leading x and SNC with a leading zero (but I've only seen values less the 0F actually being passed, not sure what happens to 10). That was what ddcutil seemed to expect.
Leading x or leading 0x? Because I would assume (by what I saw in the ddcutil code) that x?? wouldn't work, that it would expect 0x??. But I may be completely wrong.
Also leading 0 causes issues for anything above 07. Look:
#include <stdio.h>
#include <stdlib.h>
int main(int argc, char *argv[]) {
long i, v;
char s[5];
for (i=0; i<17; i++) {
sprintf(s, "%02lX", i);
v = strtol(s, NULL, 0);
printf("%2ld (int) -> %2s (str) -> %2ld (int)\n", i, s, v);
}
return 0;
} 0 (int) -> 00 (str) -> 0 (int)
1 (int) -> 01 (str) -> 1 (int)
2 (int) -> 02 (str) -> 2 (int)
3 (int) -> 03 (str) -> 3 (int)
4 (int) -> 04 (str) -> 4 (int)
5 (int) -> 05 (str) -> 5 (int)
6 (int) -> 06 (str) -> 6 (int)
7 (int) -> 07 (str) -> 7 (int)
8 (int) -> 08 (str) -> 0 (int)
9 (int) -> 09 (str) -> 0 (int)
10 (int) -> 0A (str) -> 0 (int)
11 (int) -> 0B (str) -> 0 (int)
12 (int) -> 0C (str) -> 0 (int)
13 (int) -> 0D (str) -> 0 (int)
14 (int) -> 0E (str) -> 0 (int)
15 (int) -> 0F (str) -> 0 (int)
16 (int) -> 10 (str) -> 10 (int)Vdu_controls is coded to pass CNC with an leading x and SNC with a leading zero (but I've only seen values less the 0F actually being passed, not sure what happens to 10). That was what ddcutil seemed to expect.
Leading
xor leading0x? Because I would assume (by what I saw in the ddcutil code) thatx??wouldn't work, that it would expect0x??. But I may be completely wrong.Also leading
0causes issues for anything above07. Look:#include <stdio.h> #include <stdlib.h> int main(int argc, char *argv[]) { long i, v; char s[5]; for (i=0; i<17; i++) { sprintf(s, "%02lX", i); v = strtol(s, NULL, 0); printf("%2ld (int) -> %2s (str) -> %2ld (int)\n", i, s, v); } return 0; }0 (int) -> 00 (str) -> 0 (int) 1 (int) -> 01 (str) -> 1 (int) 2 (int) -> 02 (str) -> 2 (int) 3 (int) -> 03 (str) -> 3 (int) 4 (int) -> 04 (str) -> 4 (int) 5 (int) -> 05 (str) -> 5 (int) 6 (int) -> 06 (str) -> 6 (int) 7 (int) -> 07 (str) -> 7 (int) 8 (int) -> 08 (str) -> 0 (int) 9 (int) -> 09 (str) -> 0 (int) 10 (int) -> 0A (str) -> 0 (int) 11 (int) -> 0B (str) -> 0 (int) 12 (int) -> 0C (str) -> 0 (int) 13 (int) -> 0D (str) -> 0 (int) 14 (int) -> 0E (str) -> 0 (int) 15 (int) -> 0F (str) -> 0 (int) 16 (int) -> 10 (str) -> 10 (int)
Yes but passing thing like 06 to getvcp 05 works and getvcp 5 does not. On the other had setvcp CC 3 and setvcp CC 03 both seem to work. So I probably improvised a bunch of formats in the past, the code would have to be checked to see what I actually pass.
denilsonsa
commented
1 year ago Yeah, ddcutil code is very inconsistent. Maybe we should ask them to make it more consistent. If they are going to release version 2.0, this is a good moment for a breaking change.
From what I read from the C source-code:
I believe it should be consistent, it should always use the same rule for any integer parameter, as that will be more predictable.
digitaltrails
commented
1 year ago I can't remember how I came to the conclusion, but my code assumes the x prefix is used for non-continuous 2-byte values (CNC):
def set_attribute(self, vdu_id: str, vcp_code: str, new_value: str, sleep_multiplier: float = None) -> None:
"""Send a new value to a specific VDU and vcp_code."""
current, _ = self.get_attribute(vdu_id, vcp_code, sleep_multiplier=sleep_multiplier)
if new_value != current:
if self.get_type(vcp_code) == COMPLEX_NON_CONTINUOUS_TYPE:
new_value = 'x' + new_value
self.__run__('--display', vdu_id, 'setvcp', vcp_code, new_value, sleep_multiplier=sleep_multiplier)Rather than breaking everybody's existing scripts, I would probably ask for a option to enable more consistent output. Similar to the existing --terse, something like --hex or maybe --format N in case further changes are needed).
digitaltrails
commented
1 year ago I'm not sure whether any further needs to change in vdu_controls is respect to this issue. I think not, please reopen otherwise. What was done:
Some displays may have a color preset (such as mine). It can be worth having it as a drop-down widget.
full capabilities
``` Model: Q27P1 MCCS version: 2.1 Commands: Op Code: 01 (VCP Request) Op Code: 02 (VCP Response) Op Code: 03 (VCP Set) Op Code: 07 (Timing Request) Op Code: 0C (Save Settings) Op Code: F3 (Capabilities Request) VCP Features: Feature: 02 (New control value) Feature: 04 (Restore factory defaults) Feature: 05 (Restore factory brightness/contrast defaults) Feature: 08 (Restore color defaults) Feature: 10 (Brightness) Feature: 12 (Contrast) Feature: 14 (Select color preset) Values: 01: sRGB 05: 6500 K 06: 7500 K 08: 9300 K 0b: User 1 Feature: 16 (Video gain: Red) Feature: 18 (Video gain: Green) Feature: 1A (Video gain: Blue) Feature: 60 (Input Source) Values: 01: VGA-1 03: DVI-1 Feature: 62 (Audio speaker volume) Feature: 6C (Video black level: Red) Feature: 6E (Video black level: Green) Feature: 70 (Video black level: Blue) Feature: C8 (Display controller type) Feature: C9 (Display firmware level) Feature: B0 (Settings) Feature: B6 (Display technology type) Feature: D6 (Power mode) Feature: DF (VCP Version) Feature: F8 (Manufacturer specific feature) ```