search

digitaltrails / vdu_controls

VDU controls - a control panel for monitor brightness/contrast/...
GNU General Public License v3.0
103 stars 4 forks source link

Fix undefined values preventing tool from opening #79

Closed Extent421 closed 3 months ago

Extent421 commented 3 months ago

Just got an AW3225QF, and vdu controls complains about an input source undefined value F0F and mentions that you can override the settings in the options panel. But immediately after dismissing that it then crashes when it tries to find the undefined value in the values list. This additional check prevents the invalid list look up and allows the UI to open so that you can get into the settings and change the overrides.

If self.keys can never contain None then it would probably be cleaner to keep only the list contains check and get rid of the nonetype test, since the second test would then cover both cases.

digitaltrails commented 3 months ago

Thanks for contributing this fix, much appreciated.

I might see if I can simulate the situation. I'll try to see if I can get some certainty on whether None might wind up in the list.

Extent421 commented 3 months ago

after playing with it a bit it appears what my screen is doing is it has 3 valid input modes that are detected properly, and the 4th invalid mode. I can choose any of the 3 valid inputs and it works exactly as expected, but the screen immediately reports that' it's switched to the 4th invalid input no matter what input it's actually on. So it's not effecting the function of the tool too much, but it is constantly returning back this invalid mode that the UI doesn't have

digitaltrails commented 3 months ago

but it is constantly returning back this invalid mode that the UI doesn't have

You could add the "invalid mode" to the metadata.

Go to the Settings tab for the monitor, in the capabilities override find the definition of the feature and add an additional value, and save the tab. Just refrain from selecting that value in the GUI.

The reason it's called capabilities override is because monitors seem to quite often omit describing all their features or describe them incorrectly. The man page (or help) describes some other ways to use the override.

Extent421 commented 3 months ago

Yes I did that, it didn't really matter because switching to that value does nothing on the screen. My point is more that that is how the bug was causing the UI originally to crash. No matter what input was originally selected the unknown value was always being returned, even before being able to get to the settings tab, if that helps simulate the problem any.

digitaltrails commented 3 months ago

My point is more that that is how the bug was causing the UI originally to crash. No matter what input was originally selected the unknown value was always being returned, even before being able to get to the settings tab.

The crash is fixed (thanks for the patch). Plus, with the override, the invalid value can be accepted as valid.

The question remains: where is the invalid value coming from? If you're interested in getting to the bottom of that, there is some further work that could be done.

If I recall correctly the way I use ddcutil/libddcutil should be confirming any value it sets, but I guess setting the input might be somewhat unusual.

Does the value correct if you press the refresh, in which case perhaps the get after the set is being done too soon (maybe changing inputs happens asynchronously to the request).

Could you run ddcutil on the command line, do a setvcp, and then a getvcp. Does the getvcp get the right value.

Maybe there is something special about this feature. Could you attach the capabilities override text. And also the output from a getvcp on the feature.

Have you configured vdu_controls to use the ddcutil-service (settings->dbus client enabled) or the the ddcutil command line? Perhaps there is a bug in one of these interfaces.