[Nut-upsdev] Fwd: Initial review of ONEAC driver

Arnaud Quette aquette.dev at gmail.com
Tue Mar 6 22:04:26 UTC 2012


Hi Bill,

good news: I've done the review (took me a coupe of hours) and the
commit (r3485):
http://trac.networkupstools.org/projects/nut/changeset/3485

you'll find below the summary.

2012/2/29 Charles Lepple <clepple at gmail.com>:
> [Bill: I think the list server didn't recognize your address below.
> Forwarding manually.]
>
> From: Bill Elliot <bill at handiwerks.com>
> Subject: Initial review of ONEAC driver
> Date: February 28, 2012 1:14:27 PM EST
> To: nut-upsdev at lists.alioth.debian.org
>
>
> At Arnaud's request, here is the 'svn di' patch for the Oneac driver and
> man page.
>
> The FSD status sending is controlled by the presence of a flag in
> ups.conf.  The default is to NOT implement this behavior.
>
> Looking forward to comments/feedback...be nice to the new guy!

- build ok (no warning)
- variables names => ok
- version bump => ok
- manpage: a few mods (offdelay, 9600 mention, HW list) and adjustments
- .ch header revamp (cosmetic)
- lot of indent and extraneous spaces/tabs (cosmetic)
- the general code review is ok. no change in the foundations.
- I've discarded FSDflag related stuffs, not needed as per our recent
discussion. You may want to reconsider the situation for ON too (I've
not understood the comment!)
- I've modified the driver option "shuttime" for "offdelay", which is
more consistent with existing equivalent (though it still needs
standardization)
- I've also discarded scripts/upower/95-upower-hid.rules (auto-generated)

a side comment: I still have to create a common unicode conversion
function (in common.c),  since there are several instances of such
code throughout (look for "unicode" in blazer_usb.c). that would
eliminate "EliminateLeadingZeroes" ;)

Finally, data/driver.list.in needs completion for at least OZ and OB.

Thanks a lot for your excellent contrib ^_^

cheers,
Arno



More information about the Nut-upsdev mailing list