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