drivers/nutdrv_qx.c, nutdrv_qx_voltronic.c, NEWS.adoc, docs/nut.dict: qx_process_answer(): set errno=ETIMEDOUT (not EINVAL) if raised by caller or we had a zero-length read; use this in voltronic_claim() [#3276, #3283]

Signed-off-by: Jim Klimov <jimklimov+nut@gmail.com>

Author: jimklimov

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 3634 utf-8
+personal_ws-1.1 en 3636 utf-8
 AAC
 AAS
 ABI
@@ -330,6 +330,7 @@ EE
 EEPROM
 EFI
 EG
+EINVAL
 EL
 ELCD
 EMI
@@ -352,6 +353,7 @@ ESS
 ESV
 ESXi
 ETIME
+ETIMEDOUT
 EUROCASE
 EVeRr
 EXtreme

drivers/nutdrv_qx.c

@@ -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

@@ -1692,13 +1692,26 @@ static int	voltronic_claim(void)
 	item_t	*item = find_nut_info("input.voltage", 0, 0);
 
 	/* Don't know what happened - should have looked up in the mapping table here! */
-	if (!item)
+	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 ((query_result = qx_process(item, NULL))) {
 		upsdebug_with_errno(4, "%s: failed (%d) to get 'input.voltage'", __func__, query_result);
-		return 0;
+
+		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 */
@@ -1712,25 +1725,16 @@ static int	voltronic_claim(void)
 
 	/* 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 ((query_result = qx_process(item, NULL))) {
 		upsdebug_with_errno(4, "%s: failed (%d) to get 'ups.firmware.aux'", __func__, query_result);
-
-		/*if (errno == EINVAL) {*/
-			upsdebugx(2, "%s: Sometimes the device is laggy, and we could have posted many queries and the buffer is full of replies to them; try to 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: Buffers flushed, retry the query", __func__);
-			query_result = qx_process(item, NULL);
-		/*}*/
-
-		if (query_result) {
-			dstate_delinfo("input.voltage");
-			return 0;
-		}
+		dstate_delinfo("input.voltage");
+		return 0;
 	}
 
 	/* Unable to process value/Protocol out of range */