Nut upsdev - Feb 2012 - Fwd: Initial review of ONEAC driver

[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!
> 
> Bill
> 
> 

-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.alioth.debian.org/pipermail/nut-upsdev/attachments/20120228/0c341bef/attachment-0002.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: OneacDiff
Type: text/x-patch
Size: 45642 bytes
Desc: not available
URL:
<http://lists.alioth.debian.org/pipermail/nut-upsdev/attachments/20120228/0c341bef/attachment-0001.bin>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.alioth.debian.org/pipermail/nut-upsdev/attachments/20120228/0c341bef/attachment-0003.html>

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