networkupstools
Unit-test and fix the Belkin/Liebert HID UPS exponents math
As raised on the mailing list at https://alioth-lists.debian.net/pipermail/nut-upsuser/2024-March/013626.html as well as in discussions for #2271, #2369 and #2370, there is some mess in exponents calculation of drivers/belkin-hid.c, where the driver tries to adjust for what seems wrongly encoded voltage readings in USB HID reports, which may be off by some orders of magnitude (3, 7...) depending on reading type and HW/FW model.
As detailed in https://github.com/networkupstools/nut/issues/2370#issuecomment-2017664722 : The original logic which purported to fix this was broken, but by coincidence seems to have taken the right code path for the majority of devices up till NUT v2.7.4. This was fixed in NUT v2.8.0 to correct logic, but the order of magnitude estimation results no longer fit the devices' outputs, so the scaling was not applied and this caused various voltages to be reported as zeroes - causing a practical regression.
As suggested in recent discussions on the matter, this PR adds a (rather ugly, but functional) way to test the helper conversion methods "in-vivo" - in the practically unmodified sub-driver code, mocking the methods and variables it refers to elsewhere. Maybe this can be done even without the currently posted change to no longer mark static the variables and methods we need for a test, either by #include of the driver source into a test program, or by shipping test methods as part of a driver/program source and/or custom-built binary (ZeroMQ style) - something to refine later. UPDATE: Went for including at the moment, was not hard after the other mock preparations.
At the moment the test is functional in terms of calling one of the 3 methods currently provided by the driver source (1 added in #2369) and finding that the original method fixed in 2.8.0 (to check those clauses as floating-point numbers and not zeroed out rounded integers) indeed failed to work for the typical values it expected to process.
Test #1 liebert_psi5_line_voltage_mult() GOT value 27.3 mult 100000 PASS
Test #2 liebert_psi5_line_voltage_mult() GOT value 121.2 mult 100000 PASS
LineVoltage exponent looks wrong, but not correcting.
Test #3 liebert_line_voltage_mult() GOT value 0 mult 1 FAIL EXPECTED v= 13.9 m= 1e+07 ORIGINAL (string)'1.39e-06' => (double)1.39e-06
LineVoltage exponent looks wrong, but not correcting.
Test #4 liebert_line_voltage_mult() GOT value 0 mult 1 FAIL EXPECTED v= 220.1 m= 1e+07 ORIGINAL (string)'2.201e-05' => (double)2.201e-05
Test #5 liebert_psi5_line_voltage_mult() GOT value 12 mult 1 PASS
Test #6 liebert_psi5_line_voltage_mult() GOT value 12.3 mult 1 PASS
Test #7 liebert_psi5_line_voltage_mult() GOT value 232.1 mult 1 PASS
Test #8 liebert_psi5_line_voltage_mult() GOT value 240 mult 1 PASS
Test #9 liebert_line_voltage_mult() GOT value 12 mult 1 PASS
Test #10 liebert_line_voltage_mult() GOT value 12.3 mult 1 PASS
Test #11 liebert_line_voltage_mult() GOT value 232.1 mult 1 PASS
Test #12 liebert_line_voltage_mult() GOT value 240 mult 1 PASS
Test #13 liebert_config_voltage_mult() GOT value 24 mult 1 PASS
Test #14 liebert_config_voltage_mult() GOT value 120 mult 1 PASS
From this point, some TDD can be done to adjust the implementations... and I suppose the liebert_psi5_line_voltage_mult() checks can be merged with liebert_line_voltage_mult() as was originally intended in PoC code posted in issue #2271 - there's little left to break in the original one, it seems.
(CC @jrjparks @clepple @aquette @digitlman @electroflame)
:x: Build nut 2.8.1.1610-master failed (commit https://github.com/networkupstools/nut/commit/028478361a by @jimklimov)
I think I've picked my way through that if clause. It further happened to work for @jrjparks thanks to 120V utility network - and the maths does break if I put 0.002456 (for 240V) into the new PSI5 method.
The original logic seems to have been like "check if we have a value 0..199: subtract 100, and if resulting module is within 100 - pick corresponding multiplier".
Actually the old original (with fabs(value - 1e-7) < 1e-9) went further with the likes of "check if we have a value with expected magnitude: subtract 100, and if resulting module is within 1 - pick corresponding multiplier" which IMHO hits for values "99..101", at best? so practically fails for everything.
Fixing this into "subtract 100, and if resulting module is within 300 - pick corresponding multiplier" should match practical voltages from 0 to 380 (three-pole) with the same order of magnitudes in question. Any objections? :)
:white_check_mark: Build nut 2.8.1.1613-master completed (commit https://github.com/networkupstools/nut/commit/cc082b5ca9 by @jimklimov)
Cheers, any chance for practical testing, to merge the PR this weekend? <3 :)
