-
-
Notifications
You must be signed in to change notification settings - Fork 368
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
Fix incorrect limits for input and config voltages in APC report descriptor #1290
Fix incorrect limits for input and config voltages in APC report descriptor #1290
Conversation
…riptor The Back-UPS XS 1400U has been observed to report input/config voltage limits that are appropriate for the North American 120V region even though the unit is operating in the European or other 220+V region. This change diagnoses the the problem by checking if the logical maximum values for UPS.Input.Voltage and UPS.Input.ConfigVoltage are consistent with the UPS.Input.HighVoltageTransfer and if not, increases them. A similar problem was reported for CPS units in the EU region. - introduces #defines for all standard usages in power system and battery device pages. - moves cps-hid.c FindReport() to hidparser.c as FindObject_with_ID_Node(). - updates cps-hid.c to account for new defines/function name - adds apc_fix_report_desc() to implement change for APC UPS units
Looks wonderful, thanks, hope CI likes it too :) In the original issue, I think you mentioned where the "magical numbers" (and names) for macros came from; I think it makes sense to add that know-how to comments in the header, or into some docs/*usb*.txt and reference that from the header - if someone ever has to repeat this ordeal. |
Yeah, I can do that. I was glad that I didn't have to extract the names and values from the USB-IF standards documents because someone else had already done it (though i did check it all by eye). Not for another 8+h though, since I'm in -0800 (PST) and it's too late. |
Great, and thanks again!
…On Tue, Feb 15, 2022, 09:18 Nick Briggs ***@***.***> wrote:
Yeah, I can do that. I was glad that I didn't have to extract the names
and values from the USB-IF standards documents because someone else had
already done it (though i did check it all by eye). Not for another 8+h
though, since I'm in -0800 (PST) and it's too late.
—
Reply to this email directly, view it on GitHub
<#1290 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAMPTFDMRDCY5RSZ6LE6QOTU3ID47ANCNFSM5ONSUUBA>
.
Triage notifications on the go with GitHub Mobile for iOS
<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
or Android
<https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
You are receiving this because you commented.Message ID:
***@***.***>
|
I've updated the documentation in this PR (review from domain experts would be welcome), so CC @flashydave too :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Your documentation (and indentation) changes all look good to me. Thanks.
Finally, Jenkins slothed through this build job. |
1 similar comment
Finally, Jenkins slothed through this build job. |
Closes #1189 in which some APC devices operating in regions with higher (> 120V) nominal input voltages still report North American range limits for the input and configuration voltages and thus read out incorrect values.
Diagnoses the problem by checking if the logical maximum values for UPS.Input.Voltage and UPS.Input.ConfigVoltage in the report descriptor are consistent with the UPS.Input.HighVoltageTransfer logical maximum (which is observed to be correct) and if not, resets the logical maximum values to be consistent with the higher range input.
Defines are added to
hidtypes.h
for all standardized usages in the power device and battery system pages.Existing code for a similar problem with CPS devices is made consistent with this (APC) code and common code is refactored and moved to
hidparser.c