Skip to content
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.

Sign up for GitHub

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

Jump to bottom

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

Closed
baerenwaldfreund opened this issue Feb 9, 2022 · 59 comments
Closed

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

baerenwaldfreund opened this issue Feb 9, 2022 · 59 comments
Labels
bug DDL HCL Incorrect or missing readings On some devices driver-reported values are systemically off (e.g. x10, x0.1, const+Value, etc.) Qx protocol driver Driver based on Megatec Q<number> such as new nutdrv_qx, or obsoleted blazer and some others Shutdowns and overrides and battery level triggers Issues and PRs about system shutdown, especially if battery charge/runtime remaining is involved
Milestone
2.8.1

Comments

@baerenwaldfreund
Copy link

baerenwaldfreund commented Feb 9, 2022 •
edited by jimklimov
Loading

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?
@baerenwaldfreund baerenwaldfreund changed the title override.battery.packs does't work override.battery.packs doesn't work Feb 9, 2022
@jimklimov
Copy link
Member

I am not sure how that value's override would impact voltage. I believe it is for tracking the hardware composition of (larger) UPSes - how many independent batteries it has, if some can be diagnosed and replaced separately: https://github.com/networkupstools/nut/blob/master/docs/nut-names.txt#L450

Beside that, the issue title is misleading: according to the data screenshot in your post, battery.packs: 12 is in fact applied :)

@jimklimov
Copy link
Member

Also, just in case: are you able to measure the actual battery voltage with an electric meter?

Is the battery as old as the UPS or did you replace it - their lifetime is in a few years' range at best, so a 10 years old battery is probably dead and the voltage reading may be in fact correct ;)

@baerenwaldfreund
Copy link
Author

baerenwaldfreund commented Feb 10, 2022 •
edited by jimklimov
Loading

thx for your reply.

The two batteries are new. I replaced the batteries a few weeks ago. The voltage should be 2x12V=24V (nominal voltage). I measured the open circuit voltage at 27,2V.

The original software of my UPS shows the right voltage: 27,24V. So the problem is in the nut-software.

Here the description of this parameter:

 override.battery.packs = value

    Some devices report a part of the total battery voltage.

    For instance, if battery.voltage.nominal is 24 V, but it reports a battery.voltage
of around 2 V, the number of battery.packs to correct this reading would be 12.
The driver will attempt to detect this automatically, but if this fails somehow,
you may want to override this value.

per https://networkupstools.org/docs/man/nutdrv_qx.html

So it should work. But it doesn't work :-(

I found another thread about this:
#1039

But I can't find a solution.

Any ideas?

