Hello David, First, thanks for this post. The Windows port is still in beta stage and this kind of post if very useful.
On Tue, 2011-06-28 at 20:54 -0400, David Bolen wrote: > David Bolen <[email protected]> writes: > > > Attached below is an excerpt of debugging output from usbhid-ups. It > > would appear that the interrupt and query failures have reasonably > > appropriate errors, so maybe it's just a difference in how such errors > > reflect under Windows vs. *nix. > > Following up on my own note, I think I have a theory on what is > happening, but am having trouble compiling to test a change as while I > have an pre-existing Mingw/MSYS toolchain, it doesn't have a full > autoconf toolchain yet. If anyone might have a pre-existing > "configure" script for Win32, that would help jump start things as I > could configure from there. Setting up the build environnent in Windows is kind of complex. I will send you my configure file if it can help you. Don't hesitate to ask if you are still stuck. > > I think I've run into two items. The first is that the errors from > checking the interrupt pipe in upsdrv_updateinfo aren't actually > checked for cases where they could identify a disconnected UPS, so it > just assumes no information and continues. That's not Win32 specific. > I'm not quite sure why it loops so quickly, as opposed to the top > level polltime, but perhaps the state of the driver connection is such > that it immediately satisfies the WaitForMultipleObjects call in the > Win32 version of dstate_poll_fds. It's just a silly bug. The timeout of WaitForMultipleObject is supposed to be in milliseconds but it is passed in seconds. > > Anyway, normally it looks like that would be short-lived anyway since > the periodic full poll would detect the disconnect a modest time > later. Here I think I run into an assumption in the libhid code that > errno always identifies all errors, while in the case of a win32 > failure inside of libusb-win32, it's only the result code of the > function calls that reflect the error, not errno. So USB I/O level > errors while polling are getting hidden. Another bug. errno has to be filled with the result of GetLastError() to be really useful. > > After shrinking the poll time and letting things run a bit after a > cable disconnect, I got a few other errors (from libusb_get_report) > interleaved such as: > > 5.843750 Got 0 HID objects... > 5.843750 Quick update... > 5.859375 upsdrv_updateinfo... > 5.859375 libusb_get_interrupt: libusb0-dll:err [submit_async] > submitting > request failed, win error: The device does not recognize the command. > 5.859375 Got 0 HID objects... > 5.859375 Quick update... > 5.859375 libusb_get_report: libusb0-dll:err [control_msg] sending > control > message failed, win error: The device does not recognize the command. > 5.859375 Can't retrieve Report 16: No error > 5.859375 libusb_get_report: libusb0-dll:err [control_msg] sending > control > message failed, win error: The device does not recognize the command. > 5.859375 Can't retrieve Report 15: No error > 5.859375 upsdrv_updateinfo... > 5.859375 libusb_get_interrupt: libusb0-dll:err [submit_async] > submitting > request failed, win error: The device does not recognize the command. > > From looking at the libusb-win32 source, this would be the result of a > Win32 ERROR_BAD_COMMAND code which is turned into a -EIO result for > the usb_control_message call. But in this case, it's only the result > that has the error, as it's a translation from the Windows internal > error and not from any original errno value. > > It appears that the NUT libusb.c module passes the return code on > fine, but then it's suppressed with libhid.c (refresh_report_buffer), > and eventually HIDGetDataValue, who just gets a generic -1 code, > assumes it can pass along -errno to the usbhid-ups driver, which at > that point has no error (yielding the "No error" in the "Can't > retrieve Report" message. > > It seems like adjusting functions along the path to all return the > function result code as opposed to just -1 would work but also feels > like a riskier change. I'm wondering if perhaps just making the lower > level libusb entry points (probably just #if WIN32) assign the result > code of the libusb-win32 driver calls to errno to ensure it represents > the failures would be cleaner and help obscure the Windows differences? I guess that managing correctly errno with GetLasterror should be enough. > > -- David > > PS: In reviewing the libusb-win32 source it also looks like in some > cases it can return either EINVAL or ENOMEM, neither of which are > checked for in the error checking path for usbhid-ups, so probably > could get added, if only to the WIN32 block. > Indeed, after setting errno correctly using GetLastError(), I add EINVAL as a disconnection cause and it seems to work now. Adding ENOMEM might be useful too. May I privately mail you a link to a replacement driver with those fixes for testing ? Or a patch if you manage to set up the build environnement ? Regards, Fred -- Team Open Source Eaton - http://powerquality.eaton.com -------------------------------------------------------------------------- _______________________________________________ Nut-upsuser mailing list [email protected] http://lists.alioth.debian.org/cgi-bin/mailman/listinfo/nut-upsuser
