Skip to content

Commit

Permalink
Merge pull request #2604 from tormodvolden/eaton_broken_strings
Browse files Browse the repository at this point in the history
  • Loading branch information
@jimklimov
jimklimov authored Sep 5, 2024
2 parents 0f7d5cf + 45c07f3 commit 6443582
Show file tree
Hide file tree
Showing 10 changed files with 195 additions and 268 deletions.
12 changes: 12 additions & 0 deletions NEWS.adoc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,18 @@ https://github.com/networkupstools/nut/milestone/11
reading an empty string or getting a success code `0` from libusb.
This difference should now be better logged, and not into syslog. [#2399]
- USB drivers can benefit from a new `nut_usb_get_string()` method which
can do a fallback `en_US` query for devices which report a broken "langid"
language identifier value. This notably manifested in inability to query
device Manufacturer, Model and Serial Number values with some buggy device
firmware or hardware. [PR #2604, issues #1925, #414]
* Currently this was tested to fix certain device discovery with `usbhid-ups`;
should also apply out of the box to same discovery logic in `blazer_usb`,
`nutdrv_qx`, `riello_usb` and `tripplite_usb` drivers.
* More work may be needed for other USB-capable drivers (`richcomm_usb`,
`nutdrv_atcl_usb`) and for general code to collect string readings and
other data points, and for `nut-scanner`.
- Introduced a new driver concept for interaction with OS-reported hardware
monitoring readings. Currently instantiated as `hwmon_ina219` specifically
made for Texas Instruments INA219 chip as exposed in the Linux "hwmon"
Expand Down
2 changes: 1 addition & 1 deletion drivers/blazer_usb.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@
#endif

#define DRIVER_NAME "Megatec/Q1 protocol USB driver"
#define DRIVER_VERSION "0.19"
#define DRIVER_VERSION "0.20"

/* driver description structure */
upsdrv_info_t upsdrv_info = {
Expand Down
187 changes: 54 additions & 133 deletions drivers/libusb0.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,6 @@ upsdrv_info_t comm_upsdrv_info = {
};

#define MAX_REPORT_SIZE 0x1800
#define MAX_RETRY 3

#if (!HAVE_STRCASESTR) && (HAVE_STRSTR && HAVE_STRLWR && HAVE_STRDUP)
/* Only used in this file of all NUT codebase, so not in str.{c,h}
Expand All @@ -60,7 +59,7 @@ upsdrv_info_t comm_upsdrv_info = {
static char *strcasestr(const char *haystack, const char *needle);
#endif

static void libusb_close(usb_dev_handle *udev);
static void nut_libusb_close(usb_dev_handle *udev);

/*! Add USB-related driver variables with addvar() and dstate_setinfo().
* This removes some code duplication across the USB drivers.
Expand Down Expand Up @@ -155,7 +154,7 @@ static inline int matches(USBDeviceMatcher_t *matcher, USBDevice_t *device) {
* devices from working on Mac OS X (presumably the OS is already setting
* altinterface to 0).
*/
static int nut_usb_set_altinterface(usb_dev_handle *udev)
static int nut_libusb_set_altinterface(usb_dev_handle *udev)
{
int altinterface = 0, ret = 0;
char *alt_string, *endp = NULL;
Expand Down Expand Up @@ -197,13 +196,15 @@ static int nut_usb_set_altinterface(usb_dev_handle *udev)
* is accepted, or < 1 if not. If it isn't accepted, the next device
* (if any) will be tried, until there are no more devices left.
*/
static int libusb_open(usb_dev_handle **udevp,
static int nut_libusb_open(usb_dev_handle **udevp,
USBDevice_t *curDevice, USBDeviceMatcher_t *matcher,
int (*callback)(usb_dev_handle *udev,
USBDevice_t *hd, usb_ctrl_charbuf rdbuf, usb_ctrl_charbufsize rdlen)
)
{
#ifdef HAVE_USB_DETACH_KERNEL_DRIVER_NP
int retries;
#endif
usb_ctrl_charbufsize rdlen1, rdlen2; /* report descriptor length, method 1+2 */
USBDeviceMatcher_t *m;
struct usb_device *dev;
Expand Down Expand Up @@ -313,7 +314,7 @@ static int libusb_open(usb_dev_handle **udevp,

#ifndef __linux__ /* SUN_LIBUSB (confirmed to work on Solaris and FreeBSD) */
/* Causes a double free corruption in linux if device is detached! */
libusb_close(*udevp);
nut_libusb_close(*udevp);
#endif

upsdebugx(3, "usb_busses=%p", (void*)usb_busses);
Expand Down Expand Up @@ -396,44 +397,32 @@ static int libusb_open(usb_dev_handle **udevp,
#endif

if (dev->descriptor.iManufacturer) {
retries = MAX_RETRY;
while (retries > 0) {
ret = usb_get_string_simple(udev, dev->descriptor.iManufacturer,
string, sizeof(string));
if (ret > 0) {
curDevice->Vendor = xstrdup(string);
break;
}
retries--;
upsdebugx(1, "%s get iManufacturer failed, retrying...", __func__);
ret = nut_usb_get_string(udev, dev->descriptor.iManufacturer,
string, sizeof(string));
if (ret > 0) {
curDevice->Vendor = xstrdup(string);
} else {
upsdebugx(1, "%s: get Manufacturer string failed", __func__);
}
}

if (dev->descriptor.iProduct) {
retries = MAX_RETRY;
while (retries > 0) {
ret = usb_get_string_simple(udev, dev->descriptor.iProduct,
string, sizeof(string));
if (ret > 0) {
curDevice->Product = xstrdup(string);
break;
}
retries--;
upsdebugx(1, "%s get iProduct failed, retrying...", __func__);
ret = nut_usb_get_string(udev, dev->descriptor.iProduct,
string, sizeof(string));
if (ret > 0) {
curDevice->Product = xstrdup(string);
} else {
upsdebugx(1, "%s: get Product string failed", __func__);
}
}

if (dev->descriptor.iSerialNumber) {
retries = MAX_RETRY;
while (retries > 0) {
ret = usb_get_string_simple(udev, dev->descriptor.iSerialNumber,
string, sizeof(string));
if (ret > 0) {
curDevice->Serial = xstrdup(string);
break;
}
retries--;
upsdebugx(1, "%s get iSerialNumber failed, retrying...", __func__);
ret = nut_usb_get_string(udev, dev->descriptor.iSerialNumber,
string, sizeof(string));
if (ret > 0) {
curDevice->Serial = xstrdup(string);
} else {
upsdebugx(1, "%s: get Serial Number string failed", __func__);
}
}

Expand Down Expand Up @@ -489,11 +478,10 @@ static int libusb_open(usb_dev_handle **udevp,
/* this method requires at least libusb 0.1.8:
* it force device claiming by unbinding
* attached driver... From libhid */
retries = MAX_RETRY;
#ifdef WIN32
usb_set_configuration(udev, 1);
#endif

retries = 3;
while ((ret = usb_claim_interface(udev, usb_subdriver.hid_rep_index)) < 0) {
upsdebugx(2, "failed to claim USB device: %s",
usb_strerror());
Expand Down Expand Up @@ -540,59 +528,7 @@ static int libusb_open(usb_dev_handle **udevp,
#endif
/* if_claimed = 1; */

nut_usb_set_altinterface(udev);

/* If libusb failed to identify the device strings earlier,
* can we do that after claiming the interface? Just try...
* Note that we succeeded so far, meaning these strings were
* not among matching criteria. But they can be important for
* our drivers (e.g. per-model tweaks) and pretty reporting
* of certain `device.*` and/or `ups.*` data points.
*/
if (!curDevice->Vendor) {
retries = MAX_RETRY;
while (retries > 0) {
ret = usb_get_string_simple(udev, dev->descriptor.iManufacturer,
string, sizeof(string));
if (ret > 0) {
curDevice->Vendor = xstrdup(string);
break;
}
retries--;
upsdebugx(1, "%s get iManufacturer failed, retrying...", __func__);
}
upsdebugx(2, "- Manufacturer: %s", curDevice->Vendor ? curDevice->Vendor : "unknown");
}

if (!curDevice->Product) {
retries = MAX_RETRY;
while (retries > 0) {
ret = usb_get_string_simple(udev, dev->descriptor.iProduct,
string, sizeof(string));
if (ret > 0) {
curDevice->Product = xstrdup(string);
break;
}
retries--;
upsdebugx(1, "%s get iProduct failed, retrying...", __func__);
}
upsdebugx(2, "- Product: %s", curDevice->Product ? curDevice->Product : "unknown");
}

if (!curDevice->Serial) {
retries = MAX_RETRY;
while (retries > 0) {
ret = usb_get_string_simple(udev, dev->descriptor.iSerialNumber,
string, sizeof(string));
if (ret > 0) {
curDevice->Serial = xstrdup(string);
break;
}
retries--;
upsdebugx(1, "%s get iSerialNumber failed, retrying...", __func__);
}
upsdebugx(2, "- Serial Number: %s", curDevice->Serial ? curDevice->Serial : "unknown");
}
nut_libusb_set_altinterface(udev);

/* Did the driver provide a callback method for any further
* device acceptance checks (e.g. when same ID is supported
Expand Down Expand Up @@ -803,7 +739,7 @@ static int libusb_open(usb_dev_handle **udevp,
* Error handler for usb_get/set_* functions. Return value > 0 success,
* 0 unknown or temporary failure (ignored), < 0 permanent failure (reconnect)
*/
static int libusb_strerror(const int ret, const char *desc)
static int nut_libusb_strerror(const int ret, const char *desc)
{
if (ret > 0) {
return ret;
Expand Down Expand Up @@ -850,18 +786,18 @@ static int libusb_strerror(const int ret, const char *desc)
*/

/* Expected evaluated types for the API:
* static int libusb_get_report(usb_dev_handle *udev,
* static int nut_libusb_get_report(usb_dev_handle *udev,
* int ReportId, unsigned char *raw_buf, int ReportSize)
*/
static int libusb_get_report(
static int nut_libusb_get_report(
usb_dev_handle *udev,
usb_ctrl_repindex ReportId,
usb_ctrl_charbuf raw_buf,
usb_ctrl_charbufsize ReportSize)
{
int ret;

upsdebugx(4, "Entering libusb_get_report");
upsdebugx(4, "Entering nut_libusb_get_report");

if (!udev) {
return 0;
Expand All @@ -883,14 +819,14 @@ static int libusb_get_report(
return 0;
}

return libusb_strerror(ret, __func__);
return nut_libusb_strerror(ret, __func__);
}

/* Expected evaluated types for the API:
* static int libusb_set_report(usb_dev_handle *udev,
* static int nut_libusb_set_report(usb_dev_handle *udev,
* int ReportId, unsigned char *raw_buf, int ReportSize)
*/
static int libusb_set_report(
static int nut_libusb_set_report(
usb_dev_handle *udev,
usb_ctrl_repindex ReportId,
usb_ctrl_charbuf raw_buf,
Expand Down Expand Up @@ -918,48 +854,33 @@ static int libusb_set_report(
return 0;
}

return libusb_strerror(ret, __func__);
return nut_libusb_strerror(ret, __func__);
}

/* Expected evaluated types for the API:
* static int libusb_get_string(usb_dev_handle *udev,
* int StringIdx, char *buf, size_t buflen)
* static int nut_libusb_get_string(usb_dev_handle *udev,
* int StringIdx, char *buf, int buflen)
*/
static int libusb_get_string(
static int nut_libusb_get_string(
usb_dev_handle *udev,
usb_ctrl_strindex StringIdx,
char *buf,
usb_ctrl_charbufsize buflen)
{
int ret;

#if (defined HAVE_PRAGMA_GCC_DIAGNOSTIC_PUSH_POP) && ( (defined HAVE_PRAGMA_GCC_DIAGNOSTIC_IGNORED_TYPE_LIMITS) || (defined HAVE_PRAGMA_GCC_DIAGNOSTIC_IGNORED_TAUTOLOGICAL_CONSTANT_OUT_OF_RANGE_COMPARE) || (defined HAVE_PRAGMA_GCC_DIAGNOSTIC_IGNORED_TAUTOLOGICAL_UNSIGNED_ZERO_COMPARE) )
# pragma GCC diagnostic push
#endif
#ifdef HAVE_PRAGMA_GCC_DIAGNOSTIC_IGNORED_TYPE_LIMITS
# pragma GCC diagnostic ignored "-Wtype-limits"
#endif
#ifdef HAVE_PRAGMA_GCC_DIAGNOSTIC_IGNORED_TAUTOLOGICAL_CONSTANT_OUT_OF_RANGE_COMPARE
# pragma GCC diagnostic ignored "-Wtautological-constant-out-of-range-compare"
#endif
#ifdef HAVE_PRAGMA_GCC_DIAGNOSTIC_IGNORED_TAUTOLOGICAL_UNSIGNED_ZERO_COMPARE
# pragma GCC diagnostic ignored "-Wtautological-unsigned-zero-compare"
#endif
/*
* usb.h:int usb_get_string_simple(usb_dev_handle *dev, int index,
* usb.h:int nut_usb_get_string(usb_dev_handle *dev, int index,
* usb.h- char *buf, size_t buflen);
*/
if (!udev
|| StringIdx < 0 || (uintmax_t)StringIdx > INT_MAX
|| buflen < 0 || (uintmax_t)buflen > (uintmax_t)SIZE_MAX
|| StringIdx < 1 || StringIdx > 255
|| buflen < 1
) {
#if (defined HAVE_PRAGMA_GCC_DIAGNOSTIC_PUSH_POP) && ( (defined HAVE_PRAGMA_GCC_DIAGNOSTIC_IGNORED_TYPE_LIMITS) || (defined HAVE_PRAGMA_GCC_DIAGNOSTIC_IGNORED_TAUTOLOGICAL_CONSTANT_OUT_OF_RANGE_COMPARE) || (defined HAVE_PRAGMA_GCC_DIAGNOSTIC_IGNORED_TAUTOLOGICAL_UNSIGNED_ZERO_COMPARE) )
# pragma GCC diagnostic pop
#endif
return -1;
}

ret = usb_get_string_simple(udev, StringIdx, buf, (size_t)buflen);
ret = nut_usb_get_string(udev, StringIdx, buf, (size_t)buflen);

#ifdef WIN32
errno = -ret;
Expand All @@ -969,22 +890,22 @@ static int libusb_get_string(
* logging below - also tends to happen */
if (ret == 0) {
size_t len = strlen(buf);
upsdebugx(2, "%s: usb_get_string_simple() returned "
upsdebugx(2, "%s: nut_usb_get_string() returned "
"0 (might be just success code), "
"actual buf length is %" PRIuSIZE, __func__, len);
/* if (len) */
return len;
/* else may log "libusb_get_string: Success" and return 0 below */
/* else may log "nut_libusb_get_string: Success" and return 0 below */
}

return libusb_strerror(ret, __func__);
return nut_libusb_strerror(ret, __func__);
}

/* Expected evaluated types for the API:
* static int libusb_get_interrupt(usb_dev_handle *udev,
* static int nut_libusb_get_interrupt(usb_dev_handle *udev,
* unsigned char *buf, int bufsize, int timeout)
*/
static int libusb_get_interrupt(
static int nut_libusb_get_interrupt(
usb_dev_handle *udev,
usb_ctrl_charbuf buf,
usb_ctrl_charbufsize bufsize,
Expand All @@ -1008,10 +929,10 @@ static int libusb_get_interrupt(
ret = usb_clear_halt(udev, 0x81);
}

return libusb_strerror(ret, __func__);
return nut_libusb_strerror(ret, __func__);
}

static void libusb_close(usb_dev_handle *udev)
static void nut_libusb_close(usb_dev_handle *udev)
{
if (!udev) {
return;
Expand Down Expand Up @@ -1058,12 +979,12 @@ static char *strcasestr(const char *haystack, const char *needle) {
usb_communication_subdriver_t usb_subdriver = {
USB_DRIVER_NAME,
USB_DRIVER_VERSION,
libusb_open,
libusb_close,
libusb_get_report,
libusb_set_report,
libusb_get_string,
libusb_get_interrupt,
nut_libusb_open,
nut_libusb_close,
nut_libusb_get_report,
nut_libusb_set_report,
nut_libusb_get_string,
nut_libusb_get_interrupt,
LIBUSB_DEFAULT_CONF_INDEX,
LIBUSB_DEFAULT_INTERFACE,
LIBUSB_DEFAULT_DESC_INDEX,
Expand Down
Loading

0 comments on commit 6443582

Please sign in to comment.