[aquette]

The generic MIB-2 provides system contact and location information that are now
publish respectively as device.contact and device.location

[jimklimov]

Seems reasonable, except I don't like hardcoded things hidden in the code, so I'd suggest to at least replace the explicit OIDs with macros defined at some acceptable location, or even do proper mappings for this.

[aquette]

removing the milestone, since the tech solution is not clear enough for a short term implementation

[jimklimov]

PR updated to address comments above: now each existing *-mib.c file defines device.contact, device.location and device.description (currently initialized to IETF OIDs for those, can be tuned to vendor values if ever needed) as RW STRING entries. Tested locally that writes (upsrw to an Eaton ePDU) actually work.

As a fallback, if a (future/custom) MIB table does not define any of those names, the snmp-ups.c would patch those entries (as read-only) similarly to original proposal. This was also verified, by commenting away the 3 lines from eaton mib.c file.

While testing, found a bug in su_setOID() which got it to assume anything starting with "device" is part of a daisy-chain (the tested ePDU in fact was a "slave" part of a chain, but unlike many other places the "device" string is handled, here we did not test for template-related variables and would mis-classify the settings in any case).

Also found a bug with tracking id sanity checks (precluded upsrw -w results-reporting), fix pushed separately.

[jimklimov]

Thanks to the last few commits, values like "device.contact" are now exposed once per device or daisy chain (for upsc queries), and can be written landing to daisy master (for upsrw) at least for non-templated OIDs (all of those we have, are from IETF MIB not aware about daisies).

Checked with a daisy-chain master ePDU and daisy-chain slave ePDU (both with "eaton_epdu" MIB), and an Eaton 9PX 5000i UPS with ietf, mge, pw and pxgx_ups MIBs (all they use IETF MIB values for these fields though).

Side note: with the UPS probed by 4 drivers at the same time, the one through which I had set the contact info did instantly report it back with upsc; for others it took until next big refresh (half a minute or so) to reflect the change.

[jimklimov]

Back to drawing board :\

While testing, I checked that device.mfr was successfully exposed for the many devices in a chain, but missed that it was a NULL OID too. A templated value like device.model seems exposed once (like device.contact) although the OID string has .%i...

[jimklimov]

Good news (to post a bit later) is that we can discern in su_ups_get() of all places, whether the original OID was templated (methods like su_setinfo() in fact get the specific string from there). This allows to short-circuit assignment of non-templated "device.*" entries in a daisychain to e.g. "device.1.*" specifically, and so lose meaningless dupes (contact, location, macaddr...) attributed to all devices but taken from the first in reality.

Bad news is that setting the value (upsrw) becomes very problematic, because su_find_info() can't locate e.g. device.1.contact that is not actually in the snmp_info table.

For the location/contact/description in particular, daisychain seems to preclude detection of the values at "root" level (device.contact etc.) so it is populated by fallback (read-only IETF) for the master PDU device.

[jimklimov]

For some reason, getting ALL bits to work, or be usable (e.g. both able to set daisy master and daisy slave and standalone devices) is a fine balancing act. What's more odd, at some point snmp-ups started for daisy devices (master or slave) with debug_min 1 or unspecified, fails to send data to upsd; with debug level 2 or more, or for a stand-alone UPS, it works; USB driver showed no such problem. Why didn't I take the other pill?

[jimklimov]

With recent commits, behaviors can be deemed achieving the feature (somewhat clumsily but logically) unless proven otherwise, and are as follows:

• single non-chained device exposes (upsc) and can set (upsrw) the device.contact, device.location and device.description
  ** for a UPS tested with 4 different MIBs (mge, pw, pxgx_ups, ietf) there were no lost data points and added these 3, comparing to upsc dumps made with current master branch builds
• single directly-accessible daisy chain slave device also exposes (upsc) and can set (upsrw) the device.contact, device.location and device.description (but currently seems to suffer with lowered debugging levels; maybe an issue on recent master branch though)
• the daisy-chain master exposes two sets of entries (and also currently seems to suffer with lowered debugging levels):
  ** device.contact, device.location and device.description are "fallback IETF-MIB" (read-only, read-once)
  ** indexed device.1.contact, device.1.location and device.1.description are readable and writeable, pointing to the daisy master device
  ** spurious device.{2,3...}.contact, etc. entries backed by non-templated OID definitions and in fact showing the master-device info (so also device.n.macaddr coming from IETF subtree) are not exposed anymore
  ** device.0.fieldname can not be used to read/set these entries

Along this journey, added ./tools/nut-dumpdiff.sh to help compare upsc or driver-discovery outputs (discarding variations in numeric measurements), e.g.:

# ./tools/nut-dumpdiff.sh /tmp/daisyMaster-master.txt /tmp/daisyMaster-pr4.txt
=== ./tools/nut-dumpdiff.sh: comparing '/tmp/daisyMaster-master.txt' (-) vs '/tmp/daisyMaster-pr4.txt' (+)
+Network UPS Tools - Generic SNMP UPS driver 1.19 (2.7.4-4727-ga21f32f)
+device.1.contact: 42ITy Team
+device.1.description: EPDU MA 0U (C14 10A 1P)16XC13
+device.1.location: SRV Room, 42ITy Rack
-device.2.macaddr: 00 20 85 F4 61 BE
-device.3.macaddr: 00 20 85 F4 61 BE
+device.contact: 42ITy Team
+device.description: EPDU MA 0U (C14 10A 1P)16XC13
+device.location: SRV Room, 42ITy Rack
-driver.version.data: eaton_epdu MIB 0.58
-driver.version.internal: 1.18
-driver.version: 2.7.4-4696-g763e03c
+driver.version.data: eaton_epdu MIB 0.59
+driver.version.internal: 1.19
+driver.version: 2.7.4-4727-ga21f32f

[jimklimov]

Presumably, for the data points in original question (contact, location, description) the quest is solved :)

The more generic issue of non-templated OIDs used in daisy-chain context for many other if-else code branches seen but not trampled (outlets, alarms, *.alarm, commands, etc.) probably is not consistently addressed, but I have no good ways or ideas to even test for that.

The best I can say about it currently is that "symptomatically" by the posted PR codebase state (took a week to get here with all the back and forth), there were no issues seen like lost data points or spurious new ones, when tested against a single UPS with 4 MIBs, a daisy master ePDU and a directly addressable daisy slave ePDU of same model; but I have no firm idea if there are things I missed going just by symptoms.

[aquette]

scripts/subdriver/gen-snmp-subdriver.sh should be amended too

[jimklimov]

At least, as of https://ci.networkupstools.org/job/nut/job/nut/job/PR-320/47/ the NUT CI farm liked this codebase per se, across the board :)

[jimklimov]

Great catch, apparently I kept forgetting about the snmp/usb generator scripts in a few other PRs and had to post-process later :\

[jimklimov]

After discussion with @aquette this is good to merge.

Experimentally the PR did not add regressions and did add features; the rest can be iterated later if found lacking: e.g. where I "fake" a device.1 for non-templated stuff, so far that's for generic int/str data - can be done similarly (but is not currently), for earlier bifurcations about alarms etc.