override.battery.packs doesn't work to adjust reported battery voltage

baerenwaldfreund commented 2 years ago

I spent a lot of time reactivating my old UPS: Online Xanto S700 (build before 2012). But now it almost works. But I still have a small error.

The parameter 'override.battery.packs' doesn't work. Incorrect battery voltage is still reported.

Here the output:

:; upsc ups@localhost

Init SSL without certificate database
battery.charge: 100
battery.packs: 12
battery.runtime: 2400
battery.voltage: 2.27
battery.voltage.high: 26.00
battery.voltage.low: 22.00
battery.voltage.nominal: 24.00
device.mfr: Online-UPS
device.model: Xanto S700
device.type: ups
driver.name: nutdrv_qx
driver.parameter.chargetime: 43200
driver.parameter.idleload: 10
driver.parameter.pollfreq: 5
driver.parameter.pollinterval: 2
driver.parameter.port: /dev/ttyUSB0
driver.parameter.protocol: q1
driver.parameter.runtimecal: 240,100,720,50
driver.parameter.synchronous: no
driver.version: 2.7.4
driver.version.data: Q1 0.07
driver.version.internal: 0.28
input.frequency: 50.0
input.voltage: 243.8
input.voltage.fault: 243.8
output.voltage: 230.0
ups.beeper.status: disabled
ups.delay.shutdown: 30
ups.delay.start: 180
ups.load: 0
ups.mfr: Online-UPS
ups.model: Xanto S700
ups.status: OL
ups.temperature: 25.0
ups.type: online

The batterie voltage is 2.27V. So I hoped the parameter 'override.battery.packs=12' will help. But it doesn't work. The reported voltage is still 2.27V.

Here is my ups.conf:

[ups]
        driver = nutdrv_qx
        port = "/dev/ttyUSB0"
        desc = "Online Xanto S700"
        protocol = q1
        override.battery.packs = 12
        default.battery.voltage.nominal = 24.00
        default.battery.voltage.low = 22.00
        default.battery.voltage.high = 26.00
        runtimecal = 240,100,720,50
        chargetime = 43200
        idleload = 10
        pollfreq = 5
        default.ups.mfr = "Online-UPS"
        default.ups.model = "Xanto S700"

What can I do to ensure that the correct voltage is displayed?

jimklimov commented 2 years ago

Seems I won't be able to dedicate time to this in the coming days, so PRs tested on HW would be welcome :) (maybe against my fork/branch, or a new one).

jimklimov commented 2 years ago

Anyone interested in fixing this? Go get your Hacktoberfest hats on! ;)

baerenwaldfreund commented 1 year ago

I don't understand how to solve the problem with the wrong battery.voltage and battery.packs. It's fixed, isn't it?!?!

I installed the testing package 2.8.0-5. But this problem is still there.

jimklimov commented 1 year ago

Not quite, a solution was experimented with PR #1652 but stalled a bit on... something (gotta revise and remember why). Beside the PR source branch availability, it was not merged back to master.

Not sure what "2.8.0-5" package contains (or in which OS taxonomy it is), but generally package revisions are unlikely to include code from master branch of an upstream project (more so code not even merged there yet), and are usually revisions of the packaging recipe or some patches hand-picked by distro maintainers.

y0g33 commented 1 year ago

Seasons Greetings, @jimklimov Thanks , Just in time for Christmas :-). It seems the latest code does fix all the issues.


