Instcmd setvar brushup

Makefile.am

@@ -157,7 +157,12 @@ EXTRA_DIST = LICENSE-GPL2 LICENSE-GPL3 LICENSE-DCO MAINTAINERS
 # Since the renaming of documentation to `*.adoc` extension to help IDE
 # and GitHub UIs to render the source files in a pretty fashion, we need
 # to list them:
-EXTRA_DIST += INSTALL.nut.adoc UPGRADING.adoc TODO.adoc NEWS.adoc README.adoc ci_build.adoc
+EXTRA_DIST += INSTALL.nut.adoc UPGRADING.adoc TODO.adoc NEWS.adoc README.adoc
+
+# The document is now part of qa-guide; the script might be a bit git-oriented,
+# but can be useful in builds from tarball, probably. Anyhow, having the doc
+# without the tool which it documents is odd.
+EXTRA_DIST += ci_build.sh ci_build.adoc
 
 # Tarballs created by `make dist` include the `configure.ac` and `m4/*` sources
 # but lack NUT magic logic to recreate the `configure` script if someone would

NEWS.adoc

@@ -88,6 +88,12 @@ https://github.com/networkupstools/nut/milestone/9
      does not care where the token itself was raised for its notifications.
      Driver-code related test-cases were updated to reflect these changes.
      [issue #2928, PRs #2931 and #2934]
+   * Introduced some macros in `drivers/upshandler.h` for common syslog level
+     definitions and message wording for beginning and failing `instcmd()` or
+     `setvar()` operations consistently in different drivers. As a related
+     change, operations that intend to turn off or restart the load, or can
+     do that by side effect (e.g. calibration if batteries are old or dead),
+     would explicitly `upslogx(LOG_CRIT,...)` by default before commencing.
 
  - `dummy-ups` driver updates:
    * A new instruction `ALARM` was added for the `Dummy Mode` operation

ci_build.sh

@@ -2323,7 +2323,10 @@ default|default-alldrv|default-alldrv:no-distcheck|default-all-errors|default-sp
 
             # Tack a remaining yes/no in the end to whatever scenario is there;
             # NOT a full matrix
-            if [ "${BUILDSTODO_UNMAPPED}" -gt 1 ] ; then
+            if [ "${BUILDSTODO_UNMAPPED}" -gt 1 ] && \
+            (   [ "$CI_FAILFAST" != "true" ] \
+             || [ "$CI_FAILFAST" = "true" -a "$RES_ALLERRORS" = 0 ] \
+            ) ; then
                 for NUT_UNMAPPED_VARIANT in $NUT_UNMAPPED_VARIANTS ; do
                     case "${NUT_UNMAPPED_VARIANT}" in
                         no) ;;	# we already did the default ("no") implicitly

clients/upsclient.c

@@ -887,11 +887,11 @@ static int upscli_sslinit(UPSCONN_t *ups, int verifycert)
 		upsdebugx(3, "SSL connected (%s)", SSL_get_version(ups->ssl));
 		break;
 	case 0:
-		upslog_with_errno(1, "SSL_connect do not accept handshake.");
+		upsdebug_with_errno(1, "SSL_connect do not accept handshake.");
 		ssl_error(ups->ssl, res);
 		return -1;
 	default:
-		upslog_with_errno(1, "Unknown return value from SSL_connect %d", res);
+		upsdebug_with_errno(1, "Unknown return value from SSL_connect %d", res);
 		ssl_error(ups->ssl, res);
 		return -1;
 	}

drivers/adelsystem_cbi.c

@@ -34,7 +34,7 @@
 #endif
 
 #define DRIVER_NAME	"NUT ADELSYSTEM DC-UPS CB/CBI driver (libmodbus link type: " NUT_MODBUS_LINKTYPE_STR ")"
-#define DRIVER_VERSION	"0.04"
+#define DRIVER_VERSION	"0.05"
 
 /* variables */
 static modbus_t *mbctx = NULL;							/* modbus memory context */
@@ -81,7 +81,7 @@ int register_read(modbus_t *mb, int addr, regtype_t type, void *data);
 int register_write(modbus_t *mb, int addr, regtype_t type, void *data);
 
 /* instant command triggered by upsd */
-int upscmd(const char *cmd, const char *arg);
+int upscmd(const char *cmdname, const char *extra);
 
 /* count the time elapsed since start */
 long time_elapsed(struct timeval *start);
@@ -811,36 +811,43 @@ long time_elapsed(struct timeval *start)
 }
 
 /* instant command triggered by upsd */
-int upscmd(const char *cmd, const char *arg)
+int upscmd(const char *cmdname, const char *extra)
 {
 	int rval;
 	int data;
 
-	if (!strcasecmp(cmd, "load.off")) {
+	/* May be used in logging below, but not as a command argument */
+	NUT_UNUSED_VARIABLE(extra);
+	upsdebug_INSTCMD_STARTING(cmdname, extra);
+
+	if (!strcasecmp(cmdname, "load.off")) {
 		data = 1;
+		upslog_INSTCMD_POWERSTATE_CHANGE(cmdname, extra);
 		rval = register_write(mbctx, regs[FSD].xaddr, regs[FSD].type, &data);
 		if (rval == -1) {
-			upslogx(2,
+			upsdebugx(2,
 				"ERROR:(%s) modbus_write_register: addr:0x%08x, regtype: %u, path:%s",
 				modbus_strerror(errno),
 				(unsigned int)(regs[FSD].xaddr),
 				regs[FSD].type,
 				device_path
 			);
-			upslogx(LOG_NOTICE, "load.off: failed (communication error) [%s] [%s]", cmd, arg);
+			upslogx(LOG_INSTCMD_FAILED, "load.off: failed (communication error) [%s] [%s]", NUT_STRARG(cmdname), NUT_STRARG(extra));
 			rval = STAT_INSTCMD_FAILED;
 		} else {
 			upsdebugx(2, "load.off: addr: 0x%x, data: %d",
 				(unsigned int)(regs[FSD].xaddr), data);
 			rval = STAT_INSTCMD_HANDLED;
 		}
-	} else if (!strcasecmp(cmd, "shutdown.stayoff")) {
+	} else if (!strcasecmp(cmdname, "shutdown.stayoff")) {
 		/* FIXME: Which one is this actually -
 		 * "shutdown.stayoff" or "shutdown.return"? */
 		int cnt = FSD_REPEAT_CNT;	 /* shutdown repeat counter */
 		struct timeval start;
 		long etime;
 
+		upslog_INSTCMD_POWERSTATE_CHANGE(cmdname, extra);
+
 		/* retry sending shutdown command on error */
 		while ((rval = upscmd("load.off", NULL)) != STAT_INSTCMD_HANDLED && cnt > 0) {
 			rval = gettimeofday(&start, NULL);
@@ -856,12 +863,12 @@ int upscmd(const char *cmd, const char *arg)
 		switch (rval) {
 			case STAT_INSTCMD_FAILED:
 			case STAT_INSTCMD_INVALID:
-				upslog_with_errno(LOG_ERR, "instcmd: %s failed", cmd);
+				upslog_with_errno(LOG_INSTCMD_FAILED, "instcmd: %s failed", cmdname);
 				if (handling_upsdrv_shutdown > 0)
 					set_exit_flag(EF_EXIT_FAILURE);
 				break;
 			case STAT_INSTCMD_UNKNOWN:
-				upslog_with_errno(LOG_ERR, "instcmd: %s not supported", cmd);
+				upslog_with_errno(LOG_INSTCMD_UNKNOWN, "instcmd: %s not supported", cmdname);
 				if (handling_upsdrv_shutdown > 0)
 					set_exit_flag(EF_EXIT_FAILURE);
 				break;
@@ -872,7 +879,7 @@ int upscmd(const char *cmd, const char *arg)
 				break;
 		}
 	} else {
-		upslogx(LOG_NOTICE, "instcmd: unknown command [%s] [%s]", cmd, arg);
+		upslog_INSTCMD_UNKNOWN(cmdname, extra);
 		rval = STAT_INSTCMD_UNKNOWN;
 	}
 	return rval;

drivers/al175.c

@@ -52,7 +52,7 @@ typedef	uint8_t byte_t;
 
 
 #define DRIVER_NAME	"Eltek AL175/COMLI driver"
-#define DRIVER_VERSION	"0.16"
+#define DRIVER_VERSION	"0.17"
 
 /* driver description structure */
 upsdrv_info_t upsdrv_info = {
@@ -1295,7 +1295,9 @@ static int instcmd(const char *cmdname, const char *extra)
 {
 	int err;
 
-	upsdebugx(1, "INSTCMD: %s", cmdname);
+	/* May be used in logging below, but not as a command argument */
+	NUT_UNUSED_VARIABLE(extra);
+	upsdebug_INSTCMD_STARTING(cmdname, extra);
 
 	io_new_transaction(/*timeout=*/5);
 
@@ -1305,16 +1307,18 @@ static int instcmd(const char *cmdname, const char *extra)
 	 */
 
 	if (!strcasecmp(cmdname, "test.battery.start")) {
+		upslog_INSTCMD_POWERSTATE_MAYBE(cmdname, extra);
 		err = START_BATTERY_TEST(24, 1);
 		return (!err ? STAT_INSTCMD_HANDLED : STAT_INSTCMD_FAILED);
 	}
 
 	if (!strcasecmp(cmdname, "test.battery.stop")) {
+		upslog_INSTCMD_POWERSTATE_MAYBE(cmdname, extra);
 		err = STOP_BATTERY_TEST();
 		return (!err ? STAT_INSTCMD_HANDLED : STAT_INSTCMD_FAILED);
 	}
 
-	upslogx(LOG_NOTICE, "instcmd: unknown command [%s] [%s]", cmdname, extra);
+	upslog_INSTCMD_UNKNOWN(cmdname, extra);
 	return STAT_INSTCMD_UNKNOWN;
 }
 

drivers/apc_modbus.c

@@ -44,7 +44,7 @@
 #endif
 
 #define DRIVER_NAME	"NUT APC Modbus driver " DRIVER_NAME_NUT_MODBUS_HAS_USB_WITH_STR " USB support (libmodbus link type: " NUT_MODBUS_LINKTYPE_STR ")"
-#define DRIVER_VERSION	"0.13"
+#define DRIVER_VERSION	"0.14"
 
 #if defined NUT_MODBUS_HAS_USB
 
@@ -1224,6 +1224,8 @@ static int _apc_modbus_setvar(const char *nut_varname, const char *str_value)
 	apc_modbus_register_t *apc_map = NULL, *apc_value = NULL;
 	uint16_t reg_value[16];
 
+	upsdebug_SET_STARTING(nut_varname, str_value);
+
 	for (mi = 0; mi < SIZEOF_ARRAY(apc_modbus_register_maps) && apc_value == NULL; mi++) {
 		apc_map = apc_modbus_register_maps[mi];
 
@@ -1236,20 +1238,20 @@ static int _apc_modbus_setvar(const char *nut_varname, const char *str_value)
 	}
 
 	if (!apc_map || !apc_value) {
-		upslogx(LOG_WARNING, "%s: [%s] is unknown", __func__, nut_varname);
+		upslog_SET_UNKNOWN(nut_varname, str_value);
 		return STAT_SET_UNKNOWN;
 	}
 
 	if (!(apc_value->value_flags & APC_VF_RW)) {
-		upslogx(LOG_WARNING, "%s: [%s] is not writable", __func__, nut_varname);
+		upslogx(LOG_SET_INVALID, "%s: [%s] is not writable", __func__, nut_varname);
 		return STAT_SET_INVALID;
 	}
 
 	assert(apc_value->modbus_len < SIZEOF_ARRAY(reg_value));
 
 	if (apc_value->value_converter && apc_value->value_converter->nut_to_apc) {
 		if (!apc_value->value_converter->nut_to_apc(str_value, reg_value, apc_value->modbus_len)) {
-			upslogx(LOG_WARNING, "%s: [%s] failed to convert value", __func__, nut_varname);
+			upslogx(LOG_SET_CONVERSION_FAILED, "%s: [%s] failed to convert value", __func__, nut_varname);
 			return STAT_SET_CONVERSION_FAILED;
 		}
 	} else {
@@ -1292,7 +1294,7 @@ static int _apc_modbus_setvar(const char *nut_varname, const char *str_value)
 		}
 
 		if (!r) {
-			upslogx(LOG_WARNING, "%s: [%s] failed to convert value", __func__, nut_varname);
+			upslogx(LOG_SET_CONVERSION_FAILED, "%s: [%s] failed to convert value", __func__, nut_varname);
 			return STAT_SET_CONVERSION_FAILED;
 		}
 	}
@@ -1393,7 +1395,9 @@ static int _apc_modbus_instcmd(const char *nut_cmdname, const char *extra)
 	apc_modbus_command_t *apc_command = NULL;
 	uint16_t value[4]; /* Max 64-bit */
 
+	/* May be used in logging below, but not as a command argument */
 	NUT_UNUSED_VARIABLE(extra);
+	upsdebug_INSTCMD_STARTING(nut_cmdname, extra);
 
 	for (i = 0; apc_modbus_command_map[i].nut_command_name; i++) {
 		if (!strcasecmp(nut_cmdname, apc_modbus_command_map[i].nut_command_name)) {
@@ -1403,21 +1407,22 @@ static int _apc_modbus_instcmd(const char *nut_cmdname, const char *extra)
 	}
 
 	if (!apc_command) {
-		upslogx(LOG_WARNING, "%s: [%s] is unknown", __func__, nut_cmdname);
+		upslog_INSTCMD_UNKNOWN(nut_cmdname, extra);
 		return STAT_INSTCMD_UNKNOWN;
 	}
 
 	assert(apc_command->modbus_len <= SIZEOF_ARRAY(value));
 
 	if (!_apc_modbus_from_uint64(apc_command->value, value, apc_command->modbus_len)) {
-		upslogx(LOG_WARNING, "%s: [%s] failed to convert value", __func__, nut_cmdname);
+		upslogx(LOG_INSTCMD_CONVERSION_FAILED, "%s: [%s] failed to convert value", __func__, nut_cmdname);
 		return STAT_INSTCMD_CONVERSION_FAILED;
 	}
 
 	addr = apc_command->modbus_addr;
 	nb = apc_command->modbus_len;
+	upslog_INSTCMD_POWERSTATE_CHECKED(nut_cmdname, extra);
 	if (modbus_write_registers(modbus_ctx, addr, nb, value) < 0) {
-		upslogx(LOG_ERR, "%s: Write of %d:%d failed: %s (%s)", __func__, addr, addr + nb, modbus_strerror(errno), device_path);
+		upslogx(LOG_INSTCMD_FAILED, "%s: Write of %d:%d failed: %s (%s)", __func__, addr, addr + nb, modbus_strerror(errno), device_path);
 		_apc_modbus_handle_error(modbus_ctx);
 		return STAT_INSTCMD_FAILED;
 	}

drivers/apcsmart-old.c

@@ -25,7 +25,7 @@
 #include "nut_stdint.h"
 
 #define DRIVER_NAME	"APC Smart protocol driver (old)"
-#define DRIVER_VERSION	"2.34"
+#define DRIVER_VERSION	"2.35"
 
 static upsdrv_info_t table_info = {
 	"APC command table",
@@ -1321,13 +1321,15 @@ static int setvar(const char *varname, const char *val)
 {
 	apc_vartab_t	*vt;
 
+	upsdebug_SET_STARTING(varname, val);
+
 	vt = vartab_lookup_name(varname);
 
 	if (!vt)
 		return STAT_SET_UNKNOWN;
 
 	if ((vt->flags & APC_RW) == 0) {
-		upslogx(LOG_WARNING, "setvar: [%s] is not writable", varname);
+		upslogx(LOG_SET_INVALID, "setvar: [%s] is not writable", varname);
 		return STAT_SET_INVALID;
 	}
 
@@ -1337,7 +1339,7 @@ static int setvar(const char *varname, const char *val)
 	if (vt->flags & APC_STRING)
 		return setvar_string(vt, val);
 
-	upslogx(LOG_WARNING, "setvar: Unknown type for [%s]", varname);
+	upslogx(LOG_SET_UNKNOWN, "setvar: Unknown type for [%s]", varname);
 	return STAT_SET_UNKNOWN;
 }
 
@@ -1351,7 +1353,7 @@ static int do_cmd(apc_cmdtab_t *ct)
 	ret = ser_send_char(upsfd, ct->cmd);
 
 	if (ret != 1) {
-		upslog_with_errno(LOG_ERR, "do_cmd: ser_send_char failed");
+		upslog_with_errno(LOG_INSTCMD_FAILED, "do_cmd: ser_send_char failed");
 		return STAT_INSTCMD_HANDLED;		/* FUTURE: failed */
 	}
 
@@ -1362,7 +1364,7 @@ static int do_cmd(apc_cmdtab_t *ct)
 		ret = ser_send_char(upsfd, ct->cmd);
 
 		if (ret != 1) {
-			upslog_with_errno(LOG_ERR, "do_cmd: ser_send_char failed");
+			upslog_with_errno(LOG_INSTCMD_FAILED, "do_cmd: ser_send_char failed");
 			return STAT_INSTCMD_HANDLED;	/* FUTURE: failed */
 		}
 	}
@@ -1373,7 +1375,7 @@ static int do_cmd(apc_cmdtab_t *ct)
 		return STAT_INSTCMD_HANDLED;		/* FUTURE: failed */
 
 	if (strcmp(buf, "OK") != 0) {
-		upslogx(LOG_WARNING, "Got [%s] after command [%s]",
+		upslogx(LOG_INSTCMD_FAILED, "Got [%s] after command [%s]",
 			buf, ct->name);
 
 		return STAT_INSTCMD_HANDLED;		/* FUTURE: failed */
@@ -1410,29 +1412,39 @@ static int instcmd(const char *cmdname, const char *extra)
 	int	i;
 	apc_cmdtab_t	*ct;
 
+	/* May be used in logging below, but not as a command argument */
+	NUT_UNUSED_VARIABLE(extra);
+	upsdebug_INSTCMD_STARTING(cmdname, extra);
+
 	ct = NULL;
 
 	for (i = 0; apc_cmdtab[i].name != NULL; i++)
 		if (!strcasecmp(apc_cmdtab[i].name, cmdname))
 			ct = &apc_cmdtab[i];
 
 	if (!ct) {
-		upslogx(LOG_WARNING, "instcmd: unknown command [%s] [%s]",
-			cmdname, extra);
+		upslog_INSTCMD_UNKNOWN(cmdname, extra);
 		return STAT_INSTCMD_UNKNOWN;
 	}
 
 	if ((ct->flags & APC_PRESENT) == 0) {
-		upslogx(LOG_WARNING, "instcmd: command [%s] [%s] is not supported",
-			cmdname, extra);
+		upslogx(LOG_INSTCMD_UNKNOWN,
+			"%s: command [%s] [%s] is not supported on this device",
+			__func__, NUT_STRARG(cmdname), NUT_STRARG(extra));
 		return STAT_INSTCMD_UNKNOWN;
 	}
 
-	if (!strcasecmp(cmdname, "calibrate.start"))
+	if (!strcasecmp(cmdname, "calibrate.start")) {
+		upslog_INSTCMD_POWERSTATE_MAYBE(cmdname, extra);
 		return do_cal(1);
+	}
 
-	if (!strcasecmp(cmdname, "calibrate.stop"))
+	if (!strcasecmp(cmdname, "calibrate.stop")) {
+		upslog_INSTCMD_POWERSTATE_MAYBE(cmdname, extra);
 		return do_cal(0);
+	}
+
+	upslog_INSTCMD_POWERSTATE_CHECKED(cmdname, extra);
 
 	if (ct->flags & APC_NASTY)
 		return instcmd_chktime(ct);