Merge pull request #1575 from jimklimov/issue-1566

Add a `usbhid-ups` driver option to toggle report descriptor fixup activation

Author: jimklimov

docs/man/usbhid-ups.txt

@@ -45,7 +45,7 @@ This driver is known to work on:
  - most Linux systems,
  - FreeBSD (beta stage) and maybe other *BSD,
  - Darwin / Mac OS X,
- - Solaris 10.
+ - Solaris 10 and illumos-based distributions.
 
 EXTRA ARGUMENTS
 ---------------
@@ -96,6 +96,20 @@ For most devices this combination means calibration or similar maintenance;
 however some UPS models (e.g. CyberPower UT series) emit `OL+DISCHRG` when
 wall power is lost -- and need this option to handle shutdowns.
 
+*disable_fix_report_desc*::
+Set to disable fix-ups for broken USB encoding, etc. which we apply by default
+on certain models (vendors/products) which were reported as not following the
+protocol strictly. This flag allows to disable the feature in particular device
+configurations.
++
+It is always possible that the vendors eventually release fixed firmware, or
+re-use identifiers by which we match suspected broken devices for unrelated
+products, so processing these fix-ups would be a waste of time there.
++
+It is also always possible that NUT fix-ups cause issues on some devices,
+whether due to NUT bugs or because the vendor protocol implementation is
+broken in more than one place.
+
 *vendor*='regex'::
 *product*='regex'::
 *serial*='regex'::

drivers/apc-hid.c

@@ -32,7 +32,7 @@
 #include "apc-hid.h"
 #include "usb-common.h"
 
-#define APC_HID_VERSION "APC HID 0.98"
+#define APC_HID_VERSION "APC HID 0.99"
 
 /* APC */
 #define APC_VENDORID 0x051d
@@ -534,6 +534,15 @@ static int apc_fix_report_desc(HIDDevice_t *pDev, HIDDesc_t *pDesc_arg) {
 		return 0;
 	}
 
+	if (disable_fix_report_desc) {
+		upsdebugx(3,
+			"NOT Attempting Report Descriptor fix for UPS: "
+			"Vendor: %04x, Product: %04x "
+			"(got disable_fix_report_desc in config)",
+			vendorID, productID);
+		return 0;
+	}
+
 	upsdebugx(3, "Attempting Report Descriptor fix for UPS: Vendor: %04x, Product: %04x", vendorID, productID);
 
 	/* Look at the High Voltage Transfer logical max value:

drivers/cps-hid.c

@@ -30,7 +30,7 @@
 #include "cps-hid.h"
 #include "usb-common.h"
 
-#define CPS_HID_VERSION      "CyberPower HID 0.6"
+#define CPS_HID_VERSION      "CyberPower HID 0.7"
 
 /* Cyber Power Systems */
 #define CPS_VENDORID 0x0764
@@ -286,6 +286,15 @@ static int cps_fix_report_desc(HIDDevice_t *pDev, HIDDesc_t *pDesc_arg) {
 		return 0;
 	}
 
+	if (disable_fix_report_desc) {
+		upsdebugx(3,
+			"NOT Attempting Report Descriptor fix for UPS: "
+			"Vendor: %04x, Product: %04x "
+			"(got disable_fix_report_desc in config)",
+			vendorID, productID);
+		return 0;
+	}
+
 	upsdebugx(3, "Attempting Report Descriptor fix for UPS: Vendor: %04x, Product: %04x", vendorID, productID);
 
 	/* Apply the fix cautiously by looking for input voltage, high voltage transfer and output voltage report usages.

drivers/usbhid-ups.c

@@ -28,7 +28,7 @@
  */
 
 #define DRIVER_NAME	"Generic HID driver"
-#define DRIVER_VERSION		"0.48"
+#define DRIVER_VERSION		"0.49"
 #define HU_VAR_WAITBEFORERECONNECT "waitbeforereconnect"
 
 #include "main.h"
@@ -157,6 +157,7 @@ static double interval(void);
 /* global variables */
 HIDDesc_t	*pDesc = NULL;		/* parsed Report Descriptor */
 reportbuf_t	*reportbuf = NULL;	/* buffer for most recent reports */
+int disable_fix_report_desc = 0; /* by default we apply fix-ups for broken USB encoding, etc. */
 
 /* --------------------------------------------------------------- */
 /* Struct & data for boolean processing                            */
@@ -787,6 +788,9 @@ void upsdrv_makevartable(void)
 	addvar(VAR_FLAG, "onlinedischarge",
 		"Set to treat discharging while online as being offline");
 
+	addvar(VAR_FLAG, "disable_fix_report_desc",
+		"Set to disable fix-ups for broken USB encoding, etc. which we apply by default on certain vendors/products");
+
 #ifndef SHUT_MODE
 	/* allow -x vendor=X, vendorid=X, product=X, productid=X, serial=X */
 	nut_usb_addvars();
@@ -1071,6 +1075,10 @@ void upsdrv_initups(void)
 		onlinedischarge = 1;
 	}
 
+	if (testvar("disable_fix_report_desc")) {
+		disable_fix_report_desc = 1;
+	}
+
 	val = getval("interruptsize");
 	if (val) {
 		int ipv = atoi(val);
@@ -1327,6 +1335,9 @@ int fix_report_desc(HIDDevice_t *arg_pDev, HIDDesc_t *arg_pDesc) {
 	NUT_UNUSED_VARIABLE(arg_pDev);
 	NUT_UNUSED_VARIABLE(arg_pDesc);
 
+/* Implementations should honor the user's toggle:
+ *	if (disable_fix_report_desc) return 0;
+ */
 	return 0;
 }
 

drivers/usbhid-ups.h

@@ -217,5 +217,7 @@ int setvar(const char *varname, const char *val);
 
 void possibly_supported(const char *mfr, HIDDevice_t *hd);
 
+/* by default we apply fix-ups for broken USB encoding, etc. */
+extern int disable_fix_report_desc;
 int fix_report_desc(HIDDevice_t *pDev, HIDDesc_t *arg_pDesc);
 #endif /* USBHID_UPS_H */