[Nut-upsdev] r1059:1067

[Arjen de Korte]

> thanks for today's changes to usbhid-ups. These are very sensible
> changes and were long overdue. It was of course suboptimal to parse
> the usage strings each time, rather than doing it once and putting
> them in a data structure. I am beginning to believe that when you are
> finished, usbhid-ups will actually be (gasp) elegant!

That will take some time still. I had most of these changes already in my
working copy, but split them up in individual patches. That way it is
easier to see where things break. It's not that I wrote all of this in one
day... :-)

> The difference between Features and Inputs has been on my list of
> stuff to look into for a while. When I started hacking this driver, I
> didn't understand the difference, that one is for polled data and the
> other for interrupted data. The USB specification is not very crisply
> written when it comes to this point. When I finally understood the
> difference, I had no more energy to program.

It was a steep learning curve for me also, but I think I understand the
general concept now. I too was confused seeing reports marked as 'Feature'
and 'Input', especially since they appear to yield identical information
in the HIDDumpTree().

> If I remember correctly, the report buffer does not handle Features
> and Inputs correctly. Technically, Inputs don't need to be buffered,
> since they are never polled. However, if an input arrives, then a
> later poll of the same variable should return the current value, not
> the one from a previous Feature report. So interrupt Inputs are
> currently stored in the report buffer, overwriting the corresponding
> Feature report.

As far as I can see, it does work as intended, but I have to dig deeper
into the report buffering to be absolutely sure about that.

One thing I don't like about the present way it works is the constant
(de)allocating of memory. The impact on the load will be insignificant,
but the allocation may fail. This requires handling an out-of-memory
condition, something I really would like to avoid once the driver is
running. As far as I can see, we already know what to expect (regarding
lenght), so it should be possible to allocate the full buffer when
creating it.

> This is fine as long as Input and Feature reports are identical and
> are numbered identically, which is the case for all UPS that I have
> seen so far, but is perhaps not required by the USB standard.

That's a good point I had not thought about yet. Will have to investigate
that further.

> A cleaner solution would be not to write interrupt transfers to the
> buffer, but to mark the entire report buffer as expired after an
> interrupt transfer occurs.

That might be an option too. Thanks for the hint.

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