@jimklimov jimklimov changed the title override.battery.packs doesn't work override.battery.packs doesn't work to adjust reported battery voltage Feb 10, 2022
@jimklimov jimklimov added HCL DDL bug Qx protocol driver Driver based on Megatec Q<number> such as new nutdrv_qx, or obsoleted blazer and some others labels Apr 25, 2022
jimklimov added a commit to jimklimov/nut that referenced this issue Apr 25, 2022
@jimklimov
driver.list.in: Add HCL info for Online-UPS Xanto S700 [networkupstoo…
f3a70a3 
…ls#1279]
@EmersonHeiderich
Copy link

EmersonHeiderich commented May 17, 2022 •
edited
Loading

I also have the same issue reported by the user. My NUT is returning Battery.voltage 225.00 and when viewing the UPS panel or measuring through a voltmeter, I get 216 Volts. I performed tests with blazer_ser and nutdrv_qx but got the same results.

Has anyone found a solution?

NOTE: Nobreate TS Shara TS SYAL 10 KVa connected via serial to USB converter.

ups@ups:~$ upsc tsshara01
Init SSL without certificate database
battery.charge: 100
battery.voltage: 225.00
battery.voltage.high: 221
battery.voltage.low: 166.40
battery.voltage.nominal: 192.0
device.mfr:
device.model: 11-06K
device.type: ups
driver.name: blazer_ser
driver.parameter.pollinterval: 2
driver.parameter.port: /dev/ttyUSB0
driver.parameter.synchronous: no
driver.version: 2.7.4
driver.version.internal: 1.57
input.current.nominal: 47.0
input.frequency: 59.9
input.frequency.nominal: 60
input.voltage: 122.5
input.voltage.fault: 32.9
input.voltage.nominal: 127
output.voltage: 126.9
ups.beeper.status: enabled
ups.delay.shutdown: 30
ups.delay.start: 180
ups.firmware: 11.001.011
ups.load: 36
ups.mfr:
ups.model: 11-06K
ups.status: OL
ups.temperature: 25.0
ups.type: online

@anphsw
Copy link

anphsw commented Jun 8, 2022

Facing same problem with Powerman Online 1000 Plus (USB connection):
(battery.packs are changed, but voltage still around 2V)

[ 1194.166542] usb 1-8: new low-speed USB device number 3 using xhci_hcd
[ 1194.182429] usb 1-8: New USB device found, idVendor=0001, idProduct=0000
[ 1194.182434] usb 1-8: New USB device strings: Mfr=1, Product=2, SerialNumber=0
[ 1194.182460] usb 1-8: Product: MEC0003
[ 1194.182463] usb 1-8: Manufacturer: MEC
[ 1194.183546] usb 1-8: ep 0x81 - rounding interval to 64 microframes, ep desc says 80 microframes
[ 1194.728180] generic-usb 0003:0001:0000.0001: hiddev0,hidraw0: USB HID v1.00 Device [MEC MEC0003] on usb-0000:00:14.0-8/input0

$upsc ups0@localhost
battery.charge: 100
battery.packs: 12
battery.voltage: 2.25
battery.voltage.high: 26.00
battery.voltage.low: 20.80
battery.voltage.nominal: 24
device.type: ups
driver.name: nutdrv_qx
driver.parameter.pollfreq: 30
driver.parameter.pollinterval: 2
driver.parameter.port: auto
driver.parameter.synchronous: no
driver.version: 2.7.4
driver.version.data: Megatec 0.06
driver.version.internal: 0.28
input.current.nominal: 4.0
input.frequency: 50.0
input.frequency.nominal: 50
input.voltage: 235.4
input.voltage.fault: 120.0
input.voltage.nominal: 220
output.voltage: 235.3
ups.beeper.status: enabled
ups.delay.shutdown: 30
ups.delay.start: 180
ups.firmware: V009B002D=
ups.load: 31
ups.productid: 0000
ups.status: OL BYPASS
ups.temperature: 27.0
ups.type: online
ups.vendorid: 0001

@anphsw
Copy link

anphsw commented Jun 8, 2022

Tried blazer_usb with with same override.battery.packs value - it shows actual voltage, so maybe bug in nutdrv_qx only.
$upsc ups0@localhost
battery.charge: 100
battery.packs: 12
battery.voltage: 27.0
battery.voltage.high: 26.0
battery.voltage.low: 20.8
battery.voltage.nominal: 24.0
device.mfr:
device.model:
device.type: ups
driver.name: blazer_usb
driver.parameter.pollinterval: 2
driver.parameter.port: auto
driver.parameter.synchronous: no
driver.version: 2.7.4
driver.version.internal: 0.12
input.current.nominal: 4.0
input.frequency: 50.0
input.frequency.nominal: 50
input.voltage: 235.8
input.voltage.fault: 120.0
input.voltage.nominal: 220
output.voltage: 235.7
ups.beeper.status: enabled
ups.delay.shutdown: 30
ups.delay.start: 180
ups.firmware: V009B002D=
ups.load: 31
ups.mfr:
ups.model:
ups.productid: 0000
ups.status: OL BYPASS
ups.temperature: 27.0
ups.type: online
ups.vendorid: 0001

@jimklimov
Copy link
Member

Just in case - might also be a bug in 2.7.4 release that was long ago. With 2.8.0 finally published this year, are you in position to check if it (or current git head) fares better?

@mateuszdrab
Copy link

I think there is a bug in nutdrv_qx, switched from blazer_usb and lost battery charge info additionally

@y0g33
Copy link

y0g33 commented Aug 31, 2022

FWIW, I can confirm with latest git head the bug in nutdrv_qx persists. Tested on ABB Powervalue with usb

Network UPS Tools - Generic Q* USB/Serial driver 0.32 (2.8.0-5-g9cb8de681)
USB communication driver (libusb 1.0) 0.43
battery.charge: 0
battery.voltage: 2.28
battery.voltage.high: 26.00
battery.voltage.low: 20.80
battery.voltage.nominal: 24.0
device.model: WPHVR1K0
device.type: ups
driver.name: nutdrv_qx
driver.parameter.pollfreq: 30
driver.parameter.pollinterval: 2
driver.parameter.port: auto
driver.parameter.productid: 5161
driver.parameter.synchronous: auto
driver.parameter.vendorid: 0665
driver.version: 2.8.0-5-g9cb8de681
driver.version.data: Megatec 0.06
driver.version.internal: 0.32
driver.version.usb: libusb-1.0.26 (API: 0x1000109)
input.current.nominal: 4.0
input.frequency: 50.0
input.frequency.nominal: 50
input.voltage: 238.9
input.voltage.fault: 238.4
input.voltage.nominal: 240
output.voltage: 238.7
ups.beeper.status: enabled
ups.delay.shutdown: 30
ups.delay.start: 180
ups.firmware: 02724.08
ups.load: 17
ups.productid: 5161
ups.status: OL BYPASS
ups.temperature: 24.9
ups.type: online
ups.vendorid: 0665

@jimklimov
Copy link
Member

Just to clarify: 9cb8de681 is not "latest git head" but the April release of NUT v2.8.0. Some bugs were fixed since then, though I don't remember anyone addressing this one.

One thing that comes to mind regards the change from libusb-0.1 to 1.0 - might be some issue in that lib or use of it?.. Can you build and test in-place (install not required) current NUT against libusb-0.1.x to rule that out?

@jimklimov
Copy link
Member

Also by IDs in the last post, I think the subdriver should be cypress?

@y0g33
Copy link

y0g33 commented Sep 1, 2022

Found the error, was using old build config rules. After fixing the config, still no dice :-(.

battery.charge: 100
battery.voltage: 2.28
battery.voltage.high: 26.00
battery.voltage.low: 20.80
battery.voltage.nominal: 24.0
device.model: WPHVR1K0
device.type: ups
driver.name: nutdrv_qx
driver.parameter.pollfreq: 30
driver.parameter.pollinterval: 2
driver.parameter.port: auto
driver.parameter.productid: 5161
driver.parameter.subdriver: cypress
driver.parameter.synchronous: auto
driver.parameter.vendorid: 0665
driver.version: 2.8.0-497-g532bada82
driver.version.data: Megatec 0.06
driver.version.internal: 0.32
driver.version.usb: libusb-1.0.26 (API: 0x1000109)
input.current.nominal: 4.0
input.frequency: 50.0
input.frequency.nominal: 50
input.voltage: 240.8
input.voltage.fault: 240.6
input.voltage.nominal: 240
output.voltage: 240.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 BYPASS
ups.temperature: 23.0
ups.type: online
ups.vendorid: 0665

As requested here is an output when compiled with libusb-0.1.

battery.charge: 100
battery.voltage: 2.28
battery.voltage.high: 26.00
battery.voltage.low: 20.80
battery.voltage.nominal: 24.0
device.model: WPHVR1K0
device.type: ups
driver.name: nutdrv_qx
driver.parameter.pollfreq: 30
driver.parameter.pollinterval: 2
driver.parameter.port: auto
driver.parameter.productid: 5161
driver.parameter.subdriver: cypress
driver.parameter.synchronous: auto
driver.parameter.vendorid: 0665
driver.version: 2.8.0-497-g532bada82
driver.version.data: Megatec 0.06
driver.version.internal: 0.32
driver.version.usb: libusb-0.1 (or compat)
input.current.nominal: 4.0
input.frequency: 50.0
input.frequency.nominal: 50
input.voltage: 238.8
input.voltage.fault: 239.6
input.voltage.nominal: 240
output.voltage: 238.8
ups.beeper.status: enabled
ups.delay.shutdown: 30
ups.delay.start: 180
ups.firmware: 02724.08
ups.load: 18
ups.productid: 5161
ups.status: OL BYPASS
ups.temperature: 22.9
ups.type: online
ups.vendorid: 0665

@jimklimov
Copy link
Member

jimklimov commented Sep 6, 2022 •
edited
Loading

Scribbling some notes here, to juggle for "differential diagnostics, people!" :)

  • blazer_ser returns IMHO strange results - battery (pack) yielding 216V? (or 225V for that matter... wondering if there is some rounding error with the maths and int-float conversions involved)
    • To me, the hundreds of Volts are suspicious on batteries, if that is not a rack-sized stack of UPS with expansion cabinets... but if the original software, or panel, or Voltmeter confirm that... so be it
    • "216 vs 225" needs a closer look at maths done in code. Note that part of NUT release v2.8.0 there was a large effort to fix compilation warnings, including revision of numeric type conversion and casting. It would be helpful to check if current codebase also mis-reports that reading, or fixed it "inadvertently" (and only delve further if not fixed). Notably, some casting from floating-point to integer types in the middle of calculations chain could cause effective rounding, and errors of that could aggregate in overall result.
  • blazer_usb seems to work as people expect it to, at least for NUT v2.7.4 codebase
  • nutdrv_qx seems to NOT work as people expect it to, regardless of NUT v2.7.4 of v2.8.0+ codebase and libusb0/1 backend, so at least not a regression of v2.8.0
  • some of the nutdrv_qx reports did not include a battery.packs reading so I suppose those reporters did not set the override parameter

Also:

  • the "override" and "default" parameters are not reflected in the "driver.*" collection complicating the investigation

I think there is a bug in nutdrv_qx, switched from blazer_usb and lost battery charge info additionally

  • They are different drivers with shared heritage, possibly not all mapping tables are in sync. And/or different subdriver or protocol settings were used - not enough detail in this message.
  • UPDATE: Per code analysis added below, these two issues may be related - seems charge is only calculated in some conditions if not reported by device directly. Although I think the intent was to provide it always - either natively from device reports, or guesstimated.

Noting similar code paths that deal with battery packs variables in blazer and nutdrv_qx drivers:

  • qx_initbattery() tries to guess battery.packs if not specified by config (or not learned from explicit data walk):

    • nut/drivers/nutdrv_qx.c

      Lines 320 to 359 in 4e3d0ec

      val = dstate_getinfo("battery.packs");
      if (val && (strspn(val, "0123456789 .") == strlen(val))) {
      batt.packs = strtod(val, NULL);
      } else {
      /* qx_battery -> batt.volt.act */
      if (!qx_battery() && (!d_equal(batt.volt.nom, -1))) {
      const double packs[] = { 120, 100, 80, 60, 48, 36, 30, 24, 18, 12, 8, 6, 4, 3, 2, 1, 0.5, -1 };
      int i;
      /* The battery voltage will quickly return to
      * at least the nominal value after discharging them.
      * For overlapping battery.voltage.low/high ranges
      * therefore choose the one with the highest multiplier. */
      for (i = 0; packs[i] > 0; i++) {
      if (packs[i] * batt.volt.act > 1.2 * batt.volt.nom) {
      continue;
      }
      if (packs[i] * batt.volt.act < 0.8 * batt.volt.nom) {
      upslogx(LOG_INFO,
      "Can't autodetect number of battery packs [%.0f/%.2f]",
      batt.volt.nom, batt.volt.act);
      break;
      }
      batt.packs = packs[i];
      break;
      }
      } else {
      upslogx(LOG_INFO,
      "Can't autodetect number of battery packs [%.0f/%.2f]",
      batt.volt.nom, batt.volt.act);
      }
      }
    • The bug might be here, in if (!dstate_getinfo("battery.charge") || !dstate_getinfo("battery.runtime")) { lots of init } clause at the start of the function -- at least for the original post, both dstate values are available so the driver never sets batt.packs it seems (and battery.voltage.high/low/nom for that matter):

      • nut/drivers/nutdrv_qx.c

        Lines 278 to 283 in 4e3d0ec

        /* Guesstimation: init */
        static void qx_initbattery(void)
        {
        if (!dstate_getinfo("battery.charge") || !dstate_getinfo("battery.runtime")) {
        const char *val;
    • Looking at that if clause, it seems that by original design (very original - from initial commit dee3620 of the driver a decade ago), if the device initially reports its charge and runtime on its own, we do not guesstimate them from voltages and amounts of batteries; arguably per that intention these should not have been reported at all.
    • Maybe it makes sense to restructure this bit to do actually set values we have read from device or override config regardless of whether we have practical use for them. @clepple @aquette @zykh - WDYT?
    • Notably blazer_initbattery() is not so shielded from the start:

      nut/drivers/blazer.c

      Lines 602 to 610 in 4e3d0ec

      static void blazer_initbattery(void)
      {
      const char *val;
      /* If no values were provided by the user in ups.conf, try to guesstimate
      * battery.charge, but announce it! */
      if ( (!d_equal(batt.volt.nom, 1)) && ((d_equal(batt.volt.high, -1)) || (d_equal(batt.volt.low, -1)))) {
      upslogx(LOG_INFO, "No values provided for battery high/low voltages in ups.conf\n");
  • Equivalent bit of logic in blazer to set up batt.packs (apparently from battery.voltage.nominal) is:

    • nut/drivers/blazer.c

      Lines 133 to 167 in 4e3d0ec

      /*
      * The battery voltage will quickly return to at least the nominal value after
      * discharging them. For overlapping battery.voltage.low/high ranges therefor
      * choose the one with the highest multiplier.
      */
      static double blazer_packs(const char *ptr, char **endptr)
      {
      const double packs[] = {
      120, 100, 80, 60, 48, 36, 30, 24, 18, 12, 8, 6, 4, 3, 2, 1, 0.5, -1
      };
      const char *val;
      int i;
      val = dstate_getinfo("battery.voltage.nominal");
      batt.volt.nom = strtod(val ? val : ptr, endptr);
      for (i = 0; packs[i] > 0; i++) {
      if (packs[i] * batt.volt.act > 1.2 * batt.volt.nom) {
      continue;
      }
      if (packs[i] * batt.volt.act < 0.8 * batt.volt.nom) {
      upslogx(LOG_INFO, "Can't autodetect number of battery packs [%.0f/%.2f]", batt.volt.nom, batt.volt.act);
      break;
      }
      batt.packs = packs[i];
      break;
      }
      return batt.volt.nom;
      }
  • qx_ups_walk() part which deals with battery.voltage (and estimates battery.charge) is enabled by runtimecal option for some more complicated time-sliding logic to consider battery.packs, otherwise falls back to qx_battery() for linear approximation if the option is not set:

    • nut/drivers/nutdrv_qx.c

      Lines 3739 to 3823 in 4e3d0ec

      /* Update battery guesstimation */
      if (mode == QX_WALKMODE_FULL_UPDATE
      && (d_equal(batt.runt.act, -1) || d_equal(batt.chrg.act, -1))
      ) {
      if (getval("runtimecal")) {
      time_t battery_now;
      time(&battery_now);
      /* OL */
      if (ups_status & STATUS(OL)) {
      batt.runt.est += batt.runt.nom * difftime(battery_now, battery_lastpoll) / batt.chrg.time;
      if (batt.runt.est > batt.runt.nom) {
      batt.runt.est = batt.runt.nom;
      }
      /* OB */
      } else {
      batt.runt.est -= load.eff * difftime(battery_now, battery_lastpoll);
      if (batt.runt.est < 0) {
      batt.runt.est = 0;
      }
      }
      const char *val = dstate_getinfo("battery.voltage");
      if (!val) {
      upsdebugx(2, "%s: unable to get battery.voltage", __func__);
      } else {
      /* For age-corrected estimates below,
      * see theory and experimental graphs at
      * https://github.com/networkupstools/nut/pull/1027
      */
      batt.volt.act = batt.packs * strtod(val, NULL);
      if (batt.volt.act > 0 && batt.volt.low > 0 && batt.volt.high > batt.volt.low) {
      double voltage_battery_charge = (batt.volt.act - batt.volt.low) / (batt.volt.high - batt.volt.low);
      if (voltage_battery_charge < 0) {
      voltage_battery_charge = 0;
      }
      if (voltage_battery_charge > 1) {
      voltage_battery_charge = 1;
      }
      /* Correct estimated runtime remaining for old batteries:
      * this value replacement only happens if the actual
      * voltage_battery_charge is smaller than expected by
      * previous (load-based) estimation, thus adapting to a
      * battery too old and otherwise behaving non-linearly
      */
      if (voltage_battery_charge < (batt.runt.est / batt.runt.nom)) {
      double estPrev = batt.runt.est;
      batt.runt.est = voltage_battery_charge * batt.runt.nom;
      upsdebugx(3, "%s: updating batt.runt.est from '%g' to '%g'",
      __func__, estPrev, batt.runt.est);
      }
      }
      }
      if (d_equal(batt.chrg.act, -1))
      dstate_setinfo("battery.charge", "%.0f",
      100 * batt.runt.est / batt.runt.nom);
      if (d_equal(batt.runt.act, -1) && !qx_load())
      dstate_setinfo("battery.runtime", "%.0f",
      batt.runt.est / load.eff);
      battery_lastpoll = battery_now;
      } else {
      qx_battery();
      }
      }
  • blazer_battery() and qx_battery():

    • nut/drivers/blazer.c

      Lines 89 to 114 in 4e3d0ec

      /*
      * Do whatever we think is needed when we read a battery voltage from the UPS.
      * Basically all it does now, is guestimating the battery charge, but this
      * could be extended.
      */
      static double blazer_battery(const char *ptr, char **endptr)
      {
      batt.volt.act = batt.packs * strtod(ptr, endptr);
      if ((!getval("runtimecal") || !dstate_getinfo("battery.charge")) &&
      (batt.volt.low > 0) && (batt.volt.high > batt.volt.low)) {
      batt.chrg.act = 100 * (batt.volt.act - batt.volt.low) / (batt.volt.high - batt.volt.low);
      if (batt.chrg.act < 0) {
      batt.chrg.act = 0;
      }
      if (batt.chrg.act > 100) {
      batt.chrg.act = 100;
      }
      dstate_setinfo("battery.charge", "%.0f", batt.chrg.act);
      }
      return batt.volt.act;
      }

    • nut/drivers/nutdrv_qx.c

      Lines 225 to 255 in 4e3d0ec

      /* Fill batt.volt.act and guesstimate the battery charge
      * if it isn't already available. */
      static int qx_battery(void)
      {
      const char *val = dstate_getinfo("battery.voltage");
      if (!val) {
      upsdebugx(2, "%s: unable to get battery.voltage", __func__);
      return -1;
      }
      batt.volt.act = batt.packs * strtod(val, NULL);
      if (d_equal(batt.chrg.act, -1) && batt.volt.low > 0 && batt.volt.high > batt.volt.low) {
      batt.chrg.act = 100 * (batt.volt.act - batt.volt.low) / (batt.volt.high - batt.volt.low);
      if (batt.chrg.act < 0) {
      batt.chrg.act = 0;
      }
      if (batt.chrg.act > 100) {
      batt.chrg.act = 100;
      }
      dstate_setinfo("battery.charge", "%.0f", batt.chrg.act);
      }
      return 0;
      }

jimklimov added a commit to jimklimov/nut that referenced this issue Sep 6, 2022
@jimklimov
drivers/main.c: retain "driver.parameter.override.XXX" (and ".default…
87fd7eb 
….XXX") for troubleshooting [networkupstools#1279]
jimklimov added a commit to jimklimov/nut that referenced this issue Sep 6, 2022
@jimklimov
drivers/nutdrv_qx.c: qx_initbattery(): initialize battery.packs and b…
e429c78 
…attery.voltage.low/high/nom if known always (not just if battery.charge or battery.runtime are not served) [networkupstools#1279]
@jimklimov jimklimov mentioned this issue Sep 6, 2022
Merged
@jimklimov
Copy link
Member

Can you try building and running the driver from https://github.com/jimklimov/nut/tree/issue-1279 (source of PR #1652) to check if it behaves better? So far just making educated guesses...

@jimklimov jimklimov mentioned this issue Sep 12, 2022
Open
@y0g33
Copy link

y0g33 commented Sep 12, 2022

I get compile time errors when I try to build issue-1279. This does not happen with the master-branch

Build system is RPI3 running ArchLinux.

libtool: compile:  gcc -DHAVE_CONFIG_H -I. -I../include -I../include -isystem /usr/local/include -g -O2 -Wno-reserved-identifier -Wno-unknown-warning-option -std=gnu99 -Wno-system-headers -Wall -Wextra -Wsign-compare -pedantic -Wno-error -MT strptime.lo -MD -MP -MF .deps/strptime.Tpo -c strptime.c  -fPIC -DPIC -o .libs/strptime.o
strptime.c:59:25: error: expected ';' before 'uint64_t'
   59 | typedef unsigned __int64 uint64_t;
      |                         ^~~~~~~~~
      |                         ;
strptime.c:59:1: warning: useless type name in empty declaration
   59 | typedef unsigned __int64 uint64_t;
      | ^~~~~~~
strptime.c: In function 'strptime':
strptime.c:389:25: warning: implicit declaration of function '_tzset'; did you mean 'tzset'? [-Wimplicit-function-declaration]
  389 |                         _tzset();
      |                         ^~~~~~
      |                         tzset
At top level:
cc1: note: unrecognized command-line option '-Wno-unknown-warning-option' may have been intended to silence earlier diagnostics
cc1: note: unrecognized command-line option '-Wno-reserved-identifier' may have been intended to silence earlier diagnostics
make[1]: *** [Makefile:603: strptime.lo] Error 1
make[1]: Leaving directory '/home/alarm/scratch/nut/nut/common'
make: *** [Makefile:507: all] Error 2

@jimklimov
Copy link
Member

jimklimov commented Sep 13, 2022 •
edited
Loading

That's odd... errors per se seem familiar from recent discussion on more building stacks for Windows. This file is a fallback for platforms that lack strptime() in their libc, lifted from BSD and not much chiseled since it happened to work well for the one platform and toolkit it was then intended for practically.

The primary problem here is that it got built at all. The function should just be in standard Linux/glibc. And it should be similar in current master that got built well...

If you have that workspace still around, can you check in config.log - search for strptime and see what it could complain about when checking for build environment's capabilities? Any compiler error (space rays hitting RAM, low disk space, antivirus, etc.) that end up in a failed check cause an autotools decision that the method is not available.

So is it something random, due to build machine constraints, or something NUT recipes and other code can address?

Do the two codebases you tried differ in configure.ac or m4/* files? Maybe some existing fix was not backported from master to issue branches?..

@jimklimov
Copy link
Member

Also wondering if something important for the test happens reproducibly e.g. due to 32-bit ARM (right?)

@y0g33
Copy link

y0g33 commented Sep 13, 2022

It is a 64 bit system

output snippet from config.log

 $ ./configure --enable-NIT

## --------- ##
## Platform. ##
## --------- ##

uname -m = aarch64
uname -r = 5.15.61-4-rpi-ARCH
uname -s = Linux
uname -v = #1 SMP PREEMPT Thu Sep 1 13:34:10 MDT 2022

To the untrained eye only thing that stands out is when configure exits.

target_vendor='unknown'
udevdir=''
configure: caught signal 2
configure: exit 1

@jimklimov
Copy link
Member

The signal 2 would be Ctrl+C... Did the configure script complete? It should end with a printout of major features and CFLAGS, with a 0 exit-code.

@jimklimov
Copy link
Member

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
Copy link

y0g33 commented Dec 27, 2022

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
Copy link
Member

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 jimklimov mentioned this issue Jan 6, 2023
Open
8 tasks
@jimklimov jimklimov added this to the 2.8.1 milestone Jan 12, 2023
@jimklimov jimklimov mentioned this issue Feb 1, 2023
Closed
@jimklimov jimklimov mentioned this issue May 3, 2023
Open
@jimklimov jimklimov mentioned this issue May 27, 2023
Open
@jimklimov jimklimov added the Shutdowns and overrides and battery level triggers Issues and PRs about system shutdown, especially if battery charge/runtime remaining is involved label Aug 29, 2023
@jimklimov jimklimov mentioned this issue Aug 31, 2023
Open
@jimklimov jimklimov added the Incorrect or missing readings On some devices driver-reported values are systemically off (e.g. x10, x0.1, const+Value, etc.) label Sep 2, 2023
jimklimov added a commit to jimklimov/nut that referenced this issue Sep 2, 2023
@jimklimov
drivers/main.c: retain "driver.parameter.override.XXX" (and ".default…
5daf9a2 
….XXX") for troubleshooting [networkupstools#1279]
jimklimov added a commit to jimklimov/nut that referenced this issue Sep 2, 2023
@jimklimov
drivers/main.c: retain "driver.parameter.override.XXX" (and ".default…
6f4c1bf 
….XXX") for troubleshooting [networkupstools#1279]

Signed-off-by: Jim Klimov <jimklimov+nut@gmail.com>
jimklimov added a commit that referenced this issue Sep 2, 2023
@jimklimov
Merge pull request #2032 from jimklimov/issue-1279-nonfunctional
e3fb50d 
Cherry-pick nonfunctional improvements from PR #1652 (issue #1279)
@jimklimov jimklimov mentioned this issue Sep 4, 2023
Closed
@jimklimov
Copy link
Member

jimklimov commented Sep 4, 2023 •
edited
Loading

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 #1652 (comment) -- 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
Copy link
Member

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 added a commit that referenced this issue Sep 14, 2023
@jimklimov
Merge pull request #1652 from jimklimov/issue-1279
91b3ee0 
nutdrv_qx: init battery info always if it is known [issue #1279]
@jimklimov
Copy link
Member

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.

alexwbaule pushed a commit to alexwbaule/nut that referenced this issue Oct 4, 2023
@jimklimov @alexwbaule
drivers/main.c: retain "driver.parameter.override.XXX" (and ".default…
b805982 
….XXX") for troubleshooting [networkupstools#1279]

Signed-off-by: Jim Klimov <jimklimov+nut@gmail.com>
Signed-off-by: Alex W Baulé <alexwbaule@gmail.com>
alexwbaule pushed a commit to alexwbaule/nut that referenced this issue Oct 4, 2023
@jimklimov @alexwbaule
drivers/nutdrv_qx.c: qx_initbattery(): initialize battery.packs and b…
dad23aa 
…attery.voltage.low/high/nom if known always (not just if battery.charge or battery.runtime are not served) [networkupstools#1279]

Signed-off-by: Alex W Baulé <alexwbaule@gmail.com>
alexwbaule pushed a commit to alexwbaule/nut that referenced this issue Oct 4, 2023
@jimklimov @alexwbaule
drivers/nutdrv_qx.c: qx_initbattery(): extend default batt.volt.high …
083aef6 
…range [networkupstools#1279]

Signed-off-by: Alex W Baulé <alexwbaule@gmail.com>
alexwbaule pushed a commit to alexwbaule/nut that referenced this issue Oct 4, 2023
@jimklimov @alexwbaule
drivers/nutdrv_qx.c: qx_initbattery(): report if we autodetected numb…
e3f4da6 
…er of battery packs [networkupstools#1279]

Signed-off-by: Alex W Baulé <alexwbaule@gmail.com>
alexwbaule pushed a commit to alexwbaule/nut that referenced this issue Oct 4, 2023
@jimklimov @alexwbaule
drivers/blazer.c: blazer_packs(): extend default batt.volt.high range…
27ab34e 
… when guessing battery.packs count [networkupstools#1279]

Signed-off-by: Alex W Baulé <alexwbaule@gmail.com>
alexwbaule pushed a commit to alexwbaule/nut that referenced this issue Oct 4, 2023
@jimklimov @alexwbaule
drivers/nutdrv_qx.c: qx_initbattery(): extend default batt.volt.high …
0406845 
…range when guessing battery.packs count [networkupstools#1279]

Signed-off-by: Alex W Baulé <alexwbaule@gmail.com>
alexwbaule pushed a commit to alexwbaule/nut that referenced this issue Oct 4, 2023
@jimklimov @alexwbaule
NEWS: reflect extended default ranges for max battery voltage when gu…
6e5f27d 
…essing [networkupstools#1279]

Signed-off-by: Alex W Baulé <alexwbaule@gmail.com>
@jimklimov jimklimov mentioned this issue Feb 27, 2024
Open
@dennypage dennypage mentioned this issue Feb 27, 2024
Closed
@jimklimov jimklimov mentioned this issue May 15, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug DDL HCL Incorrect or missing readings On some devices driver-reported values are systemically off (e.g. x10, x0.1, const+Value, etc.) Qx protocol driver Driver based on Megatec Q<number> such as new nutdrv_qx, or obsoleted blazer and some others Shutdowns and overrides and battery level triggers Issues and PRs about system shutdown, especially if battery charge/runtime remaining is involved
Projects
NUT HCL/DDL
Status: Todo
Milestone
2.8.1
Development

No branches or pull requests
6 participants
@jimklimov @anphsw @mateuszdrab @EmersonHeiderich @baerenwaldfreund @y0g33