[Nut-upsdev] [PATCH/RFC 1/1] APC smart driver update and new features.

[Arjen de Korte]

Citeren Michal Soltys <soltys at ziu.info>:

> The point is about override.* values, as they are implemented  
> through immutable flag. If I added for example
>
> [apctest]
> 	override.battery.minutes.low = 0
> or
> 	override.battery.minutes.low = -1

How is this supposed to work? I can imagine shutting down earlier than  
whatever buildin runtime is configured in the UPS, but not later. If  
the UPS uses battery.runtime.low=120, there is no point overriding  
this with a lower value, because when the buildin value is reached,  
the UPS will already set the LB flag. We use battery.runtime(.low) by  
the way.

> Then
>
> 1) config parser would interpret and set the immutable flag on  
> battery.minutes.low
> 2) apc driver during capabiliity detection would find it and set RW  
> flag, as the value can be read and also programmed in eeprom (few  
> discrete values in case of that particular apc unit)

As already explained, this is not meant for R/W values.

> When I added checks for asserting LB state basing on runtime/battery  
> left, this of course created conflict - as the driver removed the  
> immutable flag and happily kept polling the value - so 0 or any  
> other value had no effect whatsoever.

If a driver already supports this value, it will also act upon it. If  
you set the immutable flag on this value, NUT might display this, but  
the UPS would still set the LB flag if the battery.runtime would drop  
below the internal representation of battery.runtime.low.

> In the code, the only meaning of immutable flag - from what I saw -  
> is simply that dstate_setinfo() refuses to change it:
>
>                 /* changes should be ignored */
>                 if (node->flags & ST_FLAG_IMMUTABLE) {
>                         return 0;       /* no change */
>                 }

Indeed. But if a variable is R/W in the UPS, it is nonsense to ignore  
it. The UPS will accept changes to that value so there is no point in  
ignoring it.

> It doesn't conflict in any way with ability to write or read it  
> (to/from the actual ups unit)

And that is exactly what was intended by this flag.

> Either way, simple fix with which everything works fine :
>
> -       sttmp->flags = flags;
> +       sttmp->flags = flags | (sttmp->flags & ST_FLAG_IMMUTABLE);

No. A variable that is R/W can *never* be immutable.

Best regards, Arjen
-- 
Please keep list traffic on the list (off-list replies will be rejected)