On 20/02/14(Thu) 14:32, Reyk Floeter wrote: > On 20.02.2014, at 12:23, Martin Pieuchot <[email protected]> wrote: > >> [...] > >> + /* > >> + * init usbdata > >> + * those commands width were identified from observing apcupsd/ugen(4) > >> + * data exchange > >> + */ > >> + sc->sc_usbdata[0].cmd = UPD_CMD_GET_BATTNOMV; > >> + sc->sc_usbdata[0].dlen = 0x03; > >> + sc->sc_usbdata[0].hkind = hid_feature; > >> + sc->sc_usbdata[1].cmd = UPD_CMD_GET_BATTCURV; > >> + sc->sc_usbdata[1].dlen = 0x02; > >> + sc->sc_usbdata[1].hkind = hid_feature; > >> + sc->sc_usbdata[2].cmd = UPD_CMD_GET_BATTLOAD; > >> + sc->sc_usbdata[2].dlen = 0x03; > >> + sc->sc_usbdata[2].hkind = hid_feature; > >> + sc->sc_usbdata[3].cmd = UPD_CMD_GET_UPSMISC; > >> + sc->sc_usbdata[3].dlen = 0x03; > >> + sc->sc_usbdata[3].hkind = hid_feature; > > > > Even if for the moment you're only dealing with Feature report, be > > prepared to support Input and Ouput too, see below. > > > > But instead of hardcoding magic values, you should iterate over the > > report IDs available and try to find the Usage ID your driver supports. > > Have a look at the various HUP_* macro, HID_USAGE2() hid_is_collection() > > and friend. > > > > Maybe you'll find out that it's easier to split your driver into various > > drivers, for example one dealing with the Power Device Page and another > > one with the Battery System Page... > > > > If your driver is generic enough you might even have various upd(4) > > instances, one for each report ID, that just parse the available usages > > and map them to sensors. > > > > I don't like this approach. Because in the end I have “one” UPS with > several sensors. Splitting upd(4) into several devices makes it impossible > to associate them to a specific UPS. It gets worse when attaching two or > more UPSes to the system (which might happen). Having upd0, upd1 etc. > per UPS makes much more sense.
I agree that's having only one driver makes more sense and should be easier to interface with userland. I just wanted to point out that HID Power Devices should be reporting data using four pages of which 2 are properly defined and that it might make some sense to split the code dealing with these pages. You'll tell me :) > That’s why I proposed the upd(4) sensor super-driver that collects > information from the attached HIDs and presents them with a single sensor. > With your HID simplification diff, this becomes even easier and a bit cleaner. I know that our drivers are confusing, but when you say HID here you mean report ID, in the end you want one HID (UPS) one driver and I agree totally. Martin
