Greetings, I have been using the bcmxcp_usb driver heavily over the last two years as part of a very successful product, first from NUT 2.4.1 and now from NUT 2.6.0. Now I have a need to do setvar of the outlet.n.delay.start variables, so recently I modified bcmxcp.c to do that. I've tested it and it works well for my purposes. My plans are to begin shipping that modified driver as part of our product by the end of the year. Initially, we plan to patch the stock bcmxcp.c file from 2.6.0 as part of our build, but long-term it would be nice if the changes were incorporated into the stock bcmxcp.c file so as we move to new versions of NUT that we don't need to constantly review the patch. So, I would like to know if the NUT developers are interested in adopting these changes and if so, how can I help get them into the stock driver? Attached is my patch file. Note that the comments are a little verbose but that is only to make sure we comply with the GPL so I expect that those would be removed if the patch is incorporated into the stock bcmxcp.c. Thanks, Rich Wrenn System Architect, DataDirect Networks -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.alioth.debian.org/pipermail/nut-upsuser/attachments/20110910/feb93dfe/attachment.html> -------------- next part -------------- A non-text attachment was scrubbed... Name: bcmxcp-RW.patch Type: application/octet-stream Size: 4801 bytes Desc: bcmxcp-RW.patch URL: <http://lists.alioth.debian.org/pipermail/nut-upsuser/attachments/20110910/feb93dfe/attachment.obj>
Hi Rich, 2011/9/10 Rich Wrenn <rwrenn at ddn.com>> Greetings,**** > > ** ** > > I have been using the bcmxcp_usb driver heavily over the last two years as > part of a very successful product, first from NUT 2.4.1 and now from NUT > 2.6.0.**** > > ** ** > > Now I have a need to do setvar of the outlet.n.delay.start variables, so > recently I modified bcmxcp.c to do that. I've tested it and it works well > for my purposes. My plans are to begin shipping that modified driver as > part of our product by the end of the year.**** > > ** ** > > Initially, we plan to patch the stock bcmxcp.c file from 2.6.0 as part of > our build, but long-term it would be nice if the changes were incorporated > into the stock bcmxcp.c file so as we move to new versions of NUT that we > don't need to constantly review the patch.**** > > ** ** > > So, I would like to know if the NUT developers are interested in adopting > these changes and if so, how can I help get them into the stock driver?*** > * > > ** >sure we are interested. and you did the necessary to get it merged.> Attached is my patch file. Note that the comments are a little verbose > but that is only to make sure we comply with the GPL so I expect that those > would be removed if the patch is incorporated into the stock bcmxcp.c. >I don't have any specific remark on your patch, apart that outlet support was scheduled for XCP too. I've taken this opportunity to add also "outlet.n.shutdown.return" setting, and this fix a stack smashing: the answer buffer was too short, and there is no max len to enforce limits. This occured when using the 2 delay bytes (ie, having a value higher than 255). so, thanks very much for your patch, I've merged it in the trunk, r3217. it will be available in 2.6.2, at the end of this week. cheers, Arnaud -- Linux / Unix Expert R&D - Eaton - http://powerquality.eaton.com Network UPS Tools (NUT) Project Leader - http://www.networkupstools.org/ Debian Developer - http://www.debian.org Free Software Developer - http://arnaud.quette.free.fr/ -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.alioth.debian.org/pipermail/nut-upsuser/attachments/20110913/4dc29904/attachment.html>
Rich,
Thanks for submitting this patch.
One small thing I noticed on the Solaris buildbot:
../../drivers/bcmxcp.c: In function `setvar':
../../drivers/bcmxcp.c:1733: warning: comparison is always false due
to limited range of data type
The code:
int setvar (const char *varname, const char *val)
{
[...]
int16_t sec; /* limit the size of the timer, to avoid
overflow */
[...]
sec = atoi(val);
/* Check value:
* 0-32767 are valid values
* -1 means no Automatic off or restart
* for Auto Off Delay:
* 0-30 are valid but ill-advised */
if (sec < -1 || sec > 0x7FFF) {
return STAT_SET_INVALID;
}
I think the value of atoi() should probably be assigned to a larger
int variable, and leave the conditional there to guard against out-of-
range values.
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.alioth.debian.org/pipermail/nut-upsuser/attachments/20110913/133d9ec8/attachment-0001.html>