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

[Peter Selinger]

Arjen de Korte wrote:
> 
> >> - HIDOpenDevice() will now handle closing the device on reload if
> >> needed, so that HIDCloseDevice() can now really close it and free
> >> the allocated memory for report buffer and parsed report descriptor.
> > If I remember correctly, the old behavior on "reopen" was to keep the
> > previous report descriptor, and try to find a device that matches the
> > previously seen device exactly.
> 
> Indeed.
> 
> > This was never perfect:
> >
> > 1) it's not always possible to identify the device uniquely, when
> >    there are two identical devices (and not all models include a
> >    serial number),
> 
> True, but in that case you have a big problem if the outputs of these
> UPSes don't feed the same system(s). How would you know which clients to
> shutdown if one of them goes critical? Only if the UPSes are exactly the
> same and have the same power values (!) for all clients, this would not
> lead to problems.
> 
> > 2) we never re-read the report descriptor, and therefore never
> >    compared that it was identical to the previous one,
> 
> I agree. Theoretically, someone could have connected the UPS to a
> different system in the mean time, have changed some R/W values and we
> would never notice those changes.

No, changeable R/W values are not part of the report descriptor. The
report descriptor defines the usage tree and is usually static. One
could use it to recognize a device "more uniquely", as it serves as a
kind of fingerprint of the device. (For example, if memory serves
there are TrippLite models that contain bugs in the report descriptor
of some, but not all, devices of a given model). So the report
descriptor (or a CRC-32 checksum of it) could in principle be added to
the exact-matcher to make it more exact. (I think the reason we
haven't done this before is that the report descriptor requires
opening the device, which requires disconnecting the kernel driver,
which could lead to other trouble if the device ended up not
matching). 

In any case, R/W values would have to be re-read, as the set of
variables might have changed on reconnect, if the device connected to
didn't happen to be the same one (i.e., if the exact-matcher fails and
we fall back to the inexact method)
 
> > 3) there was at least one report of a device that looked different at
> >    reconnection time (didn't show the vendor + product strings), and
> >    therefore couldn't be matched.
> 
> Ouch!
> 
> > Perhaps a better behaviour for "reconnect" is to connect to any
> > eligible UPS, even if it's not precisely the same one as the
> > disconnected one, and then simply re-load and re-parse all the
> > data.
> 
> That is already the implementation now. On reopen, we first try the
> exact_matcher and if that fails, the regex_matcher. I prefer to keep it
> that way, so that only in case the exact match doesn't work, we risk
> selecting the 'wrong' device.

True.
 
> > In other words, reconnect is exactly the same as connecting the first
> > time.
> 
> I disagree here, although I think it is a good idea to reparse the report
> descriptor on reconnect. This does require adding a HU_WALKMODE_RELOAD for
> 'hid_ups_walk', since we need to find the new elements and we must remove
> the NUT variables that are no longer supported (which could be the case if
> we match a different device on reload).

I wasn't sure if it was possible to add or remove variables and
instant commands, after the driver was already running, or if this was
only possible during the initial init(). Does the driver/upsd protocol
allow for the dynamic creation and deletion of variables? If not, the
"inexact matching" fallback is a bit of a leap of faith.
 
> > If people have more than one UPS, and they care which is which,
> > they anyway have to specify the vendorid or productid or whatever, to
> > make it unique.
> 
> This is already the behaviour now. The first time you start a driver, it
> will connect to the first device that matches. If you start it again with
> the same parameters, it will fail, even if another device is free that
> would match.

I didn't follow that. If you have two devices, and you start two
driver instances with the same parameters, the first driver will
connect to the first device found, and the second to the second, I
thought?

-- Peter