I noticed that includes/discovery/sensors/current/apc.inc.php tests for the presence of atsConfigPhaseTableIndex to decide if it has APC PowerNet-MIB ATS snmp objects:
It’s a one line change but there’s probably a more correct way to do it and I don’t want to maintain a local modification, plus maybe this would help other people?
The question here, is to ensure that a change here would not break polling/discovery for other devices … This is the tricky part with those “funny” snmp implementations sometimes.
Surely. There is probably a better way to achieve this, hopefully without piling on lists of one-off tests and quirks. But this is already broken, in the sense that atsConfigPhaseTableIndex is not really the right object to test for whether a device is an Automatic Transfer Switch - not even most ATSs have this object, certainly none of the single-phase ones at any rate. This is a large set of devices that can never be discovered except as generic devices.
Another nms product tests for atsInputPhaseTable and atsInputTable as well as some others to make this test, so I wonder if it might make sense here to just expand the test to be more inclusive.
IMO if something will be probed wrong, it’s better to fail by looking for objects that aren’t present (a situation that librenms handles very well) rather than to fail by librenms not being able to recognize a device.