[Nut-upsdev] [nut-commits] svn commit r1074 - in trunk: .

[Peter Selinger]

Arjen de Korte wrote:
> 
> >> In addition to this, in order to make this situation occur in the real
> >> world
> >>
> >> 1) both must have the same VendorID:ProductID, Product- and Modelnames
> >> and
> >> Serialnumber (ie, they can't be distinguished through a regular
> >> expression
> >> matcher)
> >> 2) the numeric HIDPaths must not only lead to different, but also
> >> conflicting values in the UPS
> >>
> >> While I agree that the first is quite possible (we've seen examples for
> >> Tripplite for instance), I doubt the second will be.
> >
> > Your doubts are probably warranted; however, I have seen several
> > devices with small bugs in their report descriptor; it's quite
> > conceivable that they would fix them without updating the version
> > number. Also, I don't like to rely on assumptions about the sanity of
> > vendors; they are often misplaced (e.g. Belkin does not even comply
> > with parts of the USB specification).
> 
> We can already handle that, use 'matching = delayed' and the usbhid-ups
> driver will read (and parse) the report descriptor every time is needs to
> reconnect. The reason for not doing this all the time, is that parsing the
> report descriptor takes a relatively long time (a few seconds on my
> Evolution 650) while all the time we will hang around in upsdrv_initinfo()
> and are essentially blind for updates from the UPS and also won't respond
> to the server.
> 
> >> It would mean that the vendor would need two separate USB versions
> >> for their bundled Windows software also (after all, these devices
> >> can't be distinguished from one another by 1).
> > Not if the Windows software actually parses the HID tree. It is the
> > usage paths (such as UPS.Flow.ConfigFrequency) that are fixed in the
> > bundled driver, not their mapping to physical report numbers. The
> > latter are probably generated by the firmware compiler and could
> > easily change without notice.
> 
> The nasty side effect here, is that reading and parsing the report
> descriptor requires claiming the device. If this happens to be *not* the
> device we need to reconnect to, this means that the driver that *is*
> connected to it will need to reclaim it. I have not found a way yet how to
> do that cleanly.

I could be wrong, but I thought that detaching a running driver only
works for kernel drivers (keyboard, mouse). If a user level driver has
claimed the device, then any other process claiming the device should
simply fail. Have you observed otherwise? 
 
> >> While theoretically that may be possible, it would lead to serious
> >> problems when people upgrade from one type of UPS to another from
> >> the same vendor. It would be insane to do that, so when at least the
> >> VendorID:ProductID matches, you shouldn't have to expect conflicting
> >> HIDPaths and reading the report descriptor again for that reason
> >> only is not needed.
> > The report descriptor is a mapping from HIDPaths to physical report
> > locations.  Your argument proves that HIDPaths should not change when
> > people upgrade; however, without re-reading the report descriptor,
> > this does not imply that the physical report locations don't change.
> 
> If you're worried about that, 'matching = delayed' is for you. I don't
> think this should be the default though, since for devices that support to
> be uniquely matched without reading the report descriptor, it would mean a
> lot of additional effort for no good reason at all.

I think the extra computational effort at runtime is negligible in
real terms (remember that until recently, we used to parse the report
descriptor each time any variable was accessed). An extra
allocation/deallocation is unnecessary, provided that the report
descriptor is an exact match (which the matcher should check). So the
main additional effort is a programming effort, which I understand you
have already done.

Note that in the case where there is a unique matching device,
'matching = delayed' never does anything bad for the user, i.e., its
behavior, as visible to the user, is identical to the current default
behavior (and therefore backwards compatible). The only situations in
which the behaviors differ is when there are multiple similar devices
present, in which case 'matching = delayed' is the safer option, isn't
it? So perhaps one could argue that it would be a good default.

-- Peter