search

networkupstools / nut

The Network UPS Tools repository. UPS management protocol Informational RFC 9271 published by IETF at https://www.rfc-editor.org/info/rfc9271 Please star NUT on GitHub, this helps with sponsorships!
https://networkupstools.org/
Other
1.81k stars 339 forks source link

APC BackUPS 1200 BR - Rebranded Microsol Solis, wrong packet size, driver not working #231

Open rpvelloso opened 8 years ago

rpvelloso commented 8 years ago

The solis.c driver reads packets of size 25, but I'd sniffed some packets from my usb/serial port and got this:

BA6E88AE0001A49200291E0100000000000E984960609FFE BA6D88AE0001A493002A1E0100000000000E98496060A0FE BA6D88AE0001A496002B1E0100000000000E98496060A4FE BA6D88AE0001A48E002C1E0100000000000E984960609DFE BA6E88AE0001A491002D1E0100000000000E98496060A2FE ... (goes on) BA6D87AE0001A48A000C1F0100000000000E9849606079FE BA6E88AE0001A48C000D1F0100000000000E984960607EFE BA6D88AE0001A489000E1F0100000000000E984960607BFE BA6E88AE0001A48C000F1F0100000000000E9849606080FE

They are all of size 24 and the first and last bytes matches the driver validation (0xBA and 0xFE).

The UPS is an APC Back-UPS 1200 - BZ1200BR, Below is the USB/Serial config output from FreeNAS/FreeBSD:

[root@freenas ~]# usbconfig -d ugen0.5 dump_device_desc ugen0.5: "APC USB SERIAL CONVERTER. APC BY SCHNEIDER ELECTRIC" at usbus0, cfg=0 md=HOST spd=FULL (12Mbps) pwr=ON (100mA)

bLength = 0x0012 bDescriptorType = 0x0001 bcdUSB = 0x0200 bDeviceClass = 0x0002 bDeviceSubClass = 0x0000 bDeviceProtocol = 0x0000 bMaxPacketSize0 = 0x0008 idVendor = 0x051d idProduct = 0xc812 bcdDevice = 0x0100 iManufacturer = 0x0001 "APC BY SCHNEIDER ELECTRIC" iProduct = 0x0002 "APC USB SERIAL CONVERTER." iSerialNumber = 0x0000 "no string" bNumConfigurations = 0x0001

I'm using FreeBSD 'umodem' driver for the usb/serial interface and accessing it under /dev/cuaU0.

As it is the driver is not working for this UPS model.

rpvelloso commented 8 years ago

Another observation: the detection loop reads at most 20 packets and attempts to detect the model at each packet. Shouldn't it read, one byte at a time, at most "packet_size" bytes until 0xFE is found, then fail detection if 0xFE not found or attempt detection on the next "packet_size" bytes if 0xFE is found? Also, I think this driver could be modified to work with delimited packets instead of fixed size packets. This way it could handle both packets (24 and 25 bytes long).

/* trying detect solis model */
while ( ( !detected ) && ( j < 20 ) )  {
    temp[0] = 0; /* flush temp buffer */
    tam = ser_get_buf_len(upsfd, temp, tpac, 3, 0);
    if( tam == 25 ) {
        for( i = 0 ; i < tam ; i++ ) {
            Pacote[i] = temp[i];
        }
    }

    j++;
    if( tam == 25)
        CommReceive((char *)Pacote, tam);
    else
         CommReceive((char *)temp, tam);
} /* while end */
rpvelloso commented 8 years ago

The checksum byte appears to be in the same relative position as in the 25 bytes packet (before the last byte 0xFE), but always yelds a value with a difference of -19 (-0x13) with the computed checksum: packet: BA6E88AE0001A49200291E0100000000000E98496060 [9F] FE checksum: BA+6E+88+AE+00+01+A4+92+00+29+1E+01+00+00+00+00+00+0E+98+49+60+60 = 9F-13

Tested with several packets with same results.

Driver's current checksum validation: /* CheckSum verify */ CheckSum = 0; i_end = 23; for( i = 0 ; i < i_end ; ++i ) CheckSum = RecPack[ i ] + CheckSum; CheckSum = CheckSum % 256;

RecPack[ 23 ] == CheckSum;

clepple commented 8 years ago

