Prepare the code for proper handling of override.* variables. APC_USERCTRL - allows permanent overriding of the variable. Meaning even if the ups supports reading and/or writing it, the driver will not allow it (actually it will behave as if the variable wasn't supported at all). This guarantees that the value set by the user in the configuration file is respected.
Currently only battery.runtime.low overriding is allowed, which is necessary for proper functioning of ignorelb (if user decided to override that variable). APC_CRUCIAL - this flag enforces checking if the variable is actually supported. Additionallly - the driver will ask user about reporting UPS models, which don't support command querying, but do support firmware query - in the other words - models which are not present in the compatibility table, but should be included there. oldapcsetup() also checks for presence of battery.charge now. Signed-off-by: Michal Soltys <[email protected]> --- drivers/apcsmart.c | 56 +++++++++++++++++++++++++++++++++++++++++++-------- drivers/apcsmart.h | 17 ++++++++++----- 2 files changed, 58 insertions(+), 15 deletions(-) diff --git a/drivers/apcsmart.c b/drivers/apcsmart.c index ef66fce..bb8d394 100644 --- a/drivers/apcsmart.c +++ b/drivers/apcsmart.c @@ -388,7 +388,14 @@ static void do_capabilities(void) entptr = &ptr[4]; vt = vartab_lookup_char(cmd); - valid = vt && ((loc == upsloc) || (loc == '4')); + /* + * all the capabilities must first pass protocol_verify() + * tests; some of the variables are allowed to be overriden by + * a user, and we must not refresh or write them; after + * earlier protocol verification, APC_PRESENT will not be set + * on such variable + */ + valid = vt && ((loc == upsloc) || (loc == '4')) && (vt->flags & APC_PRESENT); /* mark this as writable */ if (valid) { @@ -464,6 +471,7 @@ static void oldapcsetup(void) query_ups("ups.serial", 1); query_ups("input.voltage", 1); /* This one may fail... no problem */ + query_ups("battery.charge", 1); /* lots of upses support it, also necessary for ignorelb */ update_status(); @@ -474,7 +482,7 @@ static void oldapcsetup(void) static void protocol_verify(unsigned char cmd) { - int i, found; + int i, found, flags; /* we might not care about this one */ if (strchr(CMD_IGN_CHARS, cmd)) @@ -489,7 +497,27 @@ static void protocol_verify(unsigned char cmd) upsdebugx(3, "UPS supports variable [%s]", apc_vartab[i].name); - /* load initial data */ + if ((flags = dstate_getflags(apc_vartab[i].name)) >= 0) { + /* + * variable has been defined at ups.conf level, + * check if it's of an override.* kind and + * overriding is allowed as per APC_USERCTRL + * flag; if so - return without setting + * APC_PRESENT + */ + if ((apc_vartab[i].flags & APC_USERCTRL) && (flags & ST_FLAG_IMMUTABLE)) + return; + } + + /* be extra careful about the presence of certain + * variables */ + if ((apc_vartab[i].flags & APC_CRUCIAL) && !query_ups(apc_vartab[i].name, 1)) { + upsdebugx(1, "Your UPS doesn't actually support variable [%s]", apc_vartab[i].name); + upsdebugx(1, "Please report this error"); + return; + } + + /* mark as present, load initial data */ apc_vartab[i].flags |= APC_PRESENT; poll_data(&apc_vartab[i]); @@ -553,7 +581,7 @@ static int firmware_table_lookup(void) ret = ser_get_line(upsfd, buf, sizeof(buf), ENDCHAR, IGNCHARS, SER_WAIT_SEC, SER_WAIT_USEC); - /* + /* * Some UPSes support both 'V' and 'b'. As 'b' doesn't always return * firmware version, we attempt that only if 'V' doesn't work. */ @@ -599,25 +627,26 @@ static int firmware_table_lookup(void) for (j = 0; j < strlen(compat_tab[i].cmdchars); j++) protocol_verify(compat_tab[i].cmdchars[j]); - return 1; /* matched */ + return 2; /* matched */ } } upsdebugx(2, "Not found in table - trying normal method"); - return 0; + return 1; } static void getbaseinfo(void) { unsigned int i; - int ret = 0; + int ret, retfw; char *alrts, *cmds, temp[512]; /* * try firmware lookup first; we could start with 'a', but older models * sometimes return other things than a command set */ - if (firmware_table_lookup() == 1) + retfw = firmware_table_lookup(); + if (retfw == 2) return; upsdebugx(1, "APC - Attempting to find command set"); @@ -637,7 +666,16 @@ static void getbaseinfo(void) SER_WAIT_SEC, SER_WAIT_USEC); if ((ret < 1) || (!strcmp(temp, "NA"))) { - /* We have an old dumb UPS - go to specific code for old stuff */ + /* + * We have either an old dumb UPS, or there's no info in the + * compatibility table. Go to specific code for the old stuff. + */ + if (retfw == 1) { + upsdebugx(1, "Your UPS supports firmware querying, but is unable to report its command set."); + upsdebugx(1, "It also wasn't found in the compatibility table - please consider"); + upsdebugx(1, "reporting its firmware version and verifying which parts of smart"); + upsdebugx(1, "protocol it supports."); + } oldapcsetup(); return; } diff --git a/drivers/apcsmart.h b/drivers/apcsmart.h index 939b6cb..c85f76b 100644 --- a/drivers/apcsmart.h +++ b/drivers/apcsmart.h @@ -78,8 +78,10 @@ /* Driver command table flag values */ -#define APC_POLL 0x0001 /* Poll this variable regularly */ -#define APC_PRESENT 0x0004 /* Capability seen on this UPS */ +#define APC_POLL 0x0001 /* Poll this variable regularly, if present */ +#define APC_CRUCIAL 0x0002 /* "crucial" variable, always check if it's present */ +#define APC_PRESENT 0x0004 /* presence verified - command can be polled / executed */ +#define APC_USERCTRL 0x0008 /* variable's readout value can be controlled by a user */ #define APC_RW 0x0010 /* read-write variable */ #define APC_ENUM 0x0020 /* enumerated type */ @@ -113,8 +115,8 @@ apc_vartab_t apc_vartab[] = { { "ups.firmware.old", 0, 'V' }, { "ups.firmware", 0, 'b' }, { "ups.firmware.aux", 0, 'v' }, - { "ups.model", 0, 0x01 }, + { "ups.model", 0, 0x01 }, { "ups.serial", 0, 'n' }, { "ups.mfr.date", 0, 'm' }, @@ -169,7 +171,8 @@ apc_vartab_t apc_vartab[] = { { "battery.date", APC_STRING, 'x' }, - { "battery.charge", APC_POLL|APC_F_PERCENT, 'f' }, + { "battery.charge", APC_POLL|APC_F_PERCENT| + APC_CRUCIAL, 'f' }, { "battery.charge.restart", APC_F_PERCENT, 'e' }, @@ -177,9 +180,11 @@ apc_vartab_t apc_vartab[] = { { "battery.voltage.nominal", 0, 'g' }, - { "battery.runtime", APC_POLL|APC_F_MINUTES, 'j' }, + { "battery.runtime", APC_POLL|APC_F_MINUTES| + APC_CRUCIAL, 'j' }, { "battery.runtime.low", - APC_F_MINUTES, 'q' }, + APC_F_MINUTES|APC_USERCTRL, + 'q' }, { "battery.packs", APC_F_DEC, '>' }, { "battery.packs.bad", APC_F_DEC, '<' }, -- 1.7.2.1 _______________________________________________ Nut-upsdev mailing list [email protected] http://lists.alioth.debian.org/mailman/listinfo/nut-upsdev
