usbhid-ups: fix cyberpower issues resolved when debug logging is enabled

NEWS.adoc

@@ -182,6 +182,12 @@ https://github.com/networkupstools/nut/milestone/12
    * Declared support for APC with USB ID `051d:0005` (details may evolve
      as the devices become better known). [issue #3047, PR #3081]
    * Improved support for Eaton 5S1500LCD and 5S1600LCD (US version). [#2380]
+   * Fixed `libhid::HIDDumpTree()` to optionally get more info at driver
+     start-up even if debug is not enabled, this was shown to help with
+     some devices like CP1500PFCLCDa. This is managed by new option
+     `fetchallhid` which is enabled by default for certain CPS models
+     and disabled otherwise. Previously it might be enabled as a side
+     effect of activated debug logging. [PR #3247]
    * The driver used to report `CHRG` status if the device was not "fully
      charged", however the latter status was only queried and reported by
      some (not all) subdrivers and probably not by all devices out in the

docs/man/usbhid-ups.txt

@@ -237,7 +237,19 @@ With this option, the driver will connect to any device, including
 ones that are not yet supported. This must always be combined with the
 "vendorid" option. In this mode, the driver will not do anything
 useful except for printing debugging information (typically used with
--DD).
+`-DD` or higher debugging level).
+
+*fetchallhid*='num'::
+With this option set to a positive value, the driver will pre-fetch
+all USB HID data values same as if debugging were enabled and HID
+dumping would have happened -- this helps certain devices (CPS) to
+work correctly.
++
+The option defaults to `0` (disabled, as it always was before NUT
+v2.8.5 release), except for certain device models known to be helped
+by it being `1` (conversely, if this breaks something -- you can use
+this option to explicitly set `0` back for your deployment, and please
+do report a bug to the NUT issue tracker in this case).
 
 *maxreport*::
 With this option, the driver activates a tweak to workaround buggy firmware

docs/nut.dict

@@ -1,4 +1,4 @@
-personal_ws-1.1 en 3616 utf-8
+personal_ws-1.1 en 3620 utf-8
 AAC
 AAS
 ABI
@@ -486,6 +486,7 @@ HEADs
 HEHn
 HELn
 HFILE
+HIDDumpTree
 HIDIOCINITREPORT
 HIDRDD
 HITRANS
@@ -920,6 +921,7 @@ PDUs
 PDX
 PEX
 PFCLCD
+PFCLCDa
 PGFn
 PGP
 PHP
@@ -2080,6 +2082,7 @@ executables
 executeCommand
 execve
 expandsize
+explorehidall
 extapi
 extern
 externalConsole
@@ -2103,6 +2106,7 @@ fe
 featureReport
 fenton
 fentonups
+fetchallhid
 ffee
 fffdddxxx
 ffff

drivers/cps-hid.c

@@ -86,7 +86,7 @@ static usb_device_id_t cps_usb_device_table[] = {
 	{ USB_DEVICE(CPS_VENDORID, 0x0005), NULL },
 	/* Dynex DX-800U?, CP1200AVR/BC1200D, CP825AVR-G, CP1000AVRLCD, CP1000PFCLCD, CP1500C, CP550HG, etc. */
 	{ USB_DEVICE(CPS_VENDORID, 0x0501), &cps_battery_scale },
-	/* OR2200LCDRM2U, OR700LCDRM1U, PR6000LCDRTXL5U, CP1350EPFCLCD */
+	/* OR2200LCDRM2U, OR700LCDRM1U, PR6000LCDRTXL5U, CP1350EPFCLCD, CP1500AVRLCD3, CP1500PFCLCD */
 	{ USB_DEVICE(CPS_VENDORID, 0x0601), NULL },
 
 	/* Cyber Energy branded devices by CPS */

drivers/libhid.c

@@ -405,9 +405,10 @@ static struct {
  * since it's used to produce sub-drivers "stub" using
  * scripts/subdriver/gen-usbhid-subdriver.sh
  */
-void HIDDumpTree(hid_dev_handle_t udev, HIDDevice_t *hd, usage_tables_t *utab)
+void HIDDumpTree(hid_dev_handle_t udev, HIDDevice_t *hd, usage_tables_t *utab, int force_fetch)
 {
 	size_t	i;
+	int	should_output_debug = (nut_debug_level >= 1);
 #if (defined SHUT_MODE) && SHUT_MODE
 	NUT_UNUSED_VARIABLE(hd);
 #else	/* !SHUT_MODE => USB */
@@ -416,16 +417,18 @@ void HIDDumpTree(hid_dev_handle_t udev, HIDDevice_t *hd, usage_tables_t *utab)
 	int productID = hd->ProductID;
 #endif	/* SHUT_MODE / USB */
 
-	/* Do not go further if we already know nothing will be displayed.
-	 * Some reports take a while before they timeout, so if these are
+	/* Some reports take a while before they timeout, so if these are
 	 * not used in the driver, they should only be fetched when we're
-	 * in debug mode
+	 * in debug mode OR when force_fetch is enabled (needed for some
+	 * devices like CPS 0x0601 to properly initialize report buffer cache).
 	 */
-	if (nut_debug_level < 1) {
+	if (!should_output_debug && !force_fetch) {
 		return;
 	}
 
-	upsdebugx(1, "%" PRIuSIZE " HID objects found", pDesc->nitems);
+	if (should_output_debug) {
+		upsdebugx(1, "%" PRIuSIZE " HID objects found", pDesc->nitems);
+	}
 
 	for (i = 0; i < pDesc->nitems; i++)
 	{
@@ -452,18 +455,24 @@ void HIDDumpTree(hid_dev_handle_t udev, HIDDevice_t *hd, usage_tables_t *utab)
 		}
 #endif	/* SHUT_MODE / USB */
 
-		/* Get data value */
+		/* Get data value - always fetch to populate report buffer cache */
 		if (HIDGetDataValue(udev, pData, &value, MAX_TS) == 1) {
-			upsdebugx(1, "Path: %s, Type: %s, ReportID: 0x%02x, Offset: %i, Size: %i, Value: %g",
-				HIDGetDataItem(pData, utab), HIDDataType(pData), pData->ReportID, pData->Offset, pData->Size, value);
+			if (should_output_debug) {
+				upsdebugx(1, "Path: %s, Type: %s, ReportID: 0x%02x, Offset: %i, Size: %i, Value: %g",
+					HIDGetDataItem(pData, utab), HIDDataType(pData), pData->ReportID, pData->Offset, pData->Size, value);
+			}
 			continue;
 		}
 
-		upsdebugx(1, "Path: %s, Type: %s, ReportID: 0x%02x, Offset: %i, Size: %i",
-			HIDGetDataItem(pData, utab), HIDDataType(pData), pData->ReportID, pData->Offset, pData->Size);
+		if (should_output_debug) {
+			upsdebugx(1, "Path: %s, Type: %s, ReportID: 0x%02x, Offset: %i, Size: %i",
+				HIDGetDataItem(pData, utab), HIDDataType(pData), pData->ReportID, pData->Offset, pData->Size);
+		}
 	}
 
-	fflush(stdout);
+	if (should_output_debug) {
+		fflush(stdout);
+	}
 }
 
 /* Returns text string which can be used to display type of data

drivers/libhid.h

@@ -153,7 +153,7 @@ int HIDGetEvents(hid_dev_handle_t udev, HIDData_t **event, int eventlen);
 /*
  * Support functions
  * -------------------------------------------------------------------------- */
-void HIDDumpTree(hid_dev_handle_t udev, HIDDevice_t *hd, usage_tables_t *utab);
+void HIDDumpTree(hid_dev_handle_t udev, HIDDevice_t *hd, usage_tables_t *utab, int force_fetch);
 const char *HIDDataType(const HIDData_t *hiddata);
 
 void free_report_buffer(reportbuf_t *rbuf);

drivers/usbhid-ups.c

@@ -29,9 +29,10 @@
  */
 
 #define DRIVER_NAME	"Generic HID driver"
-#define DRIVER_VERSION	"0.71"
+#define DRIVER_VERSION	"0.72"
 
 #define HU_VAR_WAITBEFORERECONNECT "waitbeforereconnect"
+#define HU_VAR_FETCHALLHID "fetchallhid"
 
 #include "main.h"	/* Must be first, includes "config.h" */
 #include "nut_stdint.h"
@@ -142,6 +143,7 @@ bool_t use_interrupt_pipe = TRUE;
 bool_t use_interrupt_pipe = FALSE;
 #endif
 static size_t interrupt_pipe_EIO_count = 0; /* How many times we had I/O errors since last reconnect? */
+static int fetchallhid = -1; /* Force all HID reports fetch on init (for some CPS devices); -1=unset, 0=off, 1=on */
 
 /**
  * How many times do we tolerate having "0 HID objects" in a row?
@@ -1318,6 +1320,9 @@ void upsdrv_makevartable(void)
 
 	addvar(VAR_FLAG, "pollonly", "Don't use interrupt pipe, only use polling (recommended for CPS devices)");
 
+	addvar(VAR_VALUE, HU_VAR_FETCHALLHID,
+		"Fetch all HID reports during initialization (0/1, needed for some CPS devices)");
+
 	addvar(VAR_VALUE, "interrupt_pipe_no_events_tolerance", "How many times in a row do we tolerate \"Got 0 HID objects\" from USB interrupts?");
 
 	addvar(VAR_FLAG, "onlinedischarge",
@@ -1666,6 +1671,29 @@ void upsdrv_initinfo(void)
 #endif
 	}
 
+	/* fetch all HID reports on init (auto-enabled for some CPS models) */
+	val = getval(HU_VAR_FETCHALLHID);
+	if (val) {
+		fetchallhid = atoi(val);
+	}
+
+	if (fetchallhid < 0) {
+		/* handle default: disable (was never enabled until this toggle) */
+		fetchallhid = 0;
+
+#if !((defined SHUT_MODE) && SHUT_MODE)
+		/* ...although auto-enable for CPS ProductID 0x0601 which needs
+		 * this for stable communication (CP1500AVRLCD3, CP1500PFCLCD, etc.) -
+		 * see https://github.com/networkupstools/nut/issues/3116
+		 */
+		if (subdriver == &cps_subdriver && curDevice.ProductID == 0x0601) {
+			fetchallhid = 1;
+			upsdebugx(1, "Enabling '%s' for CPS device 0x%04x",
+				HU_VAR_FETCHALLHID, curDevice.ProductID);
+		}
+#endif
+	}
+
 	val = getval("interrupt_pipe_no_events_tolerance");
 	if (!val || !str_to_long(val, &interrupt_pipe_no_events_tolerance, 10)) {
 		interrupt_pipe_no_events_tolerance = -1;
@@ -2106,7 +2134,7 @@ static int callback(
 		"please note that a wrong subdriver could have been chosen above; "
 		"consider testing others with an explicit driver option",
 		__func__);
-	HIDDumpTree(udev, arghd, subdriver->utab);
+	HIDDumpTree(udev, arghd, subdriver->utab, fetchallhid);
 
 #if !((defined SHUT_MODE) && SHUT_MODE)
 	/* create a new matcher for later matching */