There is a similar thread on the nut-upsuser list at the moment: see Gmane or Alioth. Hopefully you saw the recently-pushed solis_debug branch - let me know if you have suggestions on how to better format the debug output.

I agree with the general sentiment of your second comment: the parsing should try harder to accumulate a proper packet. However, I don't know how the code should handle a short packet.

I did not calculate the checksums for the other model, but I wonder if control characters are being dropped? In your case, 0x13/19 corresponds to ^S, which is XOFF. Do you get different results if you run stty -f /dev/cuaU0 raw before starting the driver?

rpvelloso commented 8 years ago

That did it (stty raw) :D The checksum and packet size are now correct. Is there a way to put this stty call (ioctl() ???) inside the driver?

The solis.c code, however, is not 100%, sometimes it detects the UPS, sometimes it doesn't, that "while" loop should be corrected or something...

packet sniffed via cu: ba;6e;89;ae;00;02;a4;9e;00;16;2c;05;02;00;00;00;00;00;0f;98;49;60;60;9c;fe

clepple commented 8 years ago

That did it (stty raw) :D The checksum and packet size are now correct. Is there a way to put this stty call (ioctl() ???) inside the driver?

Well, raw mode should get set at the same time as the baud rate: https://github.com/networkupstools/nut/blob/master/drivers/serial.c#L163

After unplugging and plugging the USB cable in again, what does stty -a -f /dev/cuaU0 print?

The solis.c code, however, is not 100%, sometimes it detects the UPS, sometimes it doesn't, that "while" loop should be corrected or something...

Want to give it a try? I suspect that a lot of the read loops can be replaced with ser_get_line() with 0xFE as the end character: https://github.com/networkupstools/nut/blob/master/drivers/serial.c#L414

rpvelloso commented 8 years ago

Already trying, thanks for the info. After testing I'll submit the diff.

2015-08-25 22:55 GMT-03:00 Charles Lepple notifications@github.com:

That did it (stty raw) :D The checksum and packet size are now correct. Is there a way to put this stty call (ioctl() ???) inside the driver?

Well, raw mode should get set at the same time as the baud rate: https://github.com/networkupstools/nut/blob/master/drivers/serial.c#L163

After unplugging and plugging the USB cable in again, what does stty -a -f /dev/cuaU0 print?

The solis.c code, however, is not 100%, sometimes it detects the UPS, sometimes it doesn't, that "while" loop should be corrected or something...

Want to give it a try? I suspect that a lot of the read loops can be replaced with ser_get_line() with 0xFE as the end character: https://github.com/networkupstools/nut/blob/master/drivers/serial.c#L414

— Reply to this email directly or view it on GitHub https://github.com/networkupstools/nut/issues/231#issuecomment-134789040 .

rpvelloso commented 8 years ago

I've made minimum changes to the source, and commented out the original code. Tested in my system and it promptly identifies the UPS now. There are some errors in the messages, too. And your suggestion for ser_get_line() with 0xFE should definitly be done... but maybe in the future :)

Any chance this patch will appear soon in a release? Don't want to do it manually everyitme I update my FreeNAS box :)

Cheers!

[root@freenas /usr/local/libexec/nut]# /tmp/solis -a ups Network UPS Tools - Microsol Solis UPS driver 0.62 (2.7.3) Detected Back-UPS 1200 BR on /dev/cuaU0 UPS Date 2006/09/15 System Date 2015/08/25 day of week Tue UPS internal Time 7:30:07 Shutdown programming not atived

root@new-host:~/nut # diff -w -B -u ../solis.c drivers/solis.c --- ../solis.c 2015-08-25 23:20:48.000000000 -0300 +++ drivers/solis.c 2015-08-25 22:34:31.556235000 -0300 @@ -787,7 +787,8 @@ static void getbaseinfo(void) {

2015-08-25 23:03 GMT-03:00 Roberto Panerai Velloso rvelloso@gmail.com:

Already trying, thanks for the info. After testing I'll submit the diff.

2015-08-25 22:55 GMT-03:00 Charles Lepple notifications@github.com:

That did it (stty raw) :D The checksum and packet size are now correct. Is there a way to put this stty call (ioctl() ???) inside the driver?

Well, raw mode should get set at the same time as the baud rate: https://github.com/networkupstools/nut/blob/master/drivers/serial.c#L163

