Nut upsdev - Sep 2005 - Re: about [ #302111 ] newhidups support for Belkin

I have checked in a cleaned-up version of patch #302111 (Belkin
support) into CVS. See

 cvs diff -uN -r before_PSE_2 -r after_PSE_2

to check the difference. Here is the CHANGES entry:

Tue Sep 13 20:50:14 UTC 2005 / Peter Selinger
<selinger@users.sourceforge.net>

 - newhidups: added Belkin support. Also added some new variables and
   instant commands (some of these were already used by other
   drivers); see new-names.txt.

 - newhidups: fixed return value bug in libusb.c:libusb_open() when no
   device was found.

Please let me know if there's anything you don't like. 

Notes: 

* I documented the new variables and instant commands in
  new-names.txt. Some of them already existed in other drivers, but
  had not yet been documented.

* One exception is the variable ups.serial.internal, which should not
  really exist. The information contained in that variable should
  normally be in ups.serial. The problem is that the Belkin UPS does
  not announce its serial number in the device descriptor (like a good
  HID device should), but in a report. So this info is not available
  at initialization time. So either one needs a specialized function
  for retrieving the Belkin serial number at initialization time, or
  one needs to set the variable ups.serial "after the fact". I could
  not figure out how to do that, so I left the variable
  ups.serial.internal, and a "FIXME" comment, for the time being. 
> - why did you remove the restriction on Belkin ProductID
>  
> (dev->descriptor.idProduct == 0x0551)
> 
> It's not useful for UPS only mfrs, but belkin also sells other
devices...
My Belkin UPS has product ID 0x0980. It seems silly to start making a
long list of Belkin UPS products, because every user would have to
email us their product ID before their device could be supported. A
better strategy seems to support all Belkin devices. They do make
non-UPS devices, but I think it's mostly passive devices like power
strips. I don't know if they make any other USB HID devices. I can see
no harm in trying to connect to the first Belkin device we find; if a
user wants to specify a specific device, they can e.g. use my new
regex mechanism.

I am planning to work on modularizing the code for newhidups a bit, so
that there can be specialized policies for different manufacturers.  A
better permanent solution might be to look at the product string (for
Belkin devices only) to determine whether it's a UPS.
> - about your comment in newhidups.h (specifically "there is not file 
> libusb.h"):  
> 
> +/* there is not file libusb.h, so we have to declare this "by
hand":
> */+int libusb_get_string(int StringIdx, char *string);
> 
> => if you mean the nut file that should go with libusb.c, then it's 
> hid-usb.h
> (the reason is linked to the old libHID, and it's various backends...)
> => if you mean the header from the libusb project, then it's named
usb.h
> and is included in hid-usb.h.
Thanks, I fixed that.

-- Peter

On 9/13/05, Peter Selinger <selinger@mathstat.dal.ca>
wrote:
> if a user wants to specify a specific device, they can e.g. use my new
> regex mechanism.
I haven't had a chance to look at this mechanism, but it sounds cool.

Windows actually has a string-based way of matching devices - it looks
like an URL-encoded string of VID, PID, class, etc. I'll try and send
more info when I get home. The reason why this matters is that we
could have a script that parses peoples' .inf files and converts it to
your matching scheme.

-- 
- Charles Lepple

2005/9/13, Peter Selinger
<selinger@mathstat.dal.ca>:
> 
> I have checked in a cleaned-up version of patch #302111 (Belkin
> support) into CVS. See

seen on the -commits list ;-)


Please let me know if there's anything you don't like.


few things


Notes:
> 
> * I documented the new variables and instant commands in
> new-names.txt. Some of them already existed in other drivers, but
> had not yet been documented.

please discuss (seen Charles ;-) before adding any entry to the naming.
if you find some undocumented vars in drivers, announce these on the -dev 
ml.

- ups.devicechemistry => that would more fit to "battery.type"
(opaque
string?!)
- other vars are ok

Also, don't forget to bump newhidups drv release, as we can attach a release
to the added mfr support...

* One exception is the variable ups.serial.internal, which should
not
> really exist. The information contained in that variable should
> normally be in ups.serial. The problem is that the Belkin UPS does
> not announce its serial number in the device descriptor (like a good
> HID device should), but in a report. So this info is not available
> at initialization time. So either one needs a specialized function
> for retrieving the Belkin serial number at initialization time, or
> one needs to set the variable ups.serial "after the fact". I
could
> not figure out how to do that, so I left the variable
> ups.serial.internal, and a "FIXME" comment, for the time being.

you can stick at ups.serial. If it already exists, it will be updated by 
dstat_setinfo(). Otherwise, it will be created. 
> - why did you remove the restriction on Belkin ProductID
> >
> > (dev->descriptor.idProduct == 0x0551)
> >
> > It's not useful for UPS only mfrs, but belkin also sells other 
> devices...
> 
> My Belkin UPS has product ID 0x0980. It seems silly to start making a
> long list of Belkin UPS products, because every user would have to
> email us their product ID before their device could be supported. A
> better strategy seems to support all Belkin devices. They do make
> non-UPS devices, but I think it's mostly passive devices like power
> strips. I don't know if they make any other USB HID devices. I can see
> no harm in trying to connect to the first Belkin device we find; if a
> user wants to specify a specific device, they can e.g. use my new
> regex mechanism.

don't they sells mice, keyboard and others USB/HID alike devs? 
for sure, it doesn't hurt, but it can make a longer probing time...

I am planning to work on modularizing the code for newhidups a bit,
so
> that there can be specialized policies for different manufacturers. A
> better permanent solution might be to look at the product string (for
> Belkin devices only) to determine whether it's a UPS.

that would result in the same thing as with Vid... Keep in mind that it 
should work with the libhid callback system, ie:
http://svn.ailab.ch/cgi-bin/viewcvs.cgi/libhid/trunk/test/test_libhid.c?rev=207&view=markup
> - about your comment in newhidups.h (specifically "there is not file
> > libusb.h"):
> >
> > +/* there is not file libusb.h, so we have to declare this "by
hand":
> > */+int libusb_get_string(int StringIdx, char *string);
> >
> > => if you mean the nut file that should go with libusb.c, then
it's
> > hid-usb.h
> > (the reason is linked to the old libHID, and it's various
backends...)
> > => if you mean the header from the libusb project, then it's
named usb.h
> > and is included in hid-usb.h.
> 
> Thanks, I fixed that.

ok, seen.

btw, you can remove "John Stamp" reference as he only added things in 
apc-hid.h...

Arnaud
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
http://lists.alioth.debian.org/pipermail/nut-upsdev/attachments/20050914/96c8a1f1/attachment.htm