make query detect stop byte

[chadek]

This PR should allow to autodetect the stop byte and get reed of all the buffersize stuff. This a for instance a draft (so DO NOT MERGE for now) that I still need to test on my hardware. If it does work, I will also delete all the buffer size related stuff which will make everything simpler

[A13xund3r]

Does it compile for you? line numbers will be incorrect, I manually edited my files commenting out deleted lines.

bananapi:skymax-demo:# make                                                                                    <master ✗>
[ 20%] Building CXX object CMakeFiles/inverter_poller.dir/inverter.cpp.o
/root/github/skymax-demo/inverter.cpp: In member function ‘bool cInverter::query(const char*, int)’:
/root/github/skymax-demo/inverter.cpp:116:11: error: ‘READ_BUFFER_SIZE’ does not name a type
  116 |     const READ_BUFFER_SIZE = 60;
      |           ^~~~~~~~~~~~~~~~
/root/github/skymax-demo/inverter.cpp:120:32: warning: pointer of type ‘void *’ used in arithmetic [-Wpointer-arith]
  120 |         n = read(fd, (void*)buf+i, READ_BUFFER_SIZE);
      |                      ~~~~~~~~~~^~
/root/github/skymax-demo/inverter.cpp:120:36: error: ‘READ_BUFFER_SIZE’ was not declared in this scope
  120 |         n = read(fd, (void*)buf+i, READ_BUFFER_SIZE);
      |                                    ^~~~~~~~~~~~~~~~
/root/github/skymax-demo/inverter.cpp:158:60: error: ‘j’ was not declared in this scope
  158 |         lprintf("INVERTER: %s reply size (%d bytes)", cmd, j);
      |                                                            ^
make[2]: *** [CMakeFiles/inverter_poller.dir/build.make:90: CMakeFiles/inverter_poller.dir/inverter.cpp.o] Error 1
make[1]: *** [CMakeFiles/Makefile2:83: CMakeFiles/inverter_poller.dir/all] Error 2
make: *** [Makefile:91: all] Error 2

[chadek]

No it didn't, now it does

[A13xund3r]

