[Nut-upsdev] [RFC apcsmart V3 07/18] drivers/apcsmart: adjust how firmware_table_lookup() handles 'b' and 'V'

[Michal Soltys]

As 'b' can return different things than firmware version on some old
models, prefer 'V' over 'b'.

Signed-off-by: Michal Soltys <soltys at ziu.info>
---
 drivers/apcsmart.c |   26 +++++++++++++++-----------
 1 files changed, 15 insertions(+), 11 deletions(-)

diff --git a/drivers/apcsmart.c b/drivers/apcsmart.c
index 917d261..ef66fce 100644
--- a/drivers/apcsmart.c
+++ b/drivers/apcsmart.c
@@ -541,27 +541,28 @@ static int firmware_table_lookup(void)
 	unsigned int	i, j;
 	char	buf[SMALLBUF];
 
-	upsdebugx(1, "Attempting firmware lookup");
+	upsdebugx(1, "Attempting firmware lookup using command 'V'");
 
-	ret = ser_send_char(upsfd, 'b');
+	ret = ser_send_char(upsfd, 'V');
 
 	if (ret != 1) {
-		upslog_with_errno(LOG_ERR, "getbaseinfo: ser_send_char failed");
+		upslog_with_errno(LOG_ERR, "firmware_table_lookup: ser_send_char failed");
 		return 0;
 	}
 
 	ret = ser_get_line(upsfd, buf, sizeof(buf), ENDCHAR, IGNCHARS, 
 		SER_WAIT_SEC, SER_WAIT_USEC);
 
-	/* see if this is an older version like an APC600 which doesn't
-	 * response to 'a' or 'b' queries
+        /*
+	 * Some UPSes support both 'V' and 'b'. As 'b' doesn't always return
+	 * firmware version, we attempt that only if 'V' doesn't work.
 	 */
 	if ((ret < 1) || (!strcmp(buf, "NA"))) {
-		upsdebugx(1, "Attempting to contact older Smart-UPS version");
-		ret = ser_send_char(upsfd, 'V');
+		upsdebugx(1, "Attempting firmware lookup using command 'b'");
+		ret = ser_send_char(upsfd, 'b');
 
 		if (ret != 1) {
-			upslog_with_errno(LOG_ERR, "getbaseinfo: ser_send_char failed");
+			upslog_with_errno(LOG_ERR, "firmware_table_lookup: ser_send_char failed");
 			return 0;
 		}
 
@@ -572,10 +573,10 @@ static int firmware_table_lookup(void)
 			upslog_with_errno(LOG_ERR, "firmware_table_lookup: ser_get_line failed");
 			return 0;
 		}
-
-		upsdebugx(2, "Firmware: [%s]", buf);
 	}
 
+	upsdebugx(2, "Firmware: [%s]", buf);
+
 	/* this will be reworked if we get a lot of these things */
 	if (!strcmp(buf, "451.2.I")) {
 		quirk_capability_overflow = 1;
@@ -612,6 +613,10 @@ static void getbaseinfo(void)
 	int	ret = 0;
 	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)
 		return;
 
@@ -632,7 +637,6 @@ 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 */
 		oldapcsetup();
 		return;
-- 
1.7.2.1