Unit-test and fix the Belkin/Liebert HID UPS exponents math

[jimklimov]

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 #2370 (comment) : 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)

[AppVeyorBot]

❌ Build nut 2.8.1.1610-master failed (commit 028478361a by @jimklimov)

[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? :)

[jimklimov]

Fix in driver proposed, testing on HW would be very welcome :)

[AppVeyorBot]

✅ Build nut 2.8.1.1613-master completed (commit cc082b5ca9 by @jimklimov)

[jimklimov]

Cheers, any chance for practical testing, to merge the PR this weekend? <3 :)

[jrjparks]

I can try it out tomorrow

[jimklimov]

Gentle bump, itching to release this fix :)