On 2015/03/28 20:02, Charles Lepple wrote: > On Mar 25, 2015, at 9:21 PM, Stuart Henderson <[email protected]> wrote: > > > Hi, the diff inline below adds a new subdriver for snmp-ups to support > > Huawei UPS, based on an observed walk from a UPS5000-E with a few bits > > filled in from the MIBs (copy at http://junkpile.org/HUAWEI_UPS_MIB/). > > Sample output from upsc follows the diff. > > Hi Stuart, > > Thanks for the patch. It builds cleanly for me, so I have no problems > merging it. > > The only two things that jump out at me could probably be addressed > with documentation. Is there a low battery alarm? If not, we should > probably mention somewhere that people will need to either synthesize > a LB status with one or both of 'override.battery.charge.low' and > 'override.battery.runtime.low'.
I didn't notice anything that looks like a low battery alarm in the Huawei MIB, though it does also return a value in UPS-MIB::upsBatteryStatus.0 so perhaps a LB alarm will be signalled there; I see that in mge-mib.c the status appears to be generated from a combination of ietf and private MIB but there is a comment "FIXME: the below may introduce status redundancy, that needs to be * adressed by the driver, as for usbhid-ups!" so I wasn't sure the best approach here. Unfortunately it isn't really possible to test low battery in my environment, I'd need to override the generator autostart/ATS which I don't think I'd get approval for (my main aim with this UPS is to use NUT to act as an abstraction layer so that it can be monitored alongside a couple of more "normal" UPS, rather than specifically to trigger shutdowns). > (Or they can shut down when it first > transfers to battery, but this sounds like the sort of UPS that would > keep going for a while.) Indeed, it is a bit larger than anything I've dealt with before, you may have noticed the battery V in my sample output ;-) > There also seem to be a few commands in the MIB, but none are > implemented here. The only ones that people might be expecting are the > shutdown.reboot and shutdown.stayoff, although I haven't used the SNMP > driver to see how well those work in general. Not strictly necessary > to implement, but we should probably take this opportunity to point > out in the snmp-ups man page that if there are no shutdown.* commands > listed in the upscmd output, there is a potential for a race condition > if the power comes back early. There's a hwUpsCtrlPowerOff command and a RW hwUpsCtrlPowerOffDelay variable, but the mib isn't clear about whether this would be "shutdown.stayoff" or "shutdown.return", I'll see if I can find more information about this in the manuals. I don't see a power-on delay in the vendor mib so I think shutdown.reboot may not be possible. There is also a battery test command, which would be a bit easier for me to test, I'll try and look at that sometime. > Arnaud, as maintainer of snmp-ups, any thoughts here? Thanks for the review Charles, and I'd be interested to hear any comments from Arnaud. Stuart _______________________________________________ Nut-upsdev mailing list [email protected] http://lists.alioth.debian.org/cgi-bin/mailman/listinfo/nut-upsdev
