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

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. 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),

2) we never re-read the report descriptor, and therefore never
   compared that it was identical to the previous one,

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.

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. 

In other words, reconnect is exactly the same as connecting the first
time. 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.

-- Peter

>> - 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.
> 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.
> 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).
> 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.

Best regards, Arjen
-- 
Eindhoven - The Netherlands
Key fingerprint - 66 4E 03 2C 9D B5 CB 9B  7A FE 7E C1 EE 88 BC 57