Skip to content

PyNUT: use command identifiers in description requests - #3628

Open
user01010111 wants to merge 5 commits into
networkupstools:masterfrom
user01010111:codex/fix-3621-command-descriptions
Open

PyNUT: use command identifiers in description requests#3628
user01010111 wants to merge 5 commits into
networkupstools:masterfrom
user01010111:codex/fix-3621-command-descriptions

Conversation

@user01010111

Copy link
Copy Markdown

Closes: #3621

On Python 3, GetUPSCommands() formats command names parsed from LIST CMD
as bytes representations, sending requests such as GET CMDDESC dummy b'load.off'. Current upsd consequently returns "Description unavailable"
even though load.off has a description.

Decode the command identifier as ASCII for both the description request and
the response-prefix length. The returned dictionary retains its bytes keys
and values, and description errors retain the existing identifier fallback.
Add a request-sensitive regression test under tests, run it through
make check, and record the fix in NEWS.adoc.

Validation:

  • The regression fails on unmodified Python 3 source and on a request-only
    fix; the complete fix passes on Python 2.6.9, 2.7.18, and 3.4 through 3.14.
  • A fresh localhost upsd with dummy-ups advertises load.off. Against the
    same server, baseline returns "Description unavailable" and the patched
    client returns "Turn off the load immediately". Exact request bytes and
    separate unavailable/error controls were checked; no UPS commands ran.
  • Generated-module, explicit-path install/uninstall, wheel-install and
    source-distribution-install checks passed. The existing suite passed
    12/12, plus all six new regression tests.
  • make spellcheck, changed-line ASCII/whitespace checks, and normal
    make distcheck-light passed. The latter exercised the extracted archive's
    build, tests, installation, uninstallation and redistribution, using its
    standard placeholder handling for unselected manual pages.

No physical UPS hardware was available for testing. Validation covered
request-sensitive transport tests, Python runtime compatibility checks,
generated and packaged modules, build and distribution checks, and simulated
localhost integration with upsd and dummy-ups. Physical UPS models, firmware
versions, and hardware combinations were not tested.

AI assistance: OpenAI Codex with gpt-6-astra (x-high reasoning).
The human contributor remains responsible for reviewing and submitting
the change.

Relevant contribution checklist:

  • Described the concrete behavior, compatibility and testing limitations.
  • Disclosed AI use and model.
  • Followed affected-file style; added source text is ASCII.
  • Registered the new test in distribution metadata; distcheck-light passed.
  • Added a NEWS entry and passed spelling checks.
  • Human contributor has reviewed and accepted responsibility for the patch.
  • DCO sign-off included in each contribution commit.

Decode LIST CMD identifiers as ASCII for GET CMDDESC requests and response
offsets, preserving the public bytes dictionary and description fallback.
Add request-sensitive regression coverage to make check and note the fix
in NEWS.adoc.

Validated with Python 2.6, 2.7 and 3.4 through 3.14, localhost upsd with
dummy-ups, generated/installed/packaged modules, make check, spellcheck
and distcheck-light.

AI assistance: OpenAI Codex with gpt-6-astra (x-high reasoning).
The human contributor remains responsible for reviewing and submitting
the change.

Closes: networkupstools#3621

Signed-off-by: user01010111 <12504630+user01010111@users.noreply.github.com>
@jimklimov jimklimov added python portability We want NUT to build and run everywhere possible Incorrect or missing readings On some devices driver-reported values are systemically off (e.g. x10, x0.1, const+Value, etc.) AI For good or bad, machine tools are upon us. Humans are still the responsible ones. labels Sep 7, 2026
@jimklimov jimklimov added this to the 2.8.6 milestone Sep 7, 2026
@jimklimov

Copy link
Copy Markdown
Member

You are on a roll again, now that the previous PR merge train has left the station, and NUT CI farm is more available to queries :)

@github-actions

github-actions Bot commented Sep 7, 2026

Copy link
Copy Markdown

A ZIP file with standard source tarball and another tarball with pre-built docs for commit ceab8e0 is temporarily available: NUT-tarballs-PR-3628.zip.

@jimklimov jimklimov added this to NUT QA Sep 8, 2026
@jimklimov jimklimov moved this to In Progress in NUT QA Sep 8, 2026
@AppVeyorBot

Copy link
Copy Markdown

Build nut 2.8.5.5250-master completed (commit fe492a87e2 by @user01010111)

@jimklimov

jimklimov commented Sep 9, 2026

Copy link
Copy Markdown
Member

Curious: this failed on just one build combo, for a fairly valid reason:

Traceback (most recent call last):
  File "../../../tests/pynut-commands-test.py", line 5, in <module>
    import unittest
ModuleNotFoundError: No module named 'unittest'
make[5]: *** [Makefile:2508: pynut-commands-test] Error 1

On the CI side, I can add the module so we would test the feature everywhere, but on the project source side - the module should be listed in build dependencies I think (setup.py.in or tox.ini? maybe with some keyword for test-not-runtime dependency?)

And perhaps the presence of the module should be checked in configure.ac and drive decisions in Makefile.am, similar to how Qt5/6 are detected, etc.

  • UPDATE: This change landed into existing scripts/python/module/test_nutclient.py.in, at least in another PR with same problem, so probably the check should be in it, not the autoconf/automake recipes. Just try to import the module, and run/skip test cases based on the outcome. Or mixed, differently handled here and there.

So maybe until this is handled, I would NOT update that build agent, so we would know builds/checks no longer crash because of this.

UPDATE: I'm on it now...

…nd `unittest` module [networkupstools#3628]

Signed-off-by: Jim Klimov <jimklimov+nut@gmail.com>
…ailability of `unittest` module [networkupstools#3628]

Signed-off-by: Jim Klimov <jimklimov+nut@gmail.com>
@AppVeyorBot

Copy link
Copy Markdown

Build nut 2.8.5.5265-master completed (commit b8eb25d79f by @jimklimov)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

AI For good or bad, machine tools are upon us. Humans are still the responsible ones. Incorrect or missing readings On some devices driver-reported values are systemically off (e.g. x10, x0.1, const+Value, etc.) portability We want NUT to build and run everywhere possible python

Projects

Status: In Progress

Development

Successfully merging this pull request may close these issues.

PyNUT GetUPSCommands() requests CMDDESC using Python's bytes representation

3 participants