upsc abb1kva
battery.charge: 100
battery.packs: 12
battery.runtime: 1622
battery.voltage: 27.12
battery.voltage.high: 27.0
battery.voltage.low: 23.6
battery.voltage.nominal: 24.0
device.model: WPHVR1K0
device.type: ups
driver.name: nutdrv_qx
driver.parameter.battery_voltage_reports_one_pack: 1
driver.parameter.bus: 001
driver.parameter.default.battery.voltage.low: 23.6
driver.parameter.override.battery.packs: 12
driver.parameter.override.battery.voltage.high: 27.0
driver.parameter.pollfreq: 30
driver.parameter.pollinterval: 2
driver.parameter.port: auto
driver.parameter.productid: 5161
driver.parameter.runtimecal: 240,100,1380,25
driver.parameter.subdriver: cypress
driver.parameter.synchronous: auto
driver.parameter.vendorid: 0665
driver.version: Windows-v2.8.0-alpha3-1006-g9d3393835
driver.version.data: Megatec 0.07
driver.version.internal: 0.33
driver.version.usb: libusb-1.0.26 (API: 0x1000109)
input.current.nominal: 4.0
input.frequency: 50.0
input.frequency.nominal: 50
input.voltage: 237.2
input.voltage.fault: 237.2
input.voltage.nominal: 240
output.voltage: 239.7
ups.beeper.status: enabled
ups.delay.shutdown: 30
ups.delay.start: 180
ups.firmware: 02724.08
ups.load: 19
ups.productid: 5161
ups.status: OL
ups.temperature: 26.0
ups.type: online
ups.vendorid: 0665

jimklimov commented 1 year ago

Thanks for the confirmation @y0g33 ! I tried to retrace the notes here and in the PR, but not sure what changed for the issue between your last test (before Hacktoberfest) and now - possibly something tangentially related in merge from master branch done later?..

If possible, please give this build some more attention (e.g. how well it reports the charge and voltage after some driver uptime), can the fix be considered completed or needs some more tweaks?

I would then hope to re-merge from master (currently has a merge-conflict) after some other PRs which are currently on CI testing do land, and merge this fix back to master after it passes for everyone to benefit.

jimklimov commented 1 year ago

Got back to this subject, again... The last two reports did substantially differ in default.battery.voltage.low added to the latter config (and notably "default", not "override" -- it is an initial value until one is read from the device, if ever). Sources for nutdrv_qx* and blazer* did not change between the Git commits behind those builds, as reported in driver.version, so the same formulas were applied -- see https://github.com/networkupstools/nut/pull/1652#issuecomment-1705708623 -- and the ratios were about 103% (should be truncated to 100% as the full charge) - so very not sure how it ended up with 70% in one of the reports.

The earlier reports (with 69%) were at least explainable by battery.voltage.high: 30.00 (in turn, due to the PR's change towards 150/120 of the nominal instead of earlier 130/120):

(27.12 - 20.8) / (30.0 - 20.8) = 0,68695...

but the override.battery.voltage.high: 27.0 in later experiment took care of that, and the ratio became over 100% (due to actual voltage being greater than high). So again, that 70% is curious.

jimklimov commented 1 year ago

Hello, @y0g33 @baerenwaldfreund @y0g33 @EmersonHeiderich @anphsw @mateuszdrab - long time no see :)

I've had a dive back into this code and updated the pull request hoping to fix it. Would any of you have a chance to try custom-building from that PR's source branch https://github.com/jimklimov/nut/tree/issue-1279 to see if these issues are finally fixed? https://github.com/networkupstools/nut/wiki/Building-NUT-for-in%E2%80%90place-upgrades-or-non%E2%80%90disruptive-tests summarizes how to do so, without necessarily impacting your installed NUT. Roughly like:

:; git clone --branch issue-1279  https://github.com/jimklimov/nut.git
:; cd nut
:; ./ci_build.sh inplace
### Stop currently running driver instance (systemd, init-scripts)
:; ./drivers/nutdrv_qx -a DEVNAME_FROM_UPS_CONF -d1 -DDDDDD
### Start back your currently running driver instance

If this time around is the charm, and new code works better(*) and not worse(**) than the current master, this can finally get merged to benefit everyone (and unblock cutting a 2.8.1 release in my eyes).

should work better for devices with smallish reported battery voltage (hopefully guessing the needed battery.packs, otherwise relying on overrides in ups.conf or CLI for tests)
if guesswork misfires for uncommon battery.voltage, it would be useful to know - but that battery_voltage_reports_one_pack would fix it
well-charged on-line devices do not "guesstimate" their charges as under 100%
(not part of this issue but might get impacted) with runtimecal in place, guesstimation of remaining run-time works reasonably

jimklimov commented 1 year ago

The PR was merged, hopefully all the issues were weeded out and this area of NUT would behave no worse than it did without the feature.

networkupstools / nut

override.battery.packs doesn't work to adjust reported battery voltage #1279