Skip to content

Conversation

@com6056
Copy link

@com6056 com6056 commented Jan 7, 2026

Fixes #3116 which is caused by a slight difference in behavior when NUT_DEBUG_LEVEL is set, see full explanation here: #3116 (comment)

When running with these changes, I no longer see the disconnects after a couple minutes with my CP1500PFCLCDa 🎉

General points

  • Described the changes in the PR submission or a separate issue, e.g.
    known published or discovered protocols, applicable hardware (expected
    compatible and actually tested/developed against), limitations, etc.

  • There may be multiple commits in the PR, aligned and commented with
    a functional change. Notably, coding style changes better belong in a
    separate PR, but certainly in a dedicated commit to simplify reviews
    of "real" changes in the other commits. Similarly for typo fixes in
    comments or text documents.

  • Please star NUT on GitHub, this helps with sponsorships! ;)

Frequent "underwater rocks" for driver addition/update PRs

  • Revised existing driver families and added a sub-driver if applicable
    (nutdrv_qx, usbhid-ups...) or added a brand new driver in the other
    case.

  • Did not extend obsoleted drivers with new hardware support features
    (notably blazer and other single-device family drivers for Qx protocols,
    except the new nutdrv_qx which should cover them all).

  • For updated existing device drivers, bumped the DRIVER_VERSION macro
    or its equivalent.

  • For USB devices (HID or not), revised that the driver uses unique
    VID/PID combinations, or raised discussions when this is not the case
    (several vendors do use same interface chips for unrelated protocols).

  • For new USB devices, built and committed the changes for the
    scripts/upower/95-upower-hid.hwdb file

  • Proposed NUT data mapping is aligned with existing docs/nut-names.txt
    file. If the device exposes useful data points not listed in the file, the
    experimental.* namespace can be used as documented there, and discussion
    should be raised on the NUT Developers mailing list to standardize the new
    concept.

  • Updated data/driver.list.in if applicable (new tested device info)

Frequent "underwater rocks" for general C code PRs

  • Did not "blindly assume" default integer type sizes and value ranges,
    structure layout and alignment in memory, endianness (layout of bytes and
    bits in memory for multi-byte numeric types), or use of generic int where
    language or libraries dictate the use of size_t (or ssize_t sometimes).
  • Progress and errors are handled with upsdebugx(), upslogx(),
    fatalx() and related methods, not with direct printf() or exit().
    Similarly, NUT helpers are used for error-checked memory allocation and
    string operations (except where customized error handling is needed,
    such as unlocking device ports, etc.)

  • Coding style (including whitespace for indentations) follows precedent
    in the code of the file, and examples/guide in docs/developers.txt file.

  • For newly added files, the Makefile.am recipes were updated and the
    make distcheck target passes.

General documentation updates

  • Updated docs/acknowledgements.txt (for vendor-backed device support)

  • Added or updated manual page information in docs/man/*.txt files
    and corresponding recipe lists in docs/man/Makefile.am for new pages

  • Passed make spellcheck, updated spell-checking dictionary in the
    docs/nut.dict file if needed (did not remove any words -- the make
    rule printout in case of changes suggests how to maintain it).

Additional work may be needed after posting this PR

  • Propose a PR for NUT DDL with detailed device data dumps from tests
    against real hardware (the more models, the better).

  • Address NUT CI farm build failures for the PR: testing on numerous
    platforms and toolkits can expose issues not seen on just one system.

  • Revise suggestions from LGTM.COM analysis about "new issues" with
    the changed codebase.

@com6056 com6056 changed the title attempt to fix cyberpower issues fixed by debug logging fix cyberpower issues resolved when debug logging is enabled Jan 7, 2026
@AppVeyorBot
Copy link

@jimklimov jimklimov added USB Connection stability issues Issues about driver<->device and/or networked connections (upsd<->upsmon...) going AWOL over time labels Jan 7, 2026
@jimklimov jimklimov added this to the 2.8.5 milestone Jan 7, 2026
@jimklimov
Copy link
Member

jimklimov commented Jan 7, 2026

Interesting catch, just shy of two decades old. I see this PR also adds explorehidall flag to usbhid-ups - can you please add documentation to the man page (and a mention in NEWS.adoc) about it please?

Also, I think its default value should be just zero, a VAR_FLAG mention sets it to 1 but otherwise there is no way to set it to 0. So it is not un-settable on CPS devices where this could cause a problem, and it remains -1 (so true in if clauses) by default on others. Maybe better redefine it as a VAR_VALUE and parse as zero/nonzero.

Also not sure if fflush(stdout) is needed there (at least nowadays, upsdebugx outputs are on stderr), and if either flush should be fenced by if (should_output_debug) {...} or remain unconditional.

@jimklimov jimklimov changed the title fix cyberpower issues resolved when debug logging is enabled usbhid-ups: fix cyberpower issues resolved when debug logging is enabled Jan 7, 2026
…workupstools#3247]

...so it can be toggled both ways (e.g. roll back the fix for CPS
if it breaks for someone), and because default "-1" is boolean true.

Signed-off-by: Jim Klimov <[email protected]>
…` to `fetchallhid` [networkupstools#3247]

The "explorehid" term has other connotations for the driver,
better not confuse people.

Signed-off-by: Jim Klimov <[email protected]>
@jimklimov
Copy link
Member

Regarding the disconnections you saw, are there any logs from around then - why did the driver decide to reconnect? Maybe the "correct" solution is to ensure the cps-hid.c subdriver polls certain data points that is currently lacks in the mapping table?

@com6056
Copy link
Author

com6056 commented Jan 10, 2026

Sorry for the delay, this actually didn't solve the issue. I've done some further digging and think I've found another potential solution though 🤞 Will push up the new changes when I can!
UPDATE: See #3259

@jimklimov
Copy link
Member

Aha... and here I was, bringing this PR into better shape :)

For the next one, please post from a dedicated feature branch (not an existing one like master) to simplify resync later on.

@jimklimov jimklimov marked this pull request as draft January 10, 2026 15:59
@com6056 com6056 closed this Jan 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Connection stability issues Issues about driver<->device and/or networked connections (upsd<->upsmon...) going AWOL over time CyberPower (CPS) USB

Projects

None yet

Development

Successfully merging this pull request may close these issues.

CyberPower CP1500AVRLCD3 doesn't reliably work unless NUT_DEBUG_LEVEL set

3 participants