Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

commands longer than 8 chars never work #7

Open
MoppelMat opened this issue Oct 16, 2019 · 21 comments
Open

commands longer than 8 chars never work #7

MoppelMat opened this issue Oct 16, 2019 · 21 comments

Comments

@MoppelMat
Copy link

MoppelMat commented Oct 16, 2019

Hello ned,

its me, André. We sent a few mails about the variable return sizes.
I tried to fix it, but was unsuccessful, so I let mine hardcoded as they work for me.

Sadly I hit a stone wall that I can not get through with another thing.

I am unable to run any command that is longer than 8 chars. No matter how ofter I try, it just does not work.
I talk a little here http://forums.aeva.asn.au/viewtopic.php?f=64&t=4332&p=75721#p75721 about it.
I added a little bit of debug but, can not fix it by myself.
try to run this command to send a 20A AC charging parameter. It never works...

docker exec -it voltronic-mqtt /opt/inverter-cli/bin/inverter_poller -d -1 -r MUCHGC020

try anything that is 8 chars and you will see timeouts or incorrect start stop bytes.
use the normal short commands like POP00 and you will see no timeouts.

see here, the same 8 char command, multiple times in a row, the last one got ACKed.

root@orgor:# docker exec -it voltronic-mqtt /opt/inverter-cli/bin/inverter_poller -d -1 -r PExxxxxxx
Wed Oct 16 05:02:47 2019 INVERTER: Debug set
Wed Oct 16 05:02:47 2019 INVERTER: Current CRC: 3A 80
Wed Oct 16 05:02:47 2019 INVERTER: cmd: PExxxxxxx n: 9
Wed Oct 16 05:02:47 2019 INVERTER: buf2: PExxxxxxx
Wed Oct 16 05:02:52 2019 INVERTER: PExxxxxxx read timeout
Wed Oct 16 05:02:52 2019 INVERTER: PExxxxxxx reply too short (0 bytes)
Reply:
root@orgor:# docker exec -it voltronic-mqtt /opt/inverter-cli/bin/inverter_poller -d -1 -r PExxxxxxx
Wed Oct 16 05:02:53 2019 INVERTER: Debug set
Wed Oct 16 05:02:53 2019 INVERTER: Current CRC: 3A 80
Wed Oct 16 05:02:53 2019 INVERTER: cmd: PExxxxxxx n: 9
Wed Oct 16 05:02:53 2019 INVERTER: buf2: PExxxxxxx
Wed Oct 16 05:02:58 2019 INVERTER: PExxxxxxx read timeout
Wed Oct 16 05:02:58 2019 INVERTER: PExxxxxxx reply too short (0 bytes)
Reply:
root@orgor:# docker exec -it voltronic-mqtt /opt/inverter-cli/bin/inverter_poller -d -1 -r PExxxxxxx
Wed Oct 16 05:02:59 2019 INVERTER: Debug set
Wed Oct 16 05:02:59 2019 INVERTER: Current CRC: 3A 80
Wed Oct 16 05:02:59 2019 INVERTER: cmd: PExxxxxxx n: 9
Wed Oct 16 05:02:59 2019 INVERTER: buf2: PExxxxxxx
Wed Oct 16 05:03:04 2019 INVERTER: PExxxxxxx read timeout
Wed Oct 16 05:03:04 2019 INVERTER: PExxxxxxx reply too short (0 bytes)
Reply:
root@orgor:# docker exec -it voltronic-mqtt /opt/inverter-cli/bin/inverter_poller -d -1 -r PExxxxxxx
Wed Oct 16 05:03:07 2019 INVERTER: Debug set
Wed Oct 16 05:03:07 2019 INVERTER: Current CRC: 3A 80
Wed Oct 16 05:03:07 2019 INVERTER: cmd: PExxxxxxx n: 9
Wed Oct 16 05:03:07 2019 INVERTER: buf2: PExxxxxxx
Wed Oct 16 05:03:11 2019 INVERTER: PExxxxxxx reply size (7 bytes)
Wed Oct 16 05:03:11 2019 INVERTER: PExxxxxxx: 7 bytes read: (ACK
Wed Oct 16 05:03:11 2019 INVERTER: PExxxxxxx query finished
Reply: ACK

sometimes I get answers from normal query commands as result that "should" not be there.
Maybe this helps debugging?

root@orgor:/opt/ha-inverter-mqtt-agent# docker exec -it voltronic-mqtt /opt/inverter-cli/bin/inverter_poller -d -1 -r MUCHGC020
Wed Oct 16 05:35:28 2019 INVERTER: Debug set
Wed Oct 16 05:35:28 2019 INVERTER: Current CRC: F3 F1
Wed Oct 16 05:35:28 2019 INVERTER: cmd: MUCHGC020 n: 9
Wed Oct 16 05:35:28 2019 INVERTER: buf2: MUCHGC020
Wed Oct 16 05:35:28 2019 INVERTER: MUCHGC020 reply size (7 bytes)
Wed Oct 16 05:35:28 2019 INVERTER: MUCHGC020: incorrect start/stop bytes. Buffer: (230.0 20??
Reply:
root@orgor:/opt/ha-inverter-mqtt-agent# docker exec -it voltronic-mqtt /opt/inverter-cli/bin/inverter_poller -d -1 -r MUCHGC020
Wed Oct 16 05:35:30 2019 INVERTER: Debug set
Wed Oct 16 05:35:30 2019 INVERTER: Current CRC: F3 F1
Wed Oct 16 05:35:30 2019 INVERTER: cmd: MUCHGC020 n: 9
Wed Oct 16 05:35:30 2019 INVERTER: buf2: MUCHGC020
Wed Oct 16 05:35:33 2019 INVERTER: MUCHGC020 read timeout
Wed Oct 16 05:35:33 2019 INVERTER: MUCHGC020 reply too short (0 bytes)
Reply:
root@orgor:/opt/ha-inverter-mqtt-agent# docker exec -it voltronic-mqtt /opt/inverter-cli/bin/inverter_poller -d -1 -r MUCHGC020
Wed Oct 16 05:35:35 2019 INVERTER: Debug set
Wed Oct 16 05:35:35 2019 INVERTER: Current CRC: F3 F1
Wed Oct 16 05:35:35 2019 INVERTER: cmd: MUCHGC020 n: 9
Wed Oct 16 05:35:35 2019 INVERTER: buf2: MUCHGC020
Wed Oct 16 05:35:37 2019 INVERTER: MUCHGC020 reply size (7 bytes)
Wed Oct 16 05:35:37 2019 INVERTER: MUCHGC020: 7 bytes read: (NAK
Wed Oct 16 05:35:37 2019 INVERTER: MUCHGC020 query finished
Reply: NAK

@nrm21
Copy link

nrm21 commented Oct 17, 2019

I'm having this issue on my modified version as well (forked from manio). It seems like the unistd::write() command fails with a timeout when more than 8 bytes (5 characters + < CRC >< CRC >< CR >) are sent to it.

I don't know why this is happening... perhaps the protocol manual doesn't document something?

@ned-kelly
Copy link
Owner

Hi guys,

I have a feeling that updating your firmware (if you're game to try) might help here - I'm not getting the issue on my new 2kw inverter (Running latest firmware) - but my older one (same inverter, just 2 years older) also has the same issue.

Not really game to try and update the firmware to test, as my older one is not a real MPP/Voltronic so I'm worried it might brick it (known issue with the non-genuine ones).

Any of you guys game to try and update and see if it fixes it?

@MoppelMat
Copy link
Author

Hi together.

My 5kW has no new firmwares. It is already the latest. Also there is no custom fw for it.

I know a few new things now.
This 8 bytes issue only exists on usb . You have to split it into 8 bytes commands each!
Since i suck at cpp, I tried python with an already working script.
I added my fronius inverter readout and the mqtt publishing of your hassio integration to port all of this into one file of python.
I was successful last night with the subscriber backchannel from hassio to the inverter.

I hope this clears something up for someone here.

@nrm21
Copy link

nrm21 commented Oct 21, 2019

Yeah I also have an off brand and am not willing to risk updating the firmware, sorry. Maybe if I had a backup inverter, but this is my only one and I need it working.

I had already tried modifying it to send only 8 byte max per write cycle but it doesn't seem to work. Wireshark (w/ USBshark installed) shows Watchpower (running on a windows laptop) that it clearly is sending 8 bytes then the rest in a separate write. But when I try that in C++ code it seems to not work. Worse, I'm flying blind on my RasPiB+ because I don't have Xwindows installed and having trouble getting usbmon working to sniff the packets on that and see what exactly the C program is doing. Honestly it's not really a huge deal for me as I'm able to do just about everything I really need anyway. I mostly use the prog to poll the inverter for data that gets sent off the influx/grafana running on a faster box elsewhere.

Still, once I work on a problem... I like to beat it! haha

@MoppelMat
Copy link
Author

MoppelMat commented Oct 21, 2019

I had already tried modifying it to send only 8 byte max per write cycle but it doesn't seem to work.

Yeah, I had the same result when trying on my own in C.

Maybe this python code helps?
There is a little wait 0.3s between commands. maybe this is it?

OH! and I see that each command has its own crc calculated!?

try:
    response = ""
    xmodem_crc_func = crcmod.predefined.mkCrcFun('xmodem')
    command_crc = command + unhexlify(hex(xmodem_crc_func(command)).replace('0x','',1)) + '\r'

    # Set the signal handler and a 5-second alarm 
    signal.signal(signal.SIGALRM, handler)
    signal.alarm(10)
    if len (command_crc) < 9:
        time.sleep (0.35)
        os.write(device, command_crc)
        
    else:
        cmd1 = command_crc[:8]
        cmd2 = command_crc[8:]
        time.sleep (0.35)
        os.write(device, cmd1)
        time.sleep (0.35)
        os.write(device, cmd2)
        time.sleep (0.25)
    while True:
        time.sleep (0.15)
        r = os.read(device, 256)
        response += r
        if '\r' in r: break


except Exception as e:
    print("error reading inverter...: " + str(e) + "Response :" + response)
    #data = ""
    if connection == "serial":
        time.sleep(20)  # Problem with some USB-Serial adapters, they are some times disconnecting, 20 second helps to reconnect at same ttySxx
        ser.open()
        time.sleep(0.5)
        return ''

@nrm21
Copy link

nrm21 commented Oct 21, 2019

Heh, I'm guessing you may be Nietschy from this thread: http://forums.aeva.asn.au/viewtopic.php?f=64&t=4332&p=75721#p75721

Yeah I saw Josse post that a few days ago myself. I'm sure the python code would likely let me send commands longer than 8 bytes but it doesn't help in explaining why the C isn't working. It just shows that the bytes are being split up by 8 bytes each and sent. When I do that in C it doesn't work. I suppose Python is doing something to the packet automatically that makes it work and C is not. That OR, I'm inadvertently doing something negative to the chunks before sending (but I don't see what and without packet sniff capability on the Pi it's hard to Tshoot).

I'm certainly not hurting for options to use, I just would rather figure this out (from a pride perspective I guess) rather than modify someone's python script. After all I've already modified someone else's C++ code once. :)

@ned-kelly
Copy link
Owner

Hi Guys,

Sorry for the slow reply - Unfortunately my inverter's blown up currently as it was struck by lightning so I'm unable to test and troubleshoot...

Will leave the issue open though, should I get my warranty replacement back from China - and if you manage to fix it in the meantime @nrm21 please submit a PR!

@shansted
Copy link

shansted commented Oct 5, 2020

So I came across this issue myself, and got it going, atleast on my inverter (A Cheap OptiSolar 3KVA). This would only work up to a length of 16, but is there a command that long?

if (n>9)
{
write(fd , &buf , 8);
//sleep(.1); seems to work fine without the delay
write(fd , &buf[8],n-8);
}

else

{
//Send a command
write(fd, &buf, n);
}

@nrm21
Copy link

nrm21 commented Oct 31, 2020

Ok yes you are right, I was able to fix what was wrong with my loop based on that code, thanks. So now any command should work.

I pushed a commit to the repo in nrm21/skymax if anyone still cares ...it should be the only update in 2020 so far.

@teetot-aq
Copy link

teetot-aq commented Jun 30, 2021

can I get some help to send longer commands from home assistant?

@polyar6688
Copy link

Help. I want to send messages via mqtt and modify the inverter values, such as the float voltage for example. But reading, I run into this 8byte problem. How can I solve it, from homeassistant, and using the sending of information by mqtt? Thanks

@nrm21
Copy link

nrm21 commented Jul 1, 2021

I'm not sure who this "help" being asked of... but I cannot help you with docker or homeassistant since I don't use that or dockerize my version of the c++ program that sends these commands, sorry.

A little history (from what I know):
I took the code that @manio created and make my own verison with some updated.
@ned-kelly also forked from @manio 's code to make a home assistant version.
I later went back and fixed my version to allow more than 8 character commands to be sent, but the code of mine and @ned-kelly 's version are different enough I don't know how to PR to him.

You will need help from either @ned-kelly or @shansted perhaps. Or you can use the version of this program that I have coded located at https://github.com/nrm21/skymax . However it comes with no attachment to homeassistant. But, if compiled to your computers architecture (version of RaspPi that you are using likely), it still might make a good drop in replacement for the homeassistant version, since I don't believe my version of the program is much different from the forked homeassistant one.

I hope that info helps.

@nrm21
Copy link

nrm21 commented Jul 4, 2021

@ned-kelly

For what it's worth I created a PR to fix this:
ned-kelly/skymax-demo#1

@teetot-aq
Copy link

will updating the inverter.cpp file within the docker image fix the issue? I tried that but it didn't seem to work or are there additional files that need to be updated?

@nrm21
Copy link

nrm21 commented Jul 4, 2021

I don't know but I will say it's quite unlikely. There seem to be too many differences in the different repos over time. I copied from the original project back in 2018 and made quite a few changes.

But I can say that I don't recall the implementation being too different from the user perspective, most of it was just refactoring internal code. What is sent to (and received from) CLI isn't that far off, so it might be possible to just recompile from my source and use that binary as drop in replacement for the binary that is in the docker image.

But also you are speaking to someone who has no real knowledge of what's in the docker image and how it interacts with the rest of the program since I don't use homeassistant. So I could just be leading you further astray unwittingly. I just have my own thing where I run the executable on a RaspPi with telegraf and the data that is spat out is sent off to an influxdb to be looked at in grafana at a later time of my choosing.

@polyar6688
Copy link

@ned-kelly

For what it's worth I created a PR to fix this:
ned-kelly/skymax-demo#1

Hello. Download the new inverter.cpp, and replace it in the inverter-cli / folder.
Restart docker ... and try to send now, via mqtt, a message for the float voltage, example PBFT53 ... and the system did not acknowledge receipt of the information. I tell you that I connect via USB, not serial. Should I update more files?

@nrm21
Copy link

nrm21 commented Jul 4, 2021

Ok so, changing out a source file is not going to do anything because it still needs to be compiled into an executable for things to work. Also according to the protocol manual 'PBFT53' is not even a valid command as you would need something like 'PBFT53.0' instead. So I'm not surprised you didn't receive an acknowledgement.

Try changing out the executable file from the in the docker version with the one found here and see if it helps:
https://github.com/nrm21/skymax/tree/master/bin

@polyar6688
Copy link

Ok so, changing out a source file is not going to do anything because it still needs to be compiled into an executable for things to work. Also according to the protocol manual 'PBFT53' is not even a valid command as you would need something like 'PBFT53.0' instead. So I'm not surprised you didn't receive an acknowledgement.

Try changing out the executable file from the in the docker version with the one found here and see if it helps:
https://github.com/nrm21/skymax/tree/master/bin

I correct myself. Try anyway. PBFT53, PBFT530, PBFT5300, PBFT53.0, etc ... and it didn't work. Thanks in advance for your input.
Trying to follow your advice ... my knowledge is somewhat limited .. How do I change the executable? I only see an access to the link you put, but I don't know what to do with that .. Thank you very much in advance

@teetot-aq
Copy link

teetot-aq commented Jul 4, 2021

I was able to get it figure out. Thank you.

I copied the section from the skymax 'inverter.cpp' into the original docker 'inverter.cpp' and compiled with cmake in terminal. I then copied over the 'inverter_poller' file into the running docker container in '/opt/inverter-cli/bin/inverter_poller'. Restarted the container and now the commands work.

The compiler threw an error for the variable 'replysize' being defined twice so I renamed 'replysize' to reply.

@nrm21
Copy link

nrm21 commented Jul 7, 2021

Cool, glad it works for you. Let @ned-kelly know so he can fix it in his downstream.

@kchiem
Copy link

kchiem commented Oct 2, 2021

I made this PR #53 with @nrm21's changes, which directly applies to this repository.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants