[Nut-upsdev] Sweex 1000VA UPS (Lakeview Research)

[Arjen de Korte]

Citeren Peter van Valderen <dosperios at gmail.com>:

> I updated my driver to work with nut 2.2.2 six months ago, you can
> find it here: http://ddcrew.com/lakeview-2.2.2.patch

This looks a lot like the lakeview_usb driver we currently have in the  
trunk, which according to a user that tried it, failed to change from  
OL to OB when the mains plug was pulled.

> It works just fine for me. It was never included because it doesn't
> conform to some part of NUT's coding standard, something regarding USB
> if I'm not mistaken.

No, although it violates the coding standards in a couple of ways,  
this is something that might be fixed. But I have my doubts about the  
following lines:

55         MESSAGE_VALUE, INDEX_VALUE, query, sizeof(query), 1000);

Here query is an argument to the call of this function of type
(unsigned char *). On a 32-bit system this will boil down to 4, but on
a 64-bit system most likely 8. This only works correctly if query is
an array (which is not the case here), so you can't use the sizeof  
operator here.

62         ret = usb_interrupt_read(upsdev, REPLY_REQUESTTYPE, reply,
sizeof(REPLY_PACKETSIZE), 1000);

Another wrong use of sizeof. I don't know what the preprocessor will
make out of 'sizeof(6)', but it most likely is not what was intended.

66         usb_comm_fail("Receive error (Request command): COMMAND:  
%x\n", query);

This is a non-portable construction. The (implicit) typecasting from
(unsigned char *) to (unsigned int) is never pretty anyway. It's  
probably better to just mention "Receive error" and use

     upsdebug_hex(2, "request command", query, 4);

to tell what caused it.

> I do not have the time nor do I care enough to
> rewrite the driver in the near future or probably ever, so this patch
> is probably your only bet of getting it working.

In that case we'll probably drop it from the trunk before nut-2.4.0  
will be released. As long as we're not confident that this driver  
actually works, we shouldn't ship it.

Best regards, Arjen
-- 
Please keep list traffic on the list