On 11-01-26 20:28, Arjen de Korte wrote:
Citeren Michal Soltys <[email protected]>:
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.
One of the main points of the patch I sent was to let the user be in
control of the shutdown (in context of apc smart units), basing the
decision /only/ on two polled values: if supported by ups -
battery.charge (I think all apc units in smart can do that, regardless
of their age) and if supported by ups - battery.runtime. User would
supply 'minbatt' and 'mintime' to control the thresholds, that would
cause the LB to be asserted - *ignoring* what ups thinks internally
about LB (as I've found it, over the years, unreliable in many cases) -
thanks to "ignorelb" option.
You suggested to not do it at driver level, but in main.c - using
battery.{runtime,charge}.low - that can be overriden through "override."
prefix (if necessary). You also suggested to use e.g.
override.battery.charge.low = 0, should user want to disable it for some
reason. That could all work fine, except that under current rules - they
can't be overridden, if they can be programmed (rw are not meant to be
immutable). APC units (dunno if all) won't tolerate battery.runtime.low = 0.
Using battery.runtime.low as an example - it's really RO variable during
normal operation, and RW only during eeprom programming (the allowed
values are few discrete settings for apc units, if applicable). ignorelb
+ override.battery.charge.low = 0 (or as in original patch - ignorelb +
minbatt = 0) would disable this check, and let user control the ups
basing the decision only on e.g. battery.charge.
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.
That's why "ignorelb" option was added to apcsmart driver (see above and
original commit message for the rationale) - this allows the control by
polling the charge/runtime, using /only/ user supplied thresholds for LB
decision. ups' internal decision (based on calibration, firmware,
battery age/quality AND battery.runtime.low (if applicable) eeprom
variable) is ignored.
That's why I also originally opted for minbatt/mintime, instead of going
for generic main.c level checks from the start.
- sttmp->flags = flags;
+ sttmp->flags = flags | (sttmp->flags & ST_FLAG_IMMUTABLE);
No. A variable that is R/W can *never* be immutable.
Then, the other options, that is if:
- additional LB checks are to remain in main.c
- new apcsmart's "ignorelb" option to have the desired effect
- the checks are to use well known battery.{runtime,charge}.low, and be
universal at the same time
the likely options are:
- handle this at driver level - e.g. preserve the immutable flag if
already set without setting rw (if necessary, e.g. in apcsmart if
ignorelb is set), ignore polling, adjust the code so it works fine, etc.
- go back to minbatt/mintime
- add e.g. ST_FLAG_USER_OVERRIDE which would be used for any (and only
for) override.* in ups.conf (instead of ST_FLAG_USER_IMMUTABLE), while
not being encumbered by "must not be rw" constraint
The 1st option looks good ?
_______________________________________________
Nut-upsdev mailing list
[email protected]
http://lists.alioth.debian.org/mailman/listinfo/nut-upsdev
