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

Read maximum VCP value #11

Merged
merged 3 commits into from
Aug 8, 2020
Merged

Read maximum VCP value #11

merged 3 commits into from
Aug 8, 2020

Conversation

xanderfrangos
Copy link
Contributor

Hello again!

As it turns out, not all displays max out their brightness/contrast at 100. Because of this, there needs to be a way to read the max value for a given VCP code and not restrict brightness/contrast to 0-100. This PR takes care of that by updating getVCP, along with adding getMaxBrightness and getMaxContrast.

GetVCPFeatureAndVCPFeatureReply already returns both the current and maximum values, so I've updated getVCP to return both in an array. The array is in the format [currentValue, maxValue]. Requesting them separately doubles execution time (ex. 250ms -> 500ms). However, I've split them up in the bindings for simplicity and backwards compatibility. Developers can use _getVCP directly to get both at once for better performance.

I've updated README.md to reflect these changes.

@hensm
Copy link
Owner

hensm commented Aug 8, 2020

Hey.

I don't mind breaking the API if it makes sense, especially since <1.x is unstable. There's also mention of a minimum value with functions like GetMonitorBrightness/GetMonitorContrast, but I'm not finding a reference to that elsewhere, even in the DDC/CI spec. Still, I'd like to think it's there for a reason.

Maybe something like this as a return from getBrightness/getContrast?

{
    "value": 50
  , "minValue": 0
  , "maxValue": 100
}

@xanderfrangos
Copy link
Contributor Author

From what I understand, 0 is the implied minimum for all "continuous" VCP codes (like brightness/contract) as there's nothing in the spec for reporting minimums. That doesn't mean it's correct. My primary monitor has a minimum VCP contrast of 53, but DDC/CI doesn't communicate that.

The functions you linked to could be getting monitor capabilities from non-DDC/CI sources (ex. special drivers, USB), in which case I could see them potentially reporting non-zero minimums. But as long as this library only uses DDC/CI, I don't see how minimums can be correctly provided. It'd just be passing back a hard-coded 0 for minValue every time. It doesn't hurt to provide an object like you're suggesting, but it may not be correct.

@hensm
Copy link
Owner

hensm commented Aug 8, 2020

Got it, that makes sense. Happy to merge this as is.

@hensm hensm merged commit 9f32020 into hensm:master Aug 8, 2020
@hensm
Copy link
Owner

hensm commented Aug 8, 2020

Published v0.1.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants