nut icon indicating copy to clipboard operation
nut copied to clipboard

[Feature request] Show units in upsc report

Open Ricc68 opened this issue 3 months ago • 3 comments

Looking at the upsc report it stands out to me that it shows numbers without units: in general numbers without units are meaningless because they lack key information to understand them. It would be a significant improvement in my view if the data emitted by upsc is shown with their units.

Ricc68 avatar Oct 19 '25 21:10 Ricc68

Protocol units for standard values are listed in doc/nut-names.txt, and to an extent should be reported via protocol (STRING, NUMBER, RANGE etc.) although I think various authors were relaxed about the latter (so most things are opaque strings by default) and there's a backlogged issue about rectifying that. This one is unlikely to be addressed before the basics like that are reliably covered (essentially it is about reporting a secondary type like NUMBER&&SECONDS). Emission of that might be under an optional key, if there isn't one already.

jimklimov avatar Oct 19 '25 21:10 jimklimov

As a bit of a follow-up and refresher of my memory, data/cmdvartab file in NUT sources does contain "layman descriptions" of various standard data point and command names, often including units of measurement in that text. Since many values are proportions, a single word for such unit as "percent" would not cut it anyway without the context "percentage of what (load, charge, capacity, time/age etc.)?"

This file is handled (and data eventually served) by upsd via server/desc.c logic like desc_get_var(), called from server/netget.c methods like get_cmddesc() or get_desc() (for VARs), near the get_type() for the aspect noted in earlier post, responding to protocol queries like DESC. If an entry is missing in that file (e.g. unmapped.* or experimental.* variable is encountered), it would not be served. Likewise if the whole file is not installed on the server side.

I see client code upsrw.c and upsset.c (CGI) calling that, so you can e.g. upsrw -l myups to see the descriptions (at least of settable values). On a side note, oddly, the Python module and GUI client don't deem to mention it though.

Anyhow, upsc.c starts with the program description as upsc - simple "client" to test communications so overloading it with features may be not the right way to go, since there are already other tools that do the job. But there may be benefit in having a one-stop answer to such matters, so maybe an additional mode to query also for descriptions and report them would make sense. But I guess it would still be semi-comfortable, as long as we try to uphold some standards (e.g. adding a desc.* or vardesc.* namespace into the reports, so alphanumerically sorted lines with descriptions and with actual data would be somewhat far apart. But at least they could be seen simultaneously. Maybe even a complex sort of namespace to cover different types of metadata we have, e.g. vardesc.<varname>.text, vardesc.<varname>.type, vardesc.<varname>.maxlen etc.?..

Note: There was a long-standing idea about adding some modern machine-parsing friendly output options like JSON and/or YAML, there the numerous metadata like those seen in upsrw outputs could make sense as part of each object which represents any reported data point. But so far this did not come to being realized (or even backlogged, I think) since the area is somewhat covered by third-party clients dealing with REST API representation, etc.

TLDR summary: I am not likely to personally devote attention to this area anytime soon, there are more pressing matters in the backlog and out of thin air, but PRs would be welcome. Not sure if all such extra features should complicate upsc as such by adding options and staying (or not?) in its backwards-compatible constraints, or start another client (maybe initially as a copy of upsc adapted until relationship is barely recognizable) with the extra features and output formats arranged in a more convenient way without breaking backwards compatibility for tools that expect certain sorted key-value output of upsc.

jimklimov avatar Oct 20 '25 06:10 jimklimov

Just by quickly searching the code, I see:

/* extended variable table - used for -x defines/flags / typedef struct vartab_s { int vartype; / VAR_* value, below */ char var; / left side of =, or whole word if none */ char val; / right side of = */ char desc; / 40 character description for -h text / int found; / set once encountered, for testvar() / int reloadable; / driver reload may redefine this value */ struct vartab_s *next; } vartab_t;

Am I correct that this is the definition of the variable? It should start from the root, the variable definition should ideally contain also it's unit, no?

Like in the code we currently have:

snprintf(temp, sizeof(temp), "Set low battery level, in %% (default=%s)", DEFAULT_LOWBATT); addvar (VAR_VALUE, HU_VAR_LOWBATT, temp);

It should be something like(for example) addvar_u (VAR_VALUE, HU_VAR_LOWBATT, UN_VAR_PERCENT, temp); where: #define UN_VAR_PERCENT 0x08

and the unit is saved in the vartab structure and used by everyone willing to access that variable.

This would not necessarily be a breaking change both in the structure and in eventual new functions addvar_u and do_addvar_u which could be used instead of the current addvar* functions.

Hah, enough speculation: it seems something unlikely to happen. I may even volunteer to write the framework but then the migration would require years and willingness of the developers.

Ricc68 avatar Oct 20 '25 15:10 Ricc68