After unplugging and plugging the USB cable in again, what does stty -a -f /dev/cuaU0 print?

The solis.c code, however, is not 100%, sometimes it detects the UPS, sometimes it doesn't, that "while" loop should be corrected or something...

Want to give it a try? I suspect that a lot of the read loops can be replaced with ser_get_line() with 0xFE as the end character: https://github.com/networkupstools/nut/blob/master/drivers/serial.c#L414

— Reply to this email directly or view it on GitHub https://github.com/networkupstools/nut/issues/231#issuecomment-134789040 .

clepple commented 8 years ago

I've made minimum changes to the source, and commented out the original code. Tested in my system and it promptly identifies the UPS now. There are some errors in the messages, too. And your suggestion for ser_get_line() with 0xFE should definitly be done... but maybe in the future :)

Cool. What about the output of 'stty -a -f /dev/cuaU0'?

rpvelloso commented 8 years ago

Just tested automatic shutdown on power failure, works perfect. On system restart the driver is also loading, automatic, without problems, I guess NUT is correctly setting RAW mode on the device. It's working from FreeNAS GUI too, changed service config, started, stopped and restarted, no trouble.

Some Microsol-APC models have different serial interfaces, would be nice if someone could test this patch on those interfaces cause I've read reports that the current version worked for some people.

OS: FreeNAS-9.3-STABLE (FreeBSD 9.3)

[root@freenas ~]# stty -a -f /dev/cuaU0 speed 9600 baud; 0 rows; 0 columns; lflags: -icanon -isig -iexten -echo -echoe -echok -echoke -echonl -echoctl -echoprt -altwerase -noflsh -tostop -flusho -pendin -nokerninfo -extproc iflags: -istrip -icrnl -inlcr -igncr -ixon -ixoff -ixany -imaxbel -ignbrk -brkint -inpck ignpar -parmrk oflags: -opost -onlcr -ocrnl tab0 -onocr -onlret cflags: cread cs8 -parenb -parodd -hupcl clocal -cstopb -crtscts -dsrflow -dtrflow -mdmbuf cchars: discard = ^O; dsusp = ^Y; eof = ^D; eol = ; eol2 = ; erase = ^?; erase2 = ^H; intr = ^C; kill = ^U; lnext = ^V; min = 1; quit = ^\; reprint = ^R; start = ^Q; status = ^T; stop = ^S; susp = ^Z; time = 0; werase = ^W;

2015-08-26 8:39 GMT-03:00 Charles Lepple notifications@github.com:

I've made minimum changes to the source, and commented out the original code. Tested in my system and it promptly identifies the UPS now. There are some errors in the messages, too. And your suggestion for ser_get_line() with 0xFE should definitly be done... but maybe in the future :)

Cool. What about the output of 'stty -a -f /dev/cuaU0'?

— Reply to this email directly or view it on GitHub https://github.com/networkupstools/nut/issues/231#issuecomment-134958249 .

rpvelloso commented 8 years ago

Charles,

How can I commit the changes I've done? Never contributed before...

clepple commented 8 years ago

See http://www.networkupstools.org/docs/developer-guide.chunked/ especially sections 3.4 and 3.6.

But I really don't think we have figured out the stty issue yet, and your patch only addresses initial detection.

rpvelloso commented 8 years ago

What stty issue? I'm running the driver directly, without running stty to set raw mode, NUT is setting it correctly when the device is opened, or else, packets wouldn't be read correctly, but that's not the case, 'cause the driver is signaling the OS when line/battery status change. Only the voltage readings remain incorrect.

rpvelloso commented 8 years ago

Shouldn't we make the driver functional ASAP and then refactor the code?

clepple commented 8 years ago

@rpvelloso I retract my suggestion about ser_get_line() - it doesn't quite line up. (I still think there should be a solis-specific call like that, though, since it is possible for the driver to get out of sync later.) So I merged your patch with some other cleanup that I had locally. Pushed to "solis_debug" branch just now.

Only the voltage readings remain incorrect.

Not sure I knew about the voltage readings. What is wrong?

rpvelloso commented 8 years ago

Great,

Voltage values are calculated correctly for Microsol branded UPS(*), but they are incorrect for APC branded UPS (upsc shows incorrect values). The values are, probably, encoded differently in APC model.

