diff --git a/NEWS.adoc b/NEWS.adoc index 16e339db4a..beeba6e4fc 100644 --- a/NEWS.adoc +++ b/NEWS.adoc @@ -86,6 +86,12 @@ https://github.com/networkupstools/nut/milestone/13 * Only claim a USB device as "supported" during discovery out of the box if `subdriver_command` was assigned (`1A86:7523` is used by CH340/341 USB chips not only in UPSes). [issue #3410] + * Added a `cypress_0665_5161_subdriver` initialization method for devices + with USB ID `0665:5161` (Belkin, Voltronic, VOLT Polska) which enables + a quirk to drain stale bytes from USB buffers of the device before our + driver sends queries, and use a longer timeout. If this change breaks + your device, please add `cypress_drain_quirk = 0` in your `ups.conf` + and let us know. [issue #2534, PR #3563] * Modified `megatec` subdriver to allow `ups.firmware` with empty contents (when a device returns all-spaces in that part of the `I` query response) out of the box and not ignore the device as unsupported, by adding the diff --git a/UPGRADING.adoc b/UPGRADING.adoc index b0146c60ed..9a01aee6cf 100644 --- a/UPGRADING.adoc +++ b/UPGRADING.adoc @@ -79,6 +79,15 @@ The new `-A filename` option defaults to trying to use a `nutauth.conf` file This was revised to follow the new common setting `reconnect_max_tries`, which defaults to trying indefinitely now. [#3541] +- Deployments using `nutdrv_qx` driver with `cypress` subdriver may be impacted + by a fix which added a `cypress_0665_5161_subdriver` initialization method for + devices with USB ID `0665:5161` (Belkin, Voltronic, VOLT Polska) which enables + a special quirk to drain stale bytes from USB buffers of the device before our + driver sends queries, and use a longer timeout. If this change breaks your + device, please add `cypress_drain_quirk = 0` in your `ups.conf` and let us + know. [issue #2534, PR #3563] + + Changes from 2.8.4 to 2.8.5 --------------------------- diff --git a/data/driver.list.in b/data/driver.list.in index 0d00169dd7..38bd28e4ec 100644 --- a/data/driver.list.in +++ b/data/driver.list.in @@ -1682,6 +1682,7 @@ "Vivaldi" "ups" "1" "EA200 LED" "USB" "richcomm_usb" +"VOLT Polska" "ups" "2" "RackUPS 1200VA/720W 2x7Ah" "USB (USB ID 0665:5161)" "nutdrv_qx" # https://github.com/networkupstools/nut/issues/2534 "Voltronic Power" "ups" "2" "Apex 1KVA" "Serial" "nutdrv_qx" "Voltronic Power" "ups" "2" "Apex 1KVA" "USB" "nutdrv_qx" "Voltronic Power" "ups" "2" "Frigate TX 1KVA" "Serial" "nutdrv_qx" diff --git a/docs/man/nutdrv_qx.txt b/docs/man/nutdrv_qx.txt index 981d549097..56116781a6 100644 --- a/docs/man/nutdrv_qx.txt +++ b/docs/man/nutdrv_qx.txt @@ -503,6 +503,15 @@ You must provide *value* (+0x409+ or +0x4095+), according to your device entry i *noscanlangid*:: If this flag is set, don't autoscan valid range for langid. +*cypress_drain_quirk =* 'value':: +Some devices using the `cypress` subdriver, e.g. VOLT Polska identified as Cypress +USB ID `0665:5161`, were seen to retain stale bytes in their USB buffer, returning +old "garbage" as response to new queries and so corrupting discovery attempts. +The `nutdrv_qx` driver can drain the USB buffer and use longer timeouts to communicate +with such devices, but such a change may potentially misfire on other devices with +same USB ID (Belkin, Voltronic). This option allows to explicitly enable or disable +this behavior; by default it is chosen according to device self-identification. + IMPLEMENTATION NOTES ^^^^^^^^^^^^^^^^^^^^ diff --git a/docs/nut.dict b/docs/nut.dict index 52faedf9ec..8fb7bfdc1c 100644 --- a/docs/nut.dict +++ b/docs/nut.dict @@ -1,4 +1,4 @@ -personal_ws-1.1 en 3800 utf-8 +personal_ws-1.1 en 3801 utf-8 AAC AAS ABI @@ -1030,6 +1030,7 @@ PnP Poettering Pohle PointBre +Polska Pos Potrans Poush diff --git a/drivers/nutdrv_qx.c b/drivers/nutdrv_qx.c index b3d8868441..669c65d322 100644 --- a/drivers/nutdrv_qx.c +++ b/drivers/nutdrv_qx.c @@ -58,7 +58,7 @@ # define DRIVER_NAME "Generic Q* Serial driver" #endif /* QX_USB */ -#define DRIVER_VERSION "0.54" +#define DRIVER_VERSION "0.55" #ifdef QX_SERIAL # include "serial.h" @@ -730,12 +730,22 @@ static int langid_fix = -1; static int (*subdriver_command)(const char *cmd, size_t cmdlen, char *buf, size_t buflen) = NULL; +/* -1: let the driver guess by USB ID; 0/1: explicit disable/enable */ +static int cypress_drain_quirk_setting = -1; +static bool_t cypress_0665_5161_quirk = FALSE; + +#define CYPRESS_0665_5161_FLUSH_REPORTS 10 +#define CYPRESS_0665_5161_FLUSH_TIMEOUT 25 +#define CYPRESS_REPLY_TIMEOUT 1000 +#define CYPRESS_0665_5161_REPLY_TIMEOUT 3000 + /* Cypress communication subdriver */ static int cypress_command(const char *cmd, size_t cmdlen, char *buf, size_t buflen) { char tmp[SMALLBUF]; size_t tmplen; int ret = 0; + int reply_timeout = CYPRESS_REPLY_TIMEOUT; size_t i; if (buflen > INT_MAX) { @@ -745,6 +755,36 @@ static int cypress_command(const char *cmd, size_t cmdlen, char *buf, size_t buf buflen = (INT_MAX - 1); } + /* Some USB devices with Cypress 0665:5161 can leave a reply from a previous + * command queued on the interrupt endpoint. Do not apply this device quirk + * to other Cypress-based UPSes. + */ + if (cypress_0665_5161_quirk) { + reply_timeout = CYPRESS_0665_5161_REPLY_TIMEOUT; + + /* Read once more than the number of reports we accept, so a timeout + * confirms that the endpoint is empty. Do not send a command if it + * remains busy: its reply could not be associated reliably. + */ + for (i = 0; i <= CYPRESS_0665_5161_FLUSH_REPORTS; i++) { + ret = usb_interrupt_read(udev, 0x81, + (usb_ctrl_charbuf)tmp, 8, CYPRESS_0665_5161_FLUSH_TIMEOUT); + + if (ret == LIBUSB_ERROR_TIMEOUT) + break; + + if (ret <= 0) + return ret; + + if (i == CYPRESS_0665_5161_FLUSH_REPORTS) { + upsdebugx(1, "cypress: input endpoint stayed busy before %s", cmd); + return LIBUSB_ERROR_BUSY; + } + + upsdebugx(3, "cypress: discarded stale input report before %s", cmd); + } + } + /* Send command */ memset(tmp, 0, sizeof(tmp)); tmplen = cmdlen > sizeof(tmp) ? sizeof(tmp) : cmdlen; @@ -779,7 +819,7 @@ static int cypress_command(const char *cmd, size_t cmdlen, char *buf, size_t buf /* ret = usb->get_interrupt(udev, (unsigned char *)&buf[i], 8, 1000); */ ret = usb_interrupt_read(udev, 0x81, - (usb_ctrl_charbuf)&buf[i], 8, 1000); + (usb_ctrl_charbuf)&buf[i], 8, reply_timeout); /* Any errors here mean that we are unable to read a reply * (which will happen after successfully writing a command @@ -2561,6 +2601,24 @@ static void *cypress_subdriver(USBDevice_t *device) { NUT_UNUSED_VARIABLE(device); + switch (cypress_drain_quirk_setting) { + case 0: cypress_0665_5161_quirk = FALSE; break; + case 1: cypress_0665_5161_quirk = TRUE; break; + default: cypress_0665_5161_quirk = FALSE; + } + subdriver_command = &cypress_command; + return NULL; +} + +static void *cypress_0665_5161_subdriver(USBDevice_t *device) +{ + NUT_UNUSED_VARIABLE(device); + + switch (cypress_drain_quirk_setting) { + case 0: cypress_0665_5161_quirk = FALSE; break; + case 1: cypress_0665_5161_quirk = TRUE; break; + default: cypress_0665_5161_quirk = TRUE; + } subdriver_command = &cypress_command; return NULL; } @@ -2705,7 +2763,7 @@ static qx_usb_device_id_t qx_usb_id[] = { { USB_DEVICE(SYSGRATION_VENDORID, 0x0000), NULL, NULL, &cypress_subdriver }, /* Agiler UPS */ { USB_DEVICE(NONAMEFFFF_VENDORID, 0x0000), NULL, NULL, &ablerex_subdriver_fun }, /* Ablerex 625L USB (Note: earlier best-fit was "krauler_subdriver" before PR #1135) */ { USB_DEVICE(LEGRAND_VENDORID, 0x0035), NULL, NULL, &krauler_subdriver }, /* Legrand Daker DK / DK Plus */ - { USB_DEVICE(CYPRESS_VENDORID, 0x5161), NULL, NULL, &cypress_subdriver }, /* Belkin F6C1200-UNV/Voltronic Power UPSes */ + { USB_DEVICE(CYPRESS_VENDORID, 0x5161), NULL, NULL, &cypress_0665_5161_subdriver }, /* Belkin F6C1200-UNV/Voltronic Power UPSes */ { USB_DEVICE(PHOENIXTEC_VENDORID, 0x0002), "Phoenixtec Power","USB Cable (V2.00)", &phoenixtec_subdriver },/* Masterguard A Series */ { USB_DEVICE(PHOENIXTEC_VENDORID, 0x0002), NULL, NULL, &cypress_subdriver }, /* Online Yunto YQ450 */ { USB_DEVICE(PHOENIXTEC_VENDORID, 0x0003), NULL, NULL, &ippon_subdriver }, /* Mustek Powermust */ @@ -3506,6 +3564,10 @@ void upsdrv_makevartable(void) "Apply the language ID workaround to the krauler subdriver " "(0x409 or 0x4095)"); addvar(VAR_FLAG, "noscanlangid", "Don't autoscan valid range for langid"); + + addvar(VAR_VALUE, "cypress_drain_quirk", + "Enable or disable USB buffer drain from stale data for devices " + "with cypress subdriver (default: enabled for 0665:5161)"); #endif /* QX_USB */ #ifdef QX_SERIAL @@ -3690,6 +3752,7 @@ void upsdrv_initups(void) char *subdrv; # endif #endif + const char *val; upsdebugx(1, "%s...", __func__); @@ -3705,7 +3768,8 @@ void upsdrv_initups(void) getval("product") || getval("serial") || getval("bus") || - getval("langid_fix") + getval("langid_fix") || + getval("cypress_drain_quirk") # if (defined WITH_USB_BUSPORT) && (WITH_USB_BUSPORT) || getval("busport") # endif @@ -3750,7 +3814,6 @@ void upsdrv_initups(void) }; int i; - const char *val; struct termios tio; /* Open and lock the serial port and set the speed to 2400 baud. */ @@ -3845,6 +3908,21 @@ void upsdrv_initups(void) } } + if ((val = getval("cypress_drain_quirk"))) { + if (!strcmp(val, "1") || !strcasecmp(val, "on") || !strcasecmp(val, "yes") || !strcasecmp(val, "true")) { + upsdebugx(2, "cypress_drain_quirk explicitly enabled: %s", val); + cypress_drain_quirk_setting = 1; + } else if (!strcmp(val, "0") || !strcasecmp(val, "off") || !strcasecmp(val, "no") || !strcasecmp(val, "false")) { + upsdebugx(2, "cypress_drain_quirk explicitly disabled: %s", val); + cypress_drain_quirk_setting = 0; + } else if (!strcmp(val, "-1") || !strcasecmp(val, "null")) { + upsdebugx(2, "cypress_drain_quirk ignored (driver will choose by USB ID): %s", val); + cypress_drain_quirk_setting = -1; + } else { + upslogx(LOG_NOTICE, "Error enabling cypress_drain_quirk: unsupported value, ignored"); + } + } + /* Pick up the subdriver name if set explicitly */ if (subdrv) {