Skip to content

Commit

Permalink
Merge pull request #2571 from jimklimov/issue-2562-retry-NULL-IDs
Browse files Browse the repository at this point in the history 
`drivers/libusb{0,1}.c`: `{,nut_}libusb_open()`: try to re-fetch `curDevice->Vendor`, `Product`, `Serial` if `NULL`, after claiming it by other criteria
  • Loading branch information
@jimklimov
jimklimov authored Aug 11, 2024
2 parents f5022a1 + 649dac1 commit bdffc30
Show file tree
Hide file tree
Showing 5 changed files with 160 additions and 4 deletions.
2 changes: 1 addition & 1 deletion NEWS.adoc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ https://github.com/networkupstools/nut/milestone/11
* General suggestion from `possibly_supported()` message method for devices
with VendorID=`0x06da` (Phoenixtec), seen in some models supported by
MGE HID or Liebert HID, updated to suggest trying `nutdrv_qx`. [#334]
* MGE HID list of `mge_model_names[]` was extended for Eaton 5PX and 5SC
* MGE HID list of `mge_model_names[]` was extended for Eaton 9E, 5PX and 5SC
series (largely guessing, feedback and PRs for adaptation to actual
string values reported by devices via USB are welcome), so these devices
would now report `battery.voltage` and `battery.voltage.nominal`. [#2380]
Expand Down
1 change: 1 addition & 0 deletions data/driver.list.in
View file
Original file line number Diff line number Diff line change
Expand Up @@ -362,6 +362,7 @@
"Eaton" "ups" "5" "5S" "USB port" "usbhid-ups"
"Eaton" "ups" "5" "5SC" "USB port" "usbhid-ups"
"Eaton" "ups" "5" "5P" "USB port" "usbhid-ups"
"Eaton" "ups" "5" "9E" "USB port" "usbhid-ups"
"Eaton" "ups" "5" "9SX" "USB port" "usbhid-ups"
"Eaton" "ups" "5" "9PX" "USB port" "usbhid-ups"
"Eaton" "ups" "5" "9PX Split Phase 6/8/10 kVA" "USB port" "usbhid-ups"
Expand Down
56 changes: 56 additions & 0 deletions drivers/libusb0.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -542,6 +542,62 @@ static int libusb_open(usb_dev_handle **udevp,

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");
}

/* Did the driver provide a callback method for any further
* device acceptance checks (e.g. when same ID is supported
* by several sub-drivers, differing by vendor/model strings)?
*/
if (!callback) {
return 1;
}
Expand Down
70 changes: 69 additions & 1 deletion drivers/libusb1.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@
#include "nut_stdint.h"

#define USB_DRIVER_NAME "USB communication driver (libusb 1.0)"
#define USB_DRIVER_VERSION "0.48"
#define USB_DRIVER_VERSION "0.49"

/* driver description structure */
upsdrv_info_t comm_upsdrv_info = {
Expand Down Expand Up @@ -601,6 +601,74 @@ static int nut_libusb_open(libusb_device_handle **udevp,

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 = libusb_get_string_descriptor_ascii(udev, dev_desc.iManufacturer,
(unsigned char*)string, sizeof(string));
if (ret > 0) {
curDevice->Vendor = strdup(string);
if (curDevice->Vendor == NULL) {
libusb_free_device_list(devlist, 1);
fatal_with_errno(EXIT_FAILURE, "Out of memory");
}
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 = libusb_get_string_descriptor_ascii(udev, dev_desc.iProduct,
(unsigned char*)string, sizeof(string));
if (ret > 0) {
curDevice->Product = strdup(string);
if (curDevice->Product == NULL) {
libusb_free_device_list(devlist, 1);
fatal_with_errno(EXIT_FAILURE, "Out of memory");
}
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 = libusb_get_string_descriptor_ascii(udev, dev_desc.iSerialNumber,
(unsigned char*)string, sizeof(string));
if (ret > 0) {
curDevice->Serial = strdup(string);
if (curDevice->Serial == NULL) {
libusb_free_device_list(devlist, 1);
fatal_with_errno(EXIT_FAILURE, "Out of memory");
}
break;
}
retries--;
upsdebugx(1, "%s get iSerialNumber failed, retrying...", __func__);
}
upsdebugx(2, "- Serial Number: %s", curDevice->Serial ? curDevice->Serial : "unknown");
}

/* Did the driver provide a callback method for any further
* device acceptance checks (e.g. when same ID is supported
* by several sub-drivers, differing by vendor/model strings)?
*/
if (!callback) {
libusb_free_config_descriptor(conf_desc);
libusb_free_device_list(devlist, 1);
Expand Down
35 changes: 33 additions & 2 deletions drivers/mge-hid.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@
# endif
#endif

#define MGE_HID_VERSION "MGE HID 1.47"
#define MGE_HID_VERSION "MGE HID 1.48"

/* (prev. MGE Office Protection Systems, prev. MGE UPS SYSTEMS) */
/* Eaton */
Expand Down Expand Up @@ -131,7 +131,8 @@ typedef enum {
MGE_PULSAR_M_2200,
MGE_PULSAR_M_3000,
MGE_PULSAR_M_3000_XL,
EATON_5P = 0x500 /* Eaton 5P / 5PX / 5SC series */
EATON_5P = 0x500, /* Eaton 5P / 5PX / 5SC series */
EATON_9E = 0x900 /* Eaton 9E entry-level series */
} models_type_t;

/* Default to line-interactive or online (ie, not offline).
Expand Down Expand Up @@ -489,6 +490,7 @@ static const char *mge_battery_voltage_nominal_fun(double value)

case MGE_PULSAR_M:
case EATON_5P:
case EATON_9E: /* Presumably per https://github.com/networkupstools/nut/issues/1925#issuecomment-1562342854 */
break;

default:
Expand All @@ -514,6 +516,7 @@ static const char *mge_battery_voltage_fun(double value)
case MGE_EVOLUTION:
case MGE_PULSAR_M:
case EATON_5P:
case EATON_9E: /* Presumably per https://github.com/networkupstools/nut/issues/1925#issuecomment-1562342854 */
break;

default:
Expand Down Expand Up @@ -1144,6 +1147,34 @@ static models_name_t mge_model_names [] =
{ "Eaton 5SC", "2200", EATON_5P, NULL },
{ "Eaton 5SC", "3000", EATON_5P, NULL },

/* Eaton 9E entry-level series per discussions in
* https://github.com/networkupstools/nut/issues/1925
* https://github.com/networkupstools/nut/issues/2380
* https://github.com/networkupstools/nut/issues/2492
*/
{ "Eaton 9E", "1000", EATON_9E, "9E1000" },
{ "Eaton 9E", "1000i", EATON_9E, "9E1000i" },
{ "Eaton 9E", "1000iau", EATON_9E, "9E1000iau" },
{ "Eaton 9E", "1000ir", EATON_9E, "9E1000ir" },
{ "Eaton 9E", "2000", EATON_9E, "9E2000" },
{ "Eaton 9E", "2000i", EATON_9E, "9E2000i" },
{ "Eaton 9E", "2000iau", EATON_9E, "9E2000iau" },
{ "Eaton 9E", "2000ir", EATON_9E, "9E2000ir" },
{ "Eaton 9E", "3000", EATON_9E, "9E3000" },
{ "Eaton 9E", "3000i", EATON_9E, "9E3000i" },
{ "Eaton 9E", "3000iau", EATON_9E, "9E3000iau" },
{ "Eaton 9E", "3000ir", EATON_9E, "9E3000ir" },
{ "Eaton 9E", "3000ixl", EATON_9E, "9E3000ixl" },
{ "Eaton 9E", "3000ixlau", EATON_9E, "9E3000ixlau" },

/* https://github.com/networkupstools/nut/issues/1925#issuecomment-1609262963
* if we failed to get iManufacturer, iProduct and iSerialNumber but saw
* the UPS.Flow.[4].ConfigApparentPower (the "2000" or "3000" part here)
*/
{ "unknown", "1000", EATON_9E, "9E1000i (presumed)" },
{ "unknown", "2000", EATON_9E, "9E2000i (presumed)" },
{ "unknown", "3000", EATON_9E, "9E3000i (presumed)" },

/* Pulsar M models */
{ "PULSAR M", "2200", MGE_PULSAR_M_2200, NULL },
{ "PULSAR M", "3000", MGE_PULSAR_M_3000, NULL },
Expand Down

0 comments on commit bdffc30

Please sign in to comment.