About the driver desync, could this be due to calling 'ser_flush_in()'? After syncronizing (detection) the serial port shouldn't be flushed ever. If part of a packet get's flushed, then the driver will, indeed, desync. I'll try to find some time to test this (remove the flush calls) and report back here, but I can't say when I'll do it. I haven't experienced any problems of this kind, though, if you could show me how to reproduce the error...

About get_line(), maybe it's related to desync. It should work.

(*) so it is said. I don't have this model to test, but the driver was originally designed for this model.

bsalvador commented 8 years ago

Hello guys, I have the APC Branded Microsol that uses the FTDI USB-Serial cable. At the time I did some changes on the solis.c in order just to know if the UPS was online or not. The values are displayed totally wrong today. Now I see the need to get those values correctly, and the idea is to build a new driver based on solis.c.

Rpvelloso, did you figured out the package bytes struct? I believe that my and yours UPS are very similar. I have reverse engineered the java application but never figured it out.

rpvelloso commented 8 years ago

Bruno,

It's the same driver for your model and mine. Only difference is the serial interface. The packet appears to have the same structure as in solis.c, although the voltages doesn't show up correct.

bsalvador commented 8 years ago

I have done some reverse engineering in the SGM software and discovered few things byte[0] is the UPS model, 0xBA is internally called: STAY1200_USB.

I have done a quick code, it is very messy, in order to parse the received bytes.

The code is not ready yet, the values are not matching, and not finalized.

Also I have included the source for the supplier software management, it is in Java and if you are familiar with it, you will find easily the solution.

Repository

https://github.com/bsalvador/STAY1200_USB

clepple commented 8 years ago

@rpvelloso and @bsalvador, any objections to merging the solis_debug branch? I realize some of the voltages are wrong, but if I recall, the branch works better than the master code.

bsalvador commented 8 years ago

my opinion, go forward with that.

rpvelloso commented 8 years ago

Charles,

It is, indeed, better than the master branch, but it is not 100%. From time to time the driver desynchronizes with the UPS and has to be restarted, but the current version is unusable (at least in my case), so IMO it should be merged and an issue kept open until this desynchronization problem is solved.

I'm out of time right now, so I can't deal with this issue right away.

Best, Roberto.

clepple commented 8 years ago

Merged solis_debug branch to master in 25973d4. We'll leave this issue open.

rpvelloso commented 6 years ago

I have made some corrections on the 'solis' driver. I have found two memory leaks and fixed them. Also removed 'flushs' that (I think) were causing packet reading desyncronization. Besides that I have made some improvements and optimizations on the source code in general. How can I submit my changes? I've made changes to the files solis.c and solis.h.

I think the memory leaks were also causing system instability and reboots. I'm testing the changes on my system right now, no problems so far.

If someone else could test it too, that would be great ( @bsalvador ???) .

Managed to reduce the source size from 1230 lines to 975.

solis.zip

rpvelloso commented 6 years ago

@clepple so far so good, no more problems related with solis driver on my FreeNAS box. Memory use is now stable (no allocations at all).

jimklimov commented 6 years ago

As for how to submit - the best way nowadays is to fork NUT on github, then check out a copy of your fork to your computer, apply the changes -- e.g. copy files from your tarball, or better yet - generate a patch-file of changes from the base version you used in 2015 and your final development, and apply this patch (maybe adapt the changes/solve conflicts in case original files had some evolution in these two years). Finally, git add the changed files in your workspace, git commit to save and describe the changeset, and git push it back to your github fork. Then you'll have an option of making a pull request on github web-gui, to propose a submission to the common project.

rpvelloso commented 6 years ago

Will do that

rpvelloso commented 6 years ago

@jimklimov can you check if it is allright? This is the first time I open a pull request here.

https://github.com/networkupstools/nut/pull/511

rpvelloso commented 6 years ago

Hello... ? Anybody? @clepple @jimklimov ? Is there something I should do before my changes get merged?

jsalatiel commented 5 years ago

@rpvelloso i am also testing your code. So far so good. Hope this get merged some time

rpvelloso commented 5 years ago

@jsalatiel if you encounter any problems/bugs, please contact me, I'll try to fix it before merging these changes.