[Nut-upsdev] Powercool PCRACK-1200VA patch update

Ian Betts ian.betts at ymail.com
Fri Apr 3 20:18:24 BST 2020


Sorry about the noise guys.

Below a significantly improved patch.

The main difference is that all calls to usb_get_string_.. have been 
wrapped in a new function nut_usb_get_string()  that is implemented in 
libusb.c

This was necessary in order to make the bufflen_fix available in 
libusb.c where usb_get_string() is called in libusb_open()

This wrapper function mops up and hides all the work concerned with 
handling of langid and bufflen.

The static vars langid_fix and buflen_fix are accordingly moved to libusb.c

regards,

Ian




diff --git a/drivers/libusb.c b/drivers/libusb.c
index 0eb054a7..42f1b93f 100644
--- a/drivers/libusb.c
+++ b/drivers/libusb.c
@@ -36,6 +36,7 @@
  #define USB_DRIVER_NAME        "USB communication driver"
  #define USB_DRIVER_VERSION    "0.33"

+
  /* driver description structure */
  upsdrv_info_t comm_upsdrv_info = {
      USB_DRIVER_NAME,
@@ -52,6 +53,10 @@ static void libusb_close(usb_dev_handle *udev);
  /*! Add USB-related driver variables with addvar().
   * This removes some code duplication across the USB drivers.
   */
+#define USB_BUFLEN_MAX 256
+static int                langid_fix = -1;
+static int                buflen_fix = -1;
+
  void nut_usb_addvars(void)
  {
      /* allow -x vendor=X, vendorid=X, product=X, productid=X, serial=X */
@@ -64,8 +69,12 @@ void nut_usb_addvars(void)

      addvar(VAR_VALUE, "bus", "Regular expression to match USB bus name");
      addvar(VAR_VALUE, "usb_set_altinterface", "Force redundant call to 
usb_set_altinterface() (value=bAlternateSetting; default=0)");
+
+    addvar(VAR_VALUE, "langid_fix", "Apply the language ID workaround 
to the krauler subdriver (0x409 or 0x4095)");
+    addvar(VAR_VALUE, "buflen_fix", "Apply the buflen workaround to the 
krauler subdriver");
  }

+
  /* From usbutils: workaround libusb API goofs:  "byte" should never be 
sign extended;
   * using "char" is trouble.  Likewise, sizes should never be negative.
   */
@@ -128,6 +137,73 @@ static int nut_usb_set_altinterface(usb_dev_handle 
*udev)

  #define usb_control_msg         typesafe_control_msg

+
+/*
+ * This is a wrapper for the usb_get_string() and 
usb_get_string_simple() functions
+ * Its purpose is to encapsulate the handling of langid_fix and buflen_fix.
+ */
+int nut_usb_get_string(usb_dev_handle *dev, int index, char *buf, 
size_t buflen) {
+
+    /* check for buflen fix */
+    int blen = buflen;
+    if(buflen_fix != -1) {
+        blen = buflen_fix;
+    }
+
+    /* check for langid fix */
+    if (langid_fix != -1) {
+
+        /* in this case we use the langid_fix value for langid */
+        int ret = usb_get_string(dev, index, langid_fix, buf, blen);
+
+        /* Limit this check, at least for now */
+        /* Invalid receive size - message corrupted */
+        if (ret != buf[0]) {
+            upsdebugx(1, "size mismatch: %d / %d", ret, buf[0]);
+            return 0;
+        }
+
+        /* Simple unicode -> ASCII inplace conversion
+            * FIXME: this code is at least shared with mge-shut/libshut
+            * Create a common function? */
+        unsigned int    di, si, size = buf[0];
+        for (di = 0, si = 2; si < size; si += 2) {
+
+            if (di >= (buflen - 1))
+                break;
+
+            if (buf[si + 1])    /* high byte */
+                buf[di++] = '?';
+            else
+                buf[di++] = buf[si];
+
+        }
+        buf[di] = 0;
+        return di;
+
+    } else if (langid_fix == 0) {
+        /* try to learn the langid and save it
+         * Asking for the zero'th index is special:-
+         * it returns a string descriptor that contains all the 
language IDs supported by the device.
+         * Typically there aren't many - often only one.
+         * The language IDs are 16 bit numbers, and they start at the 
third byte in the descriptor.
+         * See USB 2.0 specification, section 9.6.7, for more 
information on this.
+         *
+         * The only reason to do this is to avoid 
usb_get_string_simple() which automatically
+         * queries the langid with every request
+         */
+        char tbuf[USB_BUFLEN_MAX];
+        int ret = usb_get_string(dev, 0, 0, tbuf, sizeof(tbuf));
+        if (ret >= 4) {
+            langid_fix = tbuf[2] | (tbuf[3] << 8);
+            upsdebugx(1, "First supported language ID: 0x%x (please 
report to the NUT maintainer!)", langid_fix);
+        }
+    }
+    return usb_get_string_simple(dev, index, buf, blen);
+}
+
+
+
  /* On success, fill in the curDevice structure and return the report
   * descriptor length. On failure, return -1.
   * Note: When callback is not NULL, the report descriptor will be
@@ -152,7 +228,7 @@ static int libusb_open(usb_dev_handle **udevp, 
USBDevice_t *curDevice, USBDevice
      int ret, res;
      unsigned char buf[20];
      unsigned char *p;
-    char string[256];
+    char string[USB_BUFLEN_MAX];
      int i;
      /* All devices use HID descriptor at index 0. However, some newer
       * Eaton units have a light HID descriptor at index 0, and the full
@@ -173,6 +249,32 @@ static int libusb_open(usb_dev_handle **udevp, 
USBDevice_t *curDevice, USBDevice
      libusb_close(*udevp);
  #endif

+    /* check for buflen workaround */
+    int blen;
+    if(getval("buflen_fix")) {
+        if(sscanf(getval("buflen_fix"),"%d",&blen) != 1) {
+            upslogx(LOG_NOTICE, "Error enabling buflen workaround");
+        } else {
+            if (blen > USB_BUFLEN_MAX) {
+                upslogx(LOG_NOTICE,
+                "Error enabling buflen workaround, buflen must be <= %d",
+                USB_BUFLEN_MAX);
+            } else {
+                buflen_fix = blen;
+            }
+        }
+    }
+
+    /* Check for language ID workaround (#1) */
+    if (getval("langid_fix")) {
+        /* Skip "0x" prefix and set back to hexadecimal */
+        if (sscanf(getval("langid_fix") + 2, "%x", &langid_fix) != 1) {
+            upslogx(LOG_NOTICE, "Error enabling language ID workaround");
+        } else {
+            upsdebugx(2, "Language ID workaround enabled (using 
'0x%x')", langid_fix);
+        }
+    }
+
      upsdebugx(3, "usb_busses=%p", usb_busses);

      for (bus = usb_busses; bus; bus = bus->next) {
@@ -208,7 +310,7 @@ static int libusb_open(usb_dev_handle **udevp, 
USBDevice_t *curDevice, USBDevice
              curDevice->bcdDevice = dev->descriptor.bcdDevice;

              if (dev->descriptor.iManufacturer) {
-                ret = usb_get_string_simple(udev, 
dev->descriptor.iManufacturer,
+                ret = nut_usb_get_string(udev, 
dev->descriptor.iManufacturer,
                      string, sizeof(string));
                  if (ret > 0) {
                      curDevice->Vendor = strdup(string);
@@ -216,7 +318,7 @@ static int libusb_open(usb_dev_handle **udevp, 
USBDevice_t *curDevice, USBDevice
              }

              if (dev->descriptor.iProduct) {
-                ret = usb_get_string_simple(udev, dev->descriptor.iProduct,
+                ret = nut_usb_get_string(udev, dev->descriptor.iProduct,
                      string, sizeof(string));
                  if (ret > 0) {
                      curDevice->Product = strdup(string);
@@ -224,7 +326,7 @@ static int libusb_open(usb_dev_handle **udevp, 
USBDevice_t *curDevice, USBDevice
              }

              if (dev->descriptor.iSerialNumber) {
-                ret = usb_get_string_simple(udev, 
dev->descriptor.iSerialNumber,
+                ret = nut_usb_get_string(udev, 
dev->descriptor.iSerialNumber,
                      string, sizeof(string));
                  if (ret > 0) {
                      curDevice->Serial = strdup(string);
diff --git a/drivers/libusb.h b/drivers/libusb.h
index 66d26335..46414da1 100644
--- a/drivers/libusb.h
+++ b/drivers/libusb.h
@@ -60,6 +60,8 @@ typedef struct usb_communication_subdriver_s {
      unsigned char *buf, int bufsize, int timeout);
  } usb_communication_subdriver_t;

+int nut_usb_get_string(usb_dev_handle *dev, int index, char *buf, 
size_t buflen);
+
  extern usb_communication_subdriver_t    usb_subdriver;

  #endif /* LIBUSB_H */
diff --git a/drivers/main.c b/drivers/main.c
index 0b6759dd..f05de30e 100644
--- a/drivers/main.c
+++ b/drivers/main.c
@@ -673,6 +673,7 @@ int main(int argc, char **argv)

      /* get the base data established before allowing connections */
      upsdrv_initinfo();
+
      upsdrv_updateinfo();

      if (dstate_getinfo("driver.flag.ignorelb")) {
diff --git a/drivers/nutdrv_qx.c b/drivers/nutdrv_qx.c
index bb526608..ad450e39 100644
--- a/drivers/nutdrv_qx.c
+++ b/drivers/nutdrv_qx.c
@@ -421,7 +421,6 @@ static usb_dev_handle            *udev = NULL;
  static USBDevice_t            usbdevice;
  static USBDeviceMatcher_t        *reopen_matcher = NULL;
  static USBDeviceMatcher_t        *regex_matcher = NULL;
-static int                langid_fix = -1;

  static int    (*subdriver_command)(const char *cmd, char *buf, size_t 
buflen) = NULL;

@@ -717,13 +716,7 @@ static int    krauler_command(const char *cmd, char 
*buf, size_t buflen)
          for (retry = 0; retry < 10; retry++) {

              int    ret;
-
-            if (langid_fix != -1) {
-                /* Apply langid_fix value */
-                ret = usb_get_string(udev, command[i].index, 
langid_fix, buf, buflen);
-            } else {
-                ret = usb_get_string_simple(udev, command[i].index, 
buf, buflen);
-            }
+            ret = nut_usb_get_string(udev, command[i].index, buf, buflen);

              if (ret <= 0) {
                  upsdebugx(3, "read: %s (%d)", ret ? usb_strerror() : 
"timeout", ret);
@@ -733,34 +726,6 @@ static int    krauler_command(const char *cmd, char 
*buf, size_t buflen)
              /* This may serve in the future */
              upsdebugx(1, "received %d (%d)", ret, buf[0]);

-            if (langid_fix != -1) {
-                /* Limit this check, at least for now */
-                /* Invalid receive size - message corrupted */
-                if (ret != buf[0]) {
-                    upsdebugx(1, "size mismatch: %d / %d", ret, buf[0]);
-                    continue;
-                }
-
-                /* Simple unicode -> ASCII inplace conversion
-                 * FIXME: this code is at least shared with 
mge-shut/libshut
-                 * Create a common function? */
-                unsigned int    di, si, size = buf[0];
-                for (di = 0, si = 2; si < size; si += 2) {
-
-                    if (di >= (buflen - 1))
-                        break;
-
-                    if (buf[si + 1])    /* high byte */
-                        buf[di++] = '?';
-                    else
-                        buf[di++] = buf[si];
-
-                }
-
-                buf[di] = 0;
-                ret = di;
-            }
-
              /* "UPS No Ack" has a special meaning */
              if (
                  strcspn(buf, "\r") == 10 &&
@@ -860,7 +825,7 @@ static int    fabula_command(const char *cmd, char 
*buf, size_t buflen)
      upsdebugx(4, "command index: 0x%02x", index);

      /* Send command/Read reply */
-    ret = usb_get_string_simple(udev, index, buf, buflen);
+    ret = nut_usb_get_string(udev, index, buf, buflen);

      if (ret <= 0) {
          upsdebugx(3, "read: %s (%d)", ret ? usb_strerror() : 
"timeout", ret);
@@ -1075,6 +1040,7 @@ static qx_usb_device_id_t    qx_usb_id[] = {
      { USB_DEVICE(0x0001, 0x0000),    "MEC",        "MEC0003",     
&fabula_subdriver },    /* Fideltronik/MEC LUPUS 500 USB */
      { USB_DEVICE(0x0001, 0x0000),    "ATCL FOR UPS",    "ATCL FOR 
UPS",        &fuji_subdriver },    /* Fuji UPSes */
      { USB_DEVICE(0x0001, 0x0000),    NULL,        NULL,     
&krauler_subdriver },    /* Krauler UP-M500VA */
+//    { USB_DEVICE(0x0001, 0x0000),    NULL,        "MEC0003",     
&krauler_subdriver },    /* Powercool PCRACK-1200VA */
      /* End of list */
      { -1,    -1,    NULL,    NULL,    NULL }
  };
@@ -1650,7 +1616,6 @@ void    upsdrv_makevartable(void)
      /* allow -x vendor=X, vendorid=X, product=X, productid=X, serial=X */
      nut_usb_addvars();

-    addvar(VAR_VALUE, "langid_fix", "Apply the language ID workaround 
to the krauler subdriver (0x409 or 0x4095)");
  #endif    /* QX_USB */

  #ifdef QX_SERIAL
@@ -1822,7 +1787,8 @@ void    upsdrv_initups(void)
          getval("product") ||
          getval("serial") ||
          getval("bus") ||
-        getval("langid_fix")
+        getval("langid_fix") ||
+        getval("buflen_fix")
      ) {
          /* USB */
          is_usb = 1;
@@ -1928,8 +1894,7 @@ void    upsdrv_initups(void)
              { NULL }
          };

-        int    ret, langid;
-        char    tbuf[255];    /* Some devices choke on size > 255 */
+        int    ret;
          char    *regex_array[6];

          char    *subdrv = getval("subdriver");
@@ -1941,15 +1906,6 @@ void    upsdrv_initups(void)
          regex_array[4] = getval("serial");
          regex_array[5] = getval("bus");

-        /* Check for language ID workaround (#1) */
-        if (getval("langid_fix")) {
-            /* Skip "0x" prefix and set back to hexadecimal */
-            if (sscanf(getval("langid_fix") + 2, "%x", &langid_fix) != 1) {
-                upslogx(LOG_NOTICE, "Error enabling language ID 
workaround");
-            } else {
-                upsdebugx(2, "Language ID workaround enabled (using 
'0x%x')", langid_fix);
-            }
-        }

          /* Pick up the subdriver name if set explicitly */
          if (subdrv) {
@@ -2016,21 +1972,6 @@ void    upsdrv_initups(void)
          dstate_setinfo("ups.vendorid", "%04x", usbdevice.VendorID);
          dstate_setinfo("ups.productid", "%04x", usbdevice.ProductID);

-        /* Check for language ID workaround (#2) */
-        if (langid_fix != -1) {
-            /* Future improvement:
-             *   Asking for the zero'th index is special - it returns a 
string descriptor that contains all the language IDs supported by the 
device.
-             *   Typically there aren't many - often only one.
-             *   The language IDs are 16 bit numbers, and they start at 
the third byte in the descriptor.
-             *   See USB 2.0 specification, section 9.6.7, for more 
information on this.
-             * This should allow automatic application of the workaround */
-            ret = usb_get_string(udev, 0, 0, tbuf, sizeof(tbuf));
-            if (ret >= 4) {
-                langid = tbuf[2] | (tbuf[3] << 8);
-                upsdebugx(1, "First supported language ID: 0x%x (please 
report to the NUT maintainer!)", langid);
-            }
-        }
-
      #endif    /* TESTING */

      #ifdef QX_SERIAL




More information about the Nut-upsdev mailing list