[Nut-upsdev] Important regression in usbhid-ups (r1113)

[Arjen de Korte]

Arjen de Korte wrote:

>> while finishing some work on the HAL integration (I'm now mapping DBus
>> method with NUT commands, to allow at least the final UPS poweroff), I
>> was horrified to realize (and so late) the changes in
>> usbhid-ups->upsdrv_shutdown().

Those changes are for the good (the upsdrv_shutdown() function should
now work as intended).

>> We've lost a lot there, but making a generic code, instead of keeping
>> the subdrivers delegation.
>
> We haven't lost anything here. Instead, we now follow what is
> documented. On shutdown, we will use the ondelay and offdelay settings
> (as advertised in the manpage).
> 
>> For example, the shutdown.return command (which is or should be the
>> standard UPS poweroff method) has to set both delay.shutdown *and*
>> delay.start does only set the latter.
> 
> No, it sets both. First it writes the 'ondelay' time and if that is
> successful, it proceeds to write the 'offdelay' time. If both are
> successful (and ondelay > offdelay), you'll get an orderly shutdown.

The fingers were quicker than the mind. We need to distinguish two
things here:

1) The shutdown.(return|stop) functions currently do not conform to the
NUT standard for the reasons you already mentioned. Both need sending
two commands to the UPS and the current 1:1 mapping doesn't allow that.

2) The upsdrv_shutdown() function. As far as I can see, this does what
is expected from it. See above (this is where I was mistaken).
Previously, some drivers didn't conform to the 'man 8 usbhid-ups' page,
which was corrected in r1113.

We should probably remove the commands from 1) since these don't conform
to the NUT standards. Internally (in 'upsdrv_shutdown()') we can also
use the 'load.on' and 'load.off' commands, which probably also are more
clear to understand and also allow setting a delay value now.

Best regards, Arjen