Closed smainz closed 2 years ago
@flashydave, @jimklimov -- I have a NUT style question: I've taken the rd.conf
file from github.com:abend0c1/hidrdd.git (actually, a very slightly updated version of that for which I've submitted a PR), which incorporates the complete USB-IF usage definitions for power device and battery system pages and processed it with
grep -e ^0084 -e ^0085 rd.conf | sed 's/,.*$//;s/ *$//' | sed 's/ /_/g;s/_/ /' | tr '[:lower:]' '[:upper:]' | sed 's/\(0085.... \)/\1USAGE_BAT_/;s/\(0084.... \)/\1USAGE_POW_/;s/\([A-Z_]*\)_PAGE/PAGE_\1/' | awk '{print "#define "$2" 0x"$1}'
to produce defines for the usages that look like
#define PAGE_POWER_DEVICE 0x0084
#define USAGE_POW_UNDEFINED 0x00840000
#define USAGE_POW_I_NAME 0x00840001
#define USAGE_POW_PRESENT_STATUS 0x00840002
[...]
#define PAGE_BATTERY_SYSTEM 0x0085
#define USAGE_BAT_UNDEFINED 0x00850000
#define USAGE_BAT_SMB_BATTERY_MODE 0x00850001
#define USAGE_BAT_SMB_BATTERY_STATUS 0x00850002
#define USAGE_BAT_SMB_ALARM_WARNING 0x00850003
[...]
... since the power device and battery system pages have conflicting field names we need to include the POW_
or BAT_
to differentiate, but the real style question is: underscores in the variable part of the field names or not? -- that is USAGE_BAT_SMB_BATTERY_MODE
or USAGE_BAT_SMBBATTERYMODE
?
I'm happy either way but it's not my codebase so I'm asking...
I prefer the latter style so that the _ acts more like a heirarchical separator but I'm sure someone else could suggest an alternative rule. Its not my code either so I can't say definitively!
On 11:57, Sat 12 Feb 22, Nick Briggs wrote:
@flashydave, @jimklimov -- I have a NUT style question: I've taken the
rd.conf
file from github.com:abend0c1/hidrdd.git (actually, a very slightly updated version of that for which I've submitted a PR), which incorporates the complete USB-IF usage definitions for power device and battery system pages and processed it withgrep -e ^0084 -e ^0085 rd.conf | sed 's/,.*$//;s/ *$//' | sed 's/ /_/g;s/_/ /' | tr '[:lower:]' '[:upper:]' | sed 's/\(0085.... \)/\1USAGE_BAT_/;s/\(0084.... \)/\1USAGE_POW_/;s/\([A-Z_]*\)_PAGE/PAGE_\1/' | awk '{print "#define "$2" 0x"$1}'
to produce defines for the usages that look like
#define PAGE_POWER_DEVICE 0x0084 #define USAGE_POW_UNDEFINED 0x00840000 #define USAGE_POW_I_NAME 0x00840001 #define USAGE_POW_PRESENT_STATUS 0x00840002 [...] #define PAGE_BATTERY_SYSTEM 0x0085 #define USAGE_BAT_UNDEFINED 0x00850000 #define USAGE_BAT_SMB_BATTERY_MODE 0x00850001 #define USAGE_BAT_SMB_BATTERY_STATUS 0x00850002 #define USAGE_BAT_SMB_ALARM_WARNING 0x00850003 [...]
... since the power device and battery system pages have conflicting field names we need to include the
POW_
orBAT_
to differentiate, but the real style question is: underscores in the variable part of the field names or not? -- that isUSAGE_BAT_SMB_BATTERY_MODE
orUSAGE_BAT_SMBBATTERYMODE
?I'm happy either way but it's not my codebase so I'm asking...
-- Reply to this email directly or view it on GitHub: https://github.com/networkupstools/nut/issues/1189#issuecomment-1037433126 You are receiving this because you were mentioned.
Message ID: @.***>
I like underscores as more readable, especially where we can't have CamelCase etc. Maybe follow precedent of nut-ddl structured names with two underscores between tokens? like USAGEBATSMB_BATTERY_STATUS ?
Thats starting to get messy. Not only does it become irksome to type (especially for those using command line type editors) and reduces the amount of code that can seen on any guven size of screen. For example my recent contributions were all developed using ssh from my phone!
On 02:51, Sun 13 Feb 22, Jim Klimov wrote:
I like underscores as more readable, especially where we can't have CamelCase etc. Maybe follow precedent of nut-ddl structured names with two underscores between tokens? like USAGEBATSMB_BATTERY_STATUS ?
-- Reply to this email directly or view it on GitHub: https://github.com/networkupstools/nut/issues/1189#issuecomment-1038010123 You are receiving this because you were mentioned.
Message ID: @.***>
Well then, given a small dictionary of prefixes to juggle, single underscores can be good and not ambiguous... at least, it's sure better to get fixes delivered than to bikeshed over this :-)
On Sun, Feb 13, 2022, 12:17 flashydave @.***> wrote:
Thats starting to get messy. Not only does it become irksome to type (especially for those using command line type editors) and reduces the amount of code that can seen on any guven size of screen. For example my recent contributions were all developed using ssh from my phone!
On 02:51, Sun 13 Feb 22, Jim Klimov wrote:
I like underscores as more readable, especially where we can't have CamelCase etc. Maybe follow precedent of nut-ddl structured names with two underscores between tokens? like USAGEBATSMB_BATTERY_STATUS ?
-- Reply to this email directly or view it on GitHub:
https://github.com/networkupstools/nut/issues/1189#issuecomment-1038010123 You are receiving this because you were mentioned.
Message ID: @.***>
— Reply to this email directly, view it on GitHub https://github.com/networkupstools/nut/issues/1189#issuecomment-1038028148, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAMPTFCAVNIQKN53KKSAJSDU26HNRANCNFSM5IG337SA . 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 were mentioned.Message ID: @.***>
I agree!
On 11:43, Sun 13 Feb 22, Jim Klimov wrote:
Well then, given a small dictionary of prefixes to juggle, single underscores can be good and not ambiguous... at least, it's sure better to get fixes delivered than to bikeshed over this :-)
On Sun, Feb 13, 2022, 12:17 flashydave @.***> wrote:
Thats starting to get messy. Not only does it become irksome to type (especially for those using command line type editors) and reduces the amount of code that can seen on any guven size of screen. For example my recent contributions were all developed using ssh from my phone!
On 02:51, Sun 13 Feb 22, Jim Klimov wrote:
I like underscores as more readable, especially where we can't have CamelCase etc. Maybe follow precedent of nut-ddl structured names with two underscores between tokens? like USAGEBATSMB_BATTERY_STATUS ?
-- Reply to this email directly or view it on GitHub:
https://github.com/networkupstools/nut/issues/1189#issuecomment-1038010123 You are receiving this because you were mentioned.
Message ID: @.***>
— Reply to this email directly, view it on GitHub https://github.com/networkupstools/nut/issues/1189#issuecomment-1038028148, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAMPTFCAVNIQKN53KKSAJSDU26HNRANCNFSM5IG337SA . 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 were mentioned.Message ID: @.***>
-- Reply to this email directly or view it on GitHub: https://github.com/networkupstools/nut/issues/1189#issuecomment-1038380629 You are receiving this because you were mentioned.
Message ID: @.***>
OK... I've (force) pushed a revised version of 1189_APC_UPS_wrong_minmax_input_voltage with all the changes and stored a new version of usbhid-ups for x86_64 (SHA256(usbhid-ups)= 17d3420fc892f1cd8fb3d5e4c3b766708890725b76dbfe90ea8a6ddaaa27d495) on http://nhbriggs.users.sonic.net/usbhid-ups if @smainz and/or @andbez want to re-test on their APC units.
@nbriggs Is it enough for you when I run this commit on my Pi or do you need x86_64 too? The later means, i have to move cabinets.
Pi is compiling now, you will have feedback later this evening.
@smainz -- Pi is enough, I think @andbez has the x86_64 but not the setup for compiling it.
Here is:
$ sudo drivers/usbhid-ups -DDD -s usv01 -x port=auto -u root
Network UPS Tools - Generic HID driver 0.45 (2.7.4-4615-gbaae14d9)
USB communication driver (libusb 1.0) 0.42
0.390418 Using subdriver: APC HID 0.97
0.392100 [D3] Attempting Report Descriptor fix for UPS: Vendor: 051d, Product: 0002
0.392516 [D3] Fixing Report Descriptor. Set voltage LogMin = 0, LogMax = 560
0.392899 [D3] Fixing Report Descriptor. Set configVoltage LogMin = 75, LogMax = 255
0.393278 [D2] Report Descriptor Fixed
0.614673 [D1] Path: UPS.Input.Voltage, Type: Feature, ReportID: 0x31, Offset: 0, Size: 16, Value: 228
0.618179 [D3] Report[get]: (3 bytes) => 32 9b 00
0.618711 [D1] Path: UPS.Input.LowVoltageTransfer, Type: Feature, ReportID: 0x32, Offset: 0, Size: 16, Value: 155
0.621399 [D3] Report[get]: (3 bytes) => 33 18 01
Looks good. Thank you
Here is: ;-)
root@omv:~# ./usbhid-ups -a ups -u root -DDD 2>&1 Network UPS Tools - Generic HID driver 0.45 (2.7.4-4540-gaefcdc0a) USB communication driver (libusb 1.0) 0.42
0.185032 Using subdriver: APC HID 0.97 0.185036 [D3] Attempting Report Descriptor fix for UPS: Vendor: 051d, Product: 0002 0.185041 [D3] Fixing Report Descriptor. Set voltage LogMin = 0, LogMax = 560 0.185045 [D3] Fixing Report Descriptor. Set configVoltage LogMin = 75, LogMax = 255 0.185048 [D2] Report Descriptor Fixed
0.329770 [D1] Path: UPS.Input.Voltage, Type: Fea ture, ReportID: 0x31, Offset: 0, Size: 16, Value: 236 0.334925 [D3] Report[get]: (3 bytes) => 32 9b 00 0.334941 [D1] Path: UPS.Input.LowVoltageTransfer, Type: Feature, ReportID: 0x32, Offset: 0, Size: 16, Value: 155 0.339027 [D3] Report[get]: (3 bytes) => 33 18 01
@nbriggs Thank you for providing the binary file.
Thanks @smainz and @andbez for testing. I re-read my code and realized there's one more minor change to make having to do with when it reports ("Report Descriptor Fixed") that it did (or did not) find the problem. That probably only requires someone visually inspect the change, though. Famous last words and all that...
Updated again with bumped driver version number and check as mentioned above. % openssl sha256 usbhid-ups SHA256(usbhid-ups)= 828a31c62d819f04c4ad305163adb9108f0acfd9d526e0285d5ff8fa77725d49 and updated at the same link as before (x86_64 version). Submitted PR for it -- I think this one's done for now.
Thanks all for reporting, fixing and testing! Will follow up on the posted PR :)
Do you need another test or do you trust in you famous last words?
If you'd be happy to just eyeball the last change in source rather than build/test it that would be enough for me. Can't speak to anyone else's wishes.
OK, checked it and it looks good. Did not check the constants though. I like the style of your code and comments.
@smainz -- thank you, though the code style is from @flashydave so thanks to him.
I am trying to set up nut fpr my APC Back-UPS XS 1400U formerly used with apcupsd.
Got almost all working, but nut us reporting wrong input voltage:
These values are defiitly wrong:
I am in Germany and we have 230V. apcupsd reports the correct values.
OS: Raspbian GNU/Linux 10 (buster) nut version 2.7.4 aken from the official raspbian repositories
Maybe this helps to analyze: