Longer timeout after serial port init in nutdrv_qx

NEWS.adoc

@@ -143,6 +143,12 @@ https://github.com/networkupstools/nut/milestone/12
      well -- namely, that we successfully use reasonably many of the existing
      mappings. Suggest how user can help improve the driver if too few data
      points were seen. [PR #3095]
+   * The `qx_process_answer()` method was extended to raise `errno=ETIMEDOUT`
+     (rather than `EINVAL`) for short reads that are zero length (and/or that
+     `errno` value was set in caller's context already). This was used to make
+     `Voltronic` protocol initial connections more reliable when the device
+     controller lags upon first contact (is booting itself?) [issue #3276,
+     PR #3283]
    * New `nutdrv_qx_innovatae` adds support for Ippon INNOVA TAE series which
      are similar to Q1 except in reporting of nominal `voltage`, `current`,
      `frequency` data (that along with `battery_voltage_reports_one_pack` and

docs/nut.dict

@@ -1,4 +1,4 @@
-personal_ws-1.1 en 3643 utf-8
+personal_ws-1.1 en 3645 utf-8
 AAC
 AAS
 ABI
@@ -331,6 +331,7 @@ EE
 EEPROM
 EFI
 EG
+EINVAL
 EL
 ELCD
 EMI
@@ -353,6 +354,7 @@ ESS
 ESV
 ESXi
 ETIME
+ETIMEDOUT
 EUROCASE
 EVeRr
 EXtreme

drivers/nutdrv_qx.c

@@ -58,7 +58,7 @@
 #	define DRIVER_NAME	"Generic Q* Serial driver"
 #endif	/* QX_USB */
 
-#define DRIVER_VERSION	"0.48"
+#define DRIVER_VERSION	"0.49"
 
 #ifdef QX_SERIAL
 #	include "serial.h"
@@ -3643,7 +3643,7 @@ void	upsdrv_initups(void)
 		ser_set_rts(upsfd, cablepower[i].rts);
 
 		/* Allow some time to settle for the cablepower */
-		usleep(100000);
+		usleep(1100000);
 
 #	endif	/* TESTING */
 
@@ -4432,6 +4432,7 @@ static bool_t	qx_ups_walk(walkmode_t mode)
 				previous_item.answer);
 
 			/* Process the answer */
+			errno = 0;
 			retcode = qx_process_answer(item, strlen(item->answer));
 
 		/* ..otherwise: execute command to get answer from the UPS */
@@ -4663,11 +4664,15 @@ item_t	*find_nut_info(const char *varname, const unsigned long flag, const unsig
 /* Process the answer we got back from the UPS
  * Return -1 on errors, 0 on success
  * Can set errno, note that EINVAL means unsupported
- * parameter value here!
+ * parameter value here, and ETIMEDOUT can be passed
+ * from previous context for short reads, or set
+ * unilaterally for zero-length reads!
  */
 static int	qx_process_answer(item_t *item, const size_t len)
 {
-	errno = 0;
+	/* Initial errno inherited from caller, e.g. may be qx_command()
+	 * in qx_process(), but may be from other memset() etc. after it
+	 */
 
 	/* Query rejected by the UPS */
 	if (subdriver->rejected && !strcasecmp(item->answer, subdriver->rejected)) {
@@ -4679,12 +4684,19 @@ static int	qx_process_answer(item_t *item, const size_t len)
 
 	/* Short reply */
 	if (item->answer_len && len < item->answer_len) {
-		upsdebugx(2, "%s: short reply (%s) %" PRIuSIZE "<%" PRIuSIZE,
+		upsdebug_with_errno(2, "%s: short reply (%s) %" PRIuSIZE "<%" PRIuSIZE,
 			__func__, item->info_type, len, item->answer_len);
-		errno = EINVAL;
+		if (len == 0 || errno == ETIMEDOUT) {
+			errno = ETIMEDOUT;
+		} else {
+			errno = EINVAL;
+		}
 		return -1;
 	}
 
+	/* Not a systemic error by default */
+	errno = 0;
+
 	/* Wrong leading character */
 	if (item->leading && item->answer[0] != item->leading) {
 		upsdebugx(2,
@@ -4711,6 +4723,7 @@ static int	qx_process_answer(item_t *item, const size_t len)
 		snprintf(item->value, sizeof(item->value), "%s", "");
 	}
 
+	/* Reset the common error level, if some method above raised it */
 	errno = 0;
 	return 0;
 }

drivers/nutdrv_qx_voltronic.c

@@ -25,7 +25,7 @@
 #include "nutdrv_qx.h"
 #include "nutdrv_qx_voltronic.h"
 
-#define VOLTRONIC_VERSION "Voltronic 0.13"
+#define VOLTRONIC_VERSION "Voltronic 0.14"
 
 /* Support functions */
 static int	voltronic_claim(void);
@@ -1685,40 +1685,61 @@ static testing_t	voltronic_testing[] = {
 /* This function allows the subdriver to "claim" a device: return 1 if the device is supported by this subdriver, else 0. */
 static int	voltronic_claim(void)
 {
+	int	query_result;
 
 	/* We need at least QGS and QPI to run this subdriver */
 
 	item_t	*item = find_nut_info("input.voltage", 0, 0);
 
-	/* Don't know what happened */
-	if (!item)
+	/* Don't know what happened - should have looked up in the mapping table here! */
+	if (!item) {
+		upsdebug_with_errno(4, "%s: did not find 'input.voltage' in mapping table", __func__);
 		return 0;
+	}
 
 	/* No reply/Unable to get value */
-	if (qx_process(item, NULL))
-		return 0;
+	if ((query_result = qx_process(item, NULL))) {
+		upsdebug_with_errno(4, "%s: failed (%d) to get 'input.voltage'", __func__, query_result);
+
+		if (errno == ETIMEDOUT) {
+			upsdebugx(2, "%s: Sometimes the device is laggy, and we could have posted many queries and the buffer is full of replies to them (or it is still producing the answers); try to sleep, flush it and ask again", __func__);
+			usleep(5000000);	/* arbitrary 5s delay for the device to maybe produce answers to earlier voltage requests */
+			upsdebugx(2, "%s: Retry the query now, buffers will be flushed then", __func__);
+			query_result = qx_process(item, NULL);
+		}
+
+		if (query_result) {
+			/* Not a known timeout/zero-read initially, or still a bad response */
+			return 0;
+		}
+	}
 
 	/* Unable to process value */
-	if (ups_infoval_set(item) != 1)
+	if ((query_result = ups_infoval_set(item)) != 1) {
+		upsdebug_with_errno(4, "%s: failed (%d) to set infoval for 'input.voltage'", __func__, query_result);
 		return 0;
+	}
 
 	/* UPS Protocol */
 	item = find_nut_info("ups.firmware.aux", 0, 0);
 
 	/* Don't know what happened */
 	if (!item) {
+		upsdebug_with_errno(4, "%s: did not find 'ups.firmware.aux' in mapping table", __func__);
 		dstate_delinfo("input.voltage");
 		return 0;
 	}
 
 	/* No reply/Unable to get value */
-	if (qx_process(item, NULL)) {
+	if ((query_result = qx_process(item, NULL))) {
+		upsdebug_with_errno(4, "%s: failed (%d) to get 'ups.firmware.aux'", __func__, query_result);
 		dstate_delinfo("input.voltage");
 		return 0;
 	}
 
 	/* Unable to process value/Protocol out of range */
-	if (ups_infoval_set(item) != 1) {
+	if ((query_result = ups_infoval_set(item)) != 1) {
+		upsdebug_with_errno(4, "%s: failed (%d) to set infoval for 'ups.firmware.aux'", __func__, query_result);
 		dstate_delinfo("input.voltage");
 		return 0;
 	}