[Nut-upsdev] NUT and libhid

[Peter Selinger]

Arjen de Korte wrote:
> 
> 
> > I think the differences between NUT HID and libhid are probably
> > considerable at this point, as I've spent some time last year updating
> > the former.
> >
> > - the report buffer,
> 
> Thanks for reminding me, so it is you I should nag with questions about
> the report buffering, not Charles.
> 
> > - introduced a data structure to store the parsed report descriptor,
> >   rather than re-parsing it each time it is used,
> > - much improved conversion functions for physical and logical units,
> > - cleaned up many parts of the code, eliminated global variables where
> >   possible,
> 
> Regarding the latter, do you have any particular reason for dynamically
> allocating the report buffer 'rbuf'? 

If libhid wants to be a library one day, it should be reentrant. It's
true that a NUT driver connects to one and only one HID device.
However, the same may not be true for a generic application, which
could connect to many devices at once. So in the interest of good
style, all global variables should one day be replaced by a hid_state
variable that gets passed to each function call.

Currently, there are still two global variables: pDesc and rbuf. They
still exist because my strategy for updating the code was to make a
series of small, unobtrusive and easy-to-test changes, rather than a
single major rewrite. For the reason outlined above, I think the goal
should be to eliminate these. 

This said, the underlying libusb-0.1.10a library also has global
state. However, the global state of that library is restricted to the
usb bus structure, which is truly global, in the sense that one
computer has only one USB system, to which all busses are
connected. Each device is accessible via its own hid_dev_handle_t
*udev. So one could make a wrapper around libusb-0.1.10a that is
mostly reentrant. 

> As far as I can see, we always have
> one (and exactly one), which is used by all functions that operate on the
> report buffer. Instead of free'ing and allocating it each time we
> (re)connect to the UPS, I can imagine that it is less hassle to make this
> a global variable and just invalidate the timestamps (by zero'ing them).
> In that case, instead of calling free_report_buffer() and
> new_report_buffer(), we call
> 
> static reportbuf_t	rbuf;
> 
> [...]
> 
> void HIDClearReportBuffer(void)
> {
> 	int i;
> 
> 	for (i=0; i<256; i++) {
> 		rbuf.ts[i] = 0;
> 	}
> }
> 
> This would satisfy my need for a way to invalidate the contents of the
> report buffer, so that after a successful instcmd() and setvar() we would
> just call HIDClearReportBuffer() and all is well again.

All you have to do is set the timestamp to 0 for the report(s) you
would like to invalidate. I don't see why making it a global variable
makes this any easier or harder.  -- Peter

> > - and probably more.
> >
> > I don't know how many changes you made to libhid since the fork;
> > perhaps it's easier to keep improving NUT's HID?
> 
> The latter would be a lot easier IMHO...
> 
> Best regards, Arjen
>