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

[Arjen de Korte]

Arnaud Quette 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().
> 
> 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.

> It results in an immediate UPS poweroff...

In that case, you must have overridden the 'offdelay' in ups.conf.

> The same applies to
> shutdown.return. And shutdown.stop should clear both shutdown and
> start delay to be sure.

Why?

> The reason I previously kept the subdrv delegation (apart from the
> lack of visibility on other mfrs implementation) was the same as Arjen
> comment: the var. and cmd dealing with shutdown clearly have to be
> reworked since we have not a 1 to 1 mapping here.
> 
> This has made its way into 2.2.1 only (r1127) and the trunk (since r1113).
> 
> The fix will obviously have to go into 2.2.2, and the best would be to
> keep the generic method (not subdrv delegation).
> 
> So we have 2 possibilities (non exclusive. We may apply (1) quickly,
> while waiting for (2))
> 1) change upsdrv_shutdown() to proceed with setvar instead of (or
> with) the current instcmd

I'm against that. The variables that command these timers are clearly
instcmd's, not variable settings.

> 2) modify the hid2nut mapping to better deal with the shutdown related
> vars and commands.

If you don't want to use the 'ondelay' or 'offdelay' values, we can also
choose to ignore these. But this will also mean that we need to change
some manual pages.

> Can you (all subdrivers maintainers) please ack this, make some tests
> on your side and report back your feeling about that.

It works for me (with the default settings) so if it doesn't work for
you, what are the contents of 'ups.conf'?

Best regards, Arjen