[Nut-upsdev] usbhid-ups pull request for consideration

[Charles Lepple]

Hi Stephen,

It's been a little busy at home, so I haven't had a chance to go over the pull request in depth, but I didn't want you to think it hadn't been considered.

On Apr 19, 2014, at 1:23 AM, Stephen J. Butler wrote:

> I've added a pull request with changes to drivers/libusb and drivers/usbhid-ups mainly. They are to resolve issues I encountered when setting up a Tripp Lite SMART1500LCDT, but I think they're generally useful to the community. Here is the pull request comment I sent:
> 
> https://github.com/networkupstools/nut/pull/122
> 
> Had trouble getting nut to work for my new Tripp Lite SMART1500LCDT. So in best open source fashion, I hacked at it till it worked. Couple things in this pull request:
> 
> 1. Scale values for some of the SMART1500LCD parameters.

Thanks!

> 2. With my hardware and kernel, usbhid-ups wouldn't run more than 10 minutes before exiting with a fatal error. This seems to be related to holding usb_claim_interface() the entire life of the program. I've refactored claim_interface/release_interface calls to be around only when work is being done. Now usbhid-ups has run for 24+ hours without issue.

Given how long we have been running with code that claims the interface at driver startup, I am a wary about changing this behavior. Have you had a chance to raise this with the Linux USB folks? I don't see any reason why claiming the interface for an indefinite amount of time shouldn't be expected to work.

We will, of course, need to test this on other models of UPS which are supported by usbhid-ups.

Assuming for a moment that the fatal error is due to a regression in the kernel, are there other general advantages to only claiming when needed?

> 3. The very nature of USB in a home means that sometimes the driver will lose communications with the UPS (accidently unplug the hub, needed a port for a camera, etc). This shouldn't be a fatal error. I've tried to detect these situations and let usbhid-ups retry gracefully. Also, at startup the UPS no longer has to be immediately present. It will keep retrying upsdrv_initups() until it finds one (only implemented for usbhid-ups, but someone could do the work for other drivers).

To be honest, the current approach to reattaching to USB devices has always bothered me. We have had a few hard-to-find memory management bugs in the reattach code over the years, and there are race conditions when a USB device doesn't cleanly reconnect. Ensuring complete testing coverage over all of those reconnection cases is much harder.

I would be a lot more excited about retesting all of this if we could somehow leverage udev (or equivalent, on non-Linux) to make this event-driven. Then, the individual drivers don't have to deal with reconnection, and when the UPS disappears for any reason, the driver simply exits. The problem for libusb-0.1 is that I don't know of a clean way to pass a udev device specification to usb_open(). (Insert rant about most USB devices not having unique serial numbers in their device descriptors.)

Thoughts? Has anyone implemented a similar udev-to-application handoff in libusb-1.x?

-- 
Charles Lepple
clepple at gmail