Now my life is much easier.. but there is strange behaviour in one case. TLDR: If transmission is incomplete (some data at end is not received including stop and CRC bytes I'm getting

ananapi:skymax-demo:# ./inverter_poller -d -r QBMS                                                                                          <master ✗>
Thu Nov  2 11:47:10 2023 INVERTER: Debug set
Thu Nov  2 11:47:10 2023 INVERTER: Current CRC: 61 44
Thu Nov  2 11:47:10 2023 INVERTER: QBMS reply size (32 bytes)
Thu Nov  2 11:47:10 2023 INVERTER: QBMS: CRC Failed!  Reply size: 32  Buffer: (1 000 0 0 0 000 000 000 0000 00r the actual inverter polling process...

So it still consider that transmission is completed. Here I need explain my circumstance. For unknown (yet) reason sometimes I'm getting NAK response to valid command, then it will respond this way to every command regardless it is valid or not. Until I run serial port sniffer on another console, then it will do NAK once again and return to normal replies. Problem is that sniffer I found and use is quite old and taking over serial port access in random way, depend which program is first when data arrive something is missing in inverter poller or sniffer gets Error reading from port: Resource temporarily unavailable. So for case above sniffer got remaining part of message Device --> 0 (048) 0 (048) � (128) � (134) <CR> (013) there are 2 bytes of message body, CRC and CR, when all data is complete it is like that:

bananapi:skymax-demo:# ./inverter_poller -d -r QBMS                                                               
Thu Nov  2 11:47:25 2023 INVERTER: Debug set
Thu Nov  2 11:47:25 2023 INVERTER: Current CRC: 61 44
Thu Nov  2 11:47:25 2023 INVERTER: QBMS reply size (37 bytes)
Thu Nov  2 11:47:25 2023 INVERTER: QBMS: 37 bytes read: (1 000 0 0 0 000 000 000 0000 0000
Thu Nov  2 11:47:25 2023 INVERTER: QBMS query finished
Reply:  1 000 0 0 0 000 000 000 0000 0000

[chadek]

I'm glad to ear that it seams working. I'll get my hands on my hardware sunday or monday to test this out.

Again, this is still a draft, I still need to make some adjustments as I choose to "stream" the file descriptor loading a certain quantity of byte (60) which should work for most of the small commands on the first batch. I see some things that are probably not good but without anything to test for now I can do much.

[A13xund3r]

Does const int READ_BUFFER_SIZE = 60; limit RX buffer to 60 bytes? If yes then it is too short, my longest reply from inverter is 143 bytes.

[chadek]

I tried on my hardware, raw command seams to work properly but now the use of the tool normally without options seams to randomly work and printing crazy values on some variables probably due to a wrong buffer size badly detected for some reason. I'll dig more when I'll have time, probably this coming week

[chadek]

@A13xund3r, I fixed the query function to detect stop byte, it should detect automatically the reply length. Every raw cmd should work and you should be able to read data without any replysize config which I removed from code as it should be now useless. Tell me if it works on your hardware

[A13xund3r]

I don't know how to update the code with your changes other way than just copy paste changes and deleting stuff manually, or maybe @chadek your fork does include all changes? Can someone point me in right direction?

[chadek]

You can clone my repo and checkout on query_rework branch (git checkout query_rework)

[A13xund3r]

@A13xund3r, I fixed the query function to detect stop byte, it should detect automatically the reply length. Every raw cmd should work and you should be able to read data without any replysize config which I removed from code as it should be now useless. Tell me if it works on your hardware

So using your fork and query_rework branch it seems that CR detection is not working properly

bananapi:skymax-demo:# ./inverter_poller -d -r QT
Thu Nov 16 12:04:40 2023 INVERTER: Debug set
Thu Nov 16 12:04:40 2023 INVERTER: Current CRC: 27 FF
Thu Nov 16 12:04:43 2023 INVERTER: QT read timeout
Thu Nov 16 12:04:43 2023 INVERTER: QT command timeout, or couldn't find stop byte. Byte read (18 bytes)
Reply:  

when sniffer proves it is there Device --> ( (040) 2 (050) 0 (048) 2 (050) 3 (051) 1 (049) 1 (049) 2 (050) 4 (052) 0 (048) 4 (052) 3 (051) 7 (055) 4 (052) 5 (053) � (138) � (221) <CR> (013) Also reply is empty when I expect it to contain received data despite it is assumed incomplete.

[chadek]

2 possibility, your command really timeout and you probably have some weird communication issue I also have sometimes maybe because of the cable not plugged well (unplugged replug) or your inverter was still trying to send data when command timeout was reached. In that case, try to put a higher value than 2 (seconds) on inverter.cpp line 119 to see what happen. Does other commands work?

[A13xund3r]

I use RS232 communication so un/re plug trick for USB won't work, changed timeout to 8s, got command timeout, or couldn't find stop byte. Byte read (7 bytes) this is reply to inverter real time query (QT) so it normally work, 7 bytes reply mean it answered NAK. So in another console launched serial port sniffer (mentioned it earlier https://github.com/manio/skymax-demo/pull/26#issuecomment-1790614764 along with some communication issue.). I ran QT query again, first reply was:

bananapi:skymax-demo:# ./inverter_poller -d -r QT
Thu Nov 16 14:29:57 2023 INVERTER: Debug set
Thu Nov 16 14:29:57 2023 INVERTER: Current CRC: 27 FF
INVERTER: stop byte detected, buffersize might be 7 for QT Thu Nov 16 14:29:57 2023 INVERTER: QT reply size (7 bytes)
Thu Nov 16 14:29:57 2023 INVERTER: QT: 7 bytes read: (NAK 
Thu Nov 16 14:29:57 2023 INVERTER: QT query finished
Reply:  NAK

So it was exactly as expected (always after starting sniffer I have one more NAK), then got proper response but again CR was not found QT command timeout, or couldn't find stop byte. Byte read (18 bytes) 18 bytes is for proper reply. I think you are right about comm issue but I suspect serial line configuration is incorrect. To me it looks like there is wrong flow control used, it might work fine with USB but real RS232 is doing something wrong, is there option not to use flow control for port init? when I run sniffer it keep port open all the time so it work very reliable at this time. Confusing is that NAK response is 7 bytes long, as it is received, but CR is not found, could you change code so buffer is displayed even in timeout condition?

[chadek]

Line 128, the for loop condition might be wrong, I think it should be j<=i+n, that is probably why it miss the stop byte. Can you try this out ? In a second time, you may also try to lower the two usleep value to the original (10) instead of 2000: my device approximately send data once every 5000 micro seconds, so 2000 is fine but maybe yours is faster and this sleep may have an incidence on this. A last thing you can try, is to play with READ_BUFFER_SIZE value defined line 113. My device seams to send 8 bytes at a time. You could try this value or higher modulo 8 values or something else to see what happens.

I also added a print off the buffer when you get a read timeout.

[A13xund3r]

with j<=i+n, both usleeps=10, READ_BUFFER_SIZE is 15 so I don't touch it for now

bananapi:skymax-demo:# ./inverter_poller -d -r QT
Fri Nov 17 13:46:50 2023 INVERTER: Debug set
Fri Nov 17 13:46:51 2023 INVERTER: Current CRC: 27 FF
Fri Nov 17 13:46:58 2023 INVERTER: QT read timeout
Fri Nov 17 13:46:58 2023 INVERTER: QT command timeout, or couldn't find stop byte. Byte read (7 bytes). Buffer: (NAKss
****deleted content of config file****
Reply:  

Please note it got 7 bytes, start byte(1), NAK(3),CRC(2), stop byte(1). so what it got as last byte? It seems to be not printable character. Started sniffer, then:

bananapi:skymax-demo:# ./inverter_poller -d -r QT
Fri Nov 17 13:49:50 2023 INVERTER: Debug set
Fri Nov 17 13:49:50 2023 INVERTER: Current CRC: 27 FF
INVERTER: stop byte detected, buffersize might be 7 for QT Fri Nov 17 13:49:50 2023 INVERTER: QT reply size (7 bytes)
Fri Nov 17 13:49:50 2023 INVERTER: QT: 7 bytes read: (NAK 
Fri Nov 17 13:49:50 2023 INVERTER: QT query finished
Reply:  NAK
bananapi:skymax-demo:# ./inverter_poller -d -r QT
Fri Nov 17 13:50:27 2023 INVERTER: Debug set
Fri Nov 17 13:50:27 2023 INVERTER: Current CRC: 27 FF
Fri Nov 17 13:50:34 2023 INVERTER: QT read timeout
ic configuration options for the actual inverter polling process...find stop byte. Byte read (0 bytes). Buffer: QT'�
Reply:  

second reply was blocked by sniffer which got: Device --> ( (040) 2 (050) 0 (048) 1 (049) 8 (056) 0 (048) 2 (050) 2 (050) 4 (052) 0 (048) 6 (054) 2 (050) 9 (057) 1 (049) 5 (053) � (193) � (244) <CR> (013) then 3-rd attempt:

bananapi:skymax-demo:# ./inverter_poller -d -r QT
Fri Nov 17 13:50:37 2023 INVERTER: Debug set
Fri Nov 17 13:50:37 2023 INVERTER: Current CRC: 27 FF
INVERTER: stop byte detected, buffersize might be 18 for QT Fri Nov 17 13:50:37 2023 INVERTER: QT reply size (18 bytes)
Fri Nov 17 13:50:37 2023 INVERTER: QT: 18 bytes read: (20180224062925 
Fri Nov 17 13:50:37 2023 INVERTER: QT query finished
Reply:  20180224062925

Killed sniffer then:

bananapi:skymax-demo:# ./inverter_poller -d -r QT
Fri Nov 17 14:19:13 2023 INVERTER: Debug set
Fri Nov 17 14:19:13 2023 INVERTER: Current CRC: 27 FF
Fri Nov 17 14:19:20 2023 INVERTER: QT read timeout
Fri Nov 17 14:19:20 2023 INVERTER: QT command timeout, or couldn't find stop byte. Byte read (18 bytes). Buffer: (20180224065802�?
bananapi:skymax-demo:# ./inverter_poller -d -r QT  
Fri Nov 17 14:25:14 2023 INVERTER: Debug set
Fri Nov 17 14:25:14 2023 INVERTER: Current CRC: 27 FF
Fri Nov 17 14:25:21 2023 INVERTER: QT read timeout
Fri Nov 17 14:25:21 2023 INVERTER: QT command timeout, or couldn't find stop byte. Byte read (7 bytes). Buffer: (NAKss

I will try to investigate flow control configuration then will make hardware serial port sniffer to see what is going on wires. Fact that communication is pretty reliable with sniffer active suggests there is port config issue. Alternatively I'll wait until another banana pi I ordered arrive and connect it to USB port, this banana is using almost 30m cable so it is too far for USB.

[chadek]

This looks like indeed a weird communication issue if it works sometimes and sometimes not

[A13xund3r]

Got it... following https://blog.mbedded.ninja/programming/operating-systems/linux/linux-serial-ports-using-c-cpp/ I've added: settings.c_iflag &= ~(IGNBRK|BRKINT|PARMRK|ISTRIP|INLCR|IGNCR|ICRNL); // Disable any special handling of received bytes now CR is properly recognized every time, also I added:

    settings.c_lflag &= ~ICANON;
    settings.c_iflag &= ~(IXON | IXOFF | IXANY); // Turn off s/w flow ctrl

line 86 cfsetospeed(&settings, baud); // baud rate changed to cfsetspeed(&settings, baud); // baud rate first one set baud for output stream only when latter for both directions. With this everything is working fine so far. Update because apparently there is still issue with RS232 TX line config. Basically I'm checking date/time and trying to set it.

bananapi:skymax-demo:# ./inverter_poller -r QT
Reply:  20180227032655
bananapi:skymax-demo:# ./inverter_poller -r DAT231120105100
INVERTER: stop byte detected, buffersize might be 7 for DAT231120105100 Reply:  NAK
bananapi:skymax-demo:# ./inverter_poller -r DAT20231120105100
INVERTER: stop byte detected, buffersize might be 7 for DAT20231120105100 Reply:  NAK
bananapi:skymax-demo:# ./inverter_poller -r QT
Reply:  NAK
bananapi:skymax-demo:# ./inverter_poller -r QT
Reply:  NAK
bananapi:skymax-demo:# ./inverter_poller -r QT
Reply:  NAK
bananapi:skymax-demo:# ./inverter_poller -r DAT20231120105100
INVERTER: stop byte detected, buffersize might be 7 for DAT20231120105100 Reply:  NAK
bananapi:skymax-demo:# ./inverter_poller -r QT
INVERTER: stop byte detected, buffersize might be 7 for QT Reply:  NAK

Here I started sniffer so it keep port open and also does configure it.

bananapi:skymax-demo:# ./inverter_poller -r QT
Reply:  
bananapi:skymax-demo:# ./inverter_poller -r QT
Reply:  
bananapi:skymax-demo:# ./inverter_poller -r DAT20231120105100
INVERTER: stop byte detected, buffersize might be 7 for DAT20231120105100 Reply:  NAK
bananapi:skymax-demo:# ./inverter_poller -r DAT231120105100
INVERTER: stop byte detected, buffersize might be 7 for DAT231120105100 Reply:  ACK

[chadek]

Thanks for the digging, now it work everytime with this new parameters! Could you check if this last version work on your side? Then I'll set this PR reading to merge :)

[A13xund3r]

In my case outgoing stream seems to be corrupted, I have to run sniffer to get transfers without issues or after second command I'm getting NAKs, LF is added? I suspect settings.c_oflag &= ~OPOST; // Prevent special interpretation of output bytes (e.g. newline chars) is not enough. Will dig deeper.

bananapi:skymax-demo:# ./inverter_poller -r QT 
INVERTER: stop byte detected, buffersize might be 18 for QT Reply:  20231121135659
bananapi:skymax-demo:# ./inverter_poller -r QT 
INVERTER: stop byte detected, buffersize might be 7 for QT Reply:  NAK
bananapi:skymax-demo:# ./inverter_poller -r QT 
INVERTER: stop byte detected, buffersize might be 7 for QT Reply:  NAK

Update Probably done, basically I've compared output of stty -F /dev/ttyS1 -a after running inverter_pooler and sniffer and only difference was -iexten -echo after sniffer, so aded:

settings.c_lflag &= ~ECHO;             // turn off Echo input characters.
settings.c_lflag &= ~IEXTEN;           // disable implementation-defined input processing.

No more NAK after first good reply so far.

[A13xund3r]

Making separate message because previous post edit apparently is not notified.

[chadek]

Sorry for the delay, I added these new parameters. I haven't my hardware on hands to test for now but if it is working for you, we finally could mark this pr ready to merge

[synogen]

I also tested this and got inconsistent results as well, but it might be due to my environment. I am running an OrangePi Zero W with a USB to serial converter connected to my MPPSolar PIP5048GEW.

Often times there would be different reply lengths for the same command multiple times in a row and sometimes there would be "garbled" characters in the replies, indicating some sort of noise in the data. When I sent the same commands through my own little program it worked fine though.

After noticing some issues with logging and screen refresh in the terminal I then restarted the machine and suddenly the inverter_poller worked fine, I got consistent replies and no more "garbled" characters. Maybe it was just a power supply issue, I have previously experienced Pi-like boards to act up when the power supply was sub-optimal, but since my own program seemed to work OK I can't be sure.

Either way you helped me to finally set the right buffer size values in the inverter.conf, so I just wanted to say thanks and contribute my experience. 😃 I am interested in this CR getting merged as well, so hopefully there'll be no more fiddling needed with any buffer sizes anymore in the future. I also hope this update will later make its way to https://github.com/ned-kelly/docker-voltronic-homeassistant so that'll become easier to use.

[chadek]

Did not tried my last code yet, maybe today and then mark this PR as ready to merge. In my case, I choose to use this project instead : https://github.com/jblance/mpp-solar because it support a large amount of devices and seams to be maintained. The only thing I do not really like is that is it is quite complex, and by consequence bit heavier on hardware :'c

[chadek]

Just tested out again to ensure it is working all good. @manio ready to merge!

[manio]

Thanks!

[synogen]

Did not tried my last code yet, maybe today and then mark this PR as ready to merge. In my case, I choose to use this project instead : https://github.com/jblance/mpp-solar because it support a large amount of devices and seams to be maintained. The only thing I do not really like is that is it is quite complex, and by consequence bit heavier on hardware :'c

I just checked their wiki in more detail and I hadn't realized until now they had MQTT support and can even format the messages for Home Assistant which is what I was after. I was aware of the project before but only used it for their nice collection of protocol documents, I only skimmed their readme before this and since I found no mention of MQTT or Home Assistant there I didn't delve any deeper.

Thanks for pointing me to it again, I would've completely missed all the things that one can do otherwise! 👍