ksheumaker / homeassistant-apsystems_ecur

Home Assistant custom component for local querying of APSystems ECU-R Solar System
Apache License 2.0
166 stars 42 forks source link

Incompatibility ECU-R pro and ECU-C owners >read this< #67

Closed HAEdwin closed 2 years ago

HAEdwin commented 2 years ago

If you own an ECU-C or ECU-R with Sunspec logo on the back you will have compatibility issues. image Originally this integration was written based on the standard ECU-R (without SunSpec logo) and happened to be compatible with the ECU-B also. If you own one of these, this integration will work for you but notice this issue when you are running HA 2022.4 or later https://github.com/ksheumaker/homeassistant-apsystems_ecur/issues/62

I know some of you would like to help and make this integration compatible, I do not own an ECU-C or ECU-R with SunSpec logo and that makes it hard to solve your issues. But let's give it a try again... please inform me which ECU type you have and add the log. An alternative solution can be found here: https://github.com/ksheumaker/homeassistant-apsystems_ecur/issues/67#issuecomment-1100723415 and can be made part of this integration later on.

This topic is primairly intended to try to understand why it would not work on the ECU-R with SunSpec logo or ECU-C.

Use integration v1.2.11 when installing, the things I'd like to try only work on this version. Also notice that if you're on HA2022.4 or higher you also have to apply these changes https://github.com/ksheumaker/homeassistant-apsystems_ecur/issues/62

In configuration.yaml add these lines: image

(You will have to restart HA after each change in Python files or configuration.yaml)

I've seen the "connection reset by peer" error that some of you have. Also empty data is returned (and interpreted which causes an error). So let's try to introduce a time out period (self.sock.settimeout(self.timeout)). Modify APSystemsSocket.py and add it to lines 91 and 103 image

Restart HA and look at the log to see any errors. I'll await your feedback.

Xplode85 commented 2 years ago

I also noticed the lack of some data, looks like all the diagnostic data is missing.

I'm not sure if the ECU-R i manage through HA has the logo or not, but probably since it was bought last may. It's my friends setup so i don't have physical access to check atm.

I don't have time to try what's suggested here for a few days, but i'd be happy to give temporary access to to you if you wanted to look into it HAEdwin?

nectur commented 2 years ago

I do have an ECU-R-Pro Looks like version 2.0.6010 if I am reading the data correctly. One thing I noticed while throwing in some debug statements of my own, when I use the inverter command in packetsender it returns a 282 byte response, but the sock.recv() call in read_from_socket is simply returning b'' which bails out of that while loop. I can get the results from packetsender if that is helpful.

2022-04-15 17:26:39 DEBUG (MainThread) [custom_components.apsystems_ecur.config_flow] Starting config flow class... 2022-04-15 17:26:39 DEBUG (MainThread) [custom_components.apsystems_ecur.config_flow] Starting step_user 2022-04-15 17:26:39 DEBUG (MainThread) [custom_components.apsystems_ecur.config_flow] Returning to show form 2022-04-15 17:26:53 DEBUG (MainThread) [custom_components.apsystems_ecur.config_flow] Starting step_user 2022-04-15 17:26:53 DEBUG (MainThread) [custom_components.apsystems_ecur.config_flow] User Input is not none 2022-04-15 17:26:53 DEBUG (SyncWorker_4) [custom_components.apsystems_ecur.APSystemsSocket] Connecting to ECU on [WiFi IP] 8899 2022-04-15 17:26:54 DEBUG (SyncWorker_4) [custom_components.apsystems_ecur.APSystemsSocket] Connecting to ECU on [WiFi IP] 8899 2022-04-15 17:26:54 DEBUG (SyncWorker_4) [custom_components.apsystems_ecur.APSystemsSocket] Sending ECU query to socket APS1100160001END 2022-04-15 17:26:55 ERROR (MainThread) [custom_components.apsystems_ecur.config_flow] APSystemsInvalidData exception: Error getting checksum int from 'Inverter data' data=b'' Traceback (most recent call last): File "/config/custom_components/apsystems_ecur/APSystemsSocket.py", line 212, in check_ecu_checksum checksum = int(data[5:9]) ValueError: invalid literal for int() with base 10: b'' During handling of the above exception, another exception occurred: Traceback (most recent call last): File "/config/custom_components/apsystems_ecur/config_flow.py", line 45, in async_step_user test_query = await self.hass.async_add_executor_job(ap_ecu.query_ecu) File "/usr/local/lib/python3.9/concurrent/futures/thread.py", line 58, in run result = self.fn(*self.args, **self.kwargs) File "/config/custom_components/apsystems_ecur/APSystemsSocket.py", line 158, in query_ecu data = self.process_inverter_data() File "/config/custom_components/apsystems_ecur/APSystemsSocket.py", line 287, in process_inverter_data self.check_ecu_checksum(data, "Inverter data") File "/config/custom_components/apsystems_ecur/APSystemsSocket.py", line 217, in check_ecu_checksum raise APSystemsInvalidData(error) custom_components.apsystems_ecur.APSystemsSocket.APSystemsInvalidData: Error getting checksum int from 'Inverter data' data=b'' 2022-04-15 17:26:55 DEBUG (MainThread) [custom_components.apsystems_ecur.config_flow] Returning to show form

HAEdwin commented 2 years ago

@nectur Returning b'' is one of the things to focus on. Packetsender does it right indeed so there's got to be something in using the socket. First I'd like to try out the timeout. Do the log results above include the use of the timeout statement?

nectur commented 2 years ago

@HAEdwin I tried it both with and without the timeout, it did not make a difference either way.

HAEdwin commented 2 years ago

@nectur What if you remark lines 94 and 95 like this: image and restart.

tv3 commented 2 years ago

@nectur If you have an ECU-r-pro (like me), you can try my alternative. It's an adapted version if this integration, based on querying the internal webserver of the ECU. It has been running for 2 months. I have upgraded HA and HACS without problems too.

copy the files from my repo to replace the current one and restart HA https://github.com/tv3/homeassistant-apsystems_ecur

nectur commented 2 years ago

@tv3 Yup, I have yours currently as a backup. I had jumped over to it after writing issue #59. Just trying to help out @HAEdwin with this debugging.

So I modified what you requested a bit as I didn't care for the concept of the infinite loop: image

It never seemed to be able to parse any data:

`2022-04-16 13:11:08 DEBUG (SyncWorker_6) [custom_components.apsystems_ecur.APSystemsSocket] Sending ECU query to socket APS1100280002216200010298END 2022-04-16 13:11:08 DEBUG (SyncWorker_6) [custom_components.apsystems_ecur.APSystemsSocket] data: b'' 2022-04-16 13:11:08 DEBUG (SyncWorker_6) [custom_components.apsystems_ecur.APSystemsSocket] data: b'' size: 0 end_data: b'' 2022-04-16 13:11:08 DEBUG (SyncWorker_6) [custom_components.apsystems_ecur.APSystemsSocket] data: b'' 2022-04-16 13:11:08 DEBUG (SyncWorker_6) [custom_components.apsystems_ecur.APSystemsSocket] data: b'' size: 0 end_data: b''

... repeating 100x

2022-04-16 13:11:08 DEBUG (SyncWorker_6) [custom_components.apsystems_ecur.APSystemsSocket] data: b'' 2022-04-16 13:11:08 DEBUG (SyncWorker_6) [custom_components.apsystems_ecur.APSystemsSocket] data: b'' size: 0 end_data: b'' 2022-04-16 13:11:08 DEBUG (SyncWorker_6) [custom_components.apsystems_ecur.APSystemsSocket] data: b'' 2022-04-16 13:11:08 DEBUG (SyncWorker_6) [custom_components.apsystems_ecur.APSystemsSocket] data: b'' size: 0 end_data: b'' 2022-04-16 13:11:08 DEBUG (SyncWorker_6) [custom_components.apsystems_ecur.APSystemsSocket] data: b'' 2022-04-16 13:11:08 DEBUG (SyncWorker_6) [custom_components.apsystems_ecur.APSystemsSocket] read_buffer before return: b'' 2022-04-16 13:11:08 DEBUG (SyncWorker_6) [custom_components.apsystems_ecur.APSystemsSocket] inverter data after read: b'' 2022-04-16 13:11:08 DEBUG (SyncWorker_6) [custom_components.apsystems_ecur.APSystemsSocket] Sending ECU query to socket APS1100280030216200010298END 2022-04-16 13:11:10 ERROR (MainThread) [custom_components.apsystems_ecur.config_flow] Unhandled exception: [Errno 32] Broken pipe`

Something I realized, I don't think the settimeout calls you asked for are actually performing a wait if that's what you were intending. They are simply setting the timeout value on the socket, so that it if doesn't receive the data within that timeout it returns. That should only need to be set once per socket. I am going to attempt it with a time.sleep command instead. Will let you know.

Edit: time.sleep didn't help either.

HAEdwin commented 2 years ago

@nectur It is indeed a timeout on the socket and it is said applies independently to each call to socket read/write operation (https://pyquestions.com/how-to-use-python-socket-settimeout-properly) I've examined the effects closely here: https://github.com/ksheumaker/homeassistant-apsystems_ecur/discussions/50 But nevertheless it seems not to be the issue here.

Does 216200010298 indeed reflect your ECU-ID? It's odd that the ECU does not respond to this query. How does Packetsender respond to APS1100280002216200010298END?

HAEdwin commented 2 years ago

This part is weird image I think line 135 can get remarked it's a duplicate for 138 which is the better approach.

HAEdwin commented 2 years ago

From what I remember, closing and opening the socket did help. Maybe alter this part (from where it goes wrong): image to this image

nectur commented 2 years ago

closing and reopening before each command seemed to do the trick!

image

Also noticed that the close socket call in the lifetime energy = 0 case was an incorrect method name, fixed that on my copy.

Added it to the following inverter signal call as well, not sure if it was necessary or not...

HAEdwin commented 2 years ago

Oh I see, self.socket_close() is wrong indeed well noticed! Not sure if self.open_socket should be sock = self.open_socket() I'm a bit scared for socket exhaustion with this solution so it might break later on. I question myself why this is needed. Looks like the ECU doesn't bind or something.

nectur commented 2 years ago

open_socket() doesn't return anything, so setting it to a local variable won't do anything.

I will give it some run time and see how it looks. I do have a plug already in the loop that watches for 2 'unavailable' reads in a row and then will reboot the ECU due to the ECU's own stability issues.

Minor item I am seeing already though is for some reason it doesn't seem to have created the extra inverter entities for voltage, frequency, temp, and signal. Not major for me at all right now. At some point I might try re-configuring the integration and see if they come back, but I am out of time to play with this for right now.

HAEdwin commented 2 years ago

Indeed the ECU "skips a beat" sometimes but with the interval of 60 seconds that I use, it's not really an issue because the ECU refreshes data once every 5 minutes anyway. With the current HA 2022.4 I can see the last time it skipped. image

Strange if the extra inverter entities are missing, does the inverty_qty match with the number of inverters? Anyway, on my part everything is still running on the "older" ECU-R with the addition of closing and opening the port in between the write/read operations. I leave it running for a while. Thanks for your help.

The integration allready provides the cache count and triggers after five unsuccesfull reads where the cache was needed, you might want to add the reboot there. It is known that the ECU does some maintenance in the early morning where no results are returned and the cache needs to be used five times. The timeframe is almost the same everytime 2022-04-17 02:52:17 ERROR (MainThread) [custom_components.apsystems_ecur] Error fetching apsystems_ecur data: Error using cached data more than 5 times. See log, and try power cycling the ECU. but I just leave it running and it recovers from it after the ECU is finished with maintenance.

HAEdwin commented 2 years ago

@nectur Everything still running? No issues here with my standard ECU-R.

Changes we made so far which help the compatibility with the ECU-R with ECU-R pro firmware (SunSpec logo on the back) AND ECU-C. We are now looking at how stable it remains for a longer period of time.

To sum things up:

nectur commented 2 years ago

@HAEdwin Everything seems to still be running well. No restarts have been necessary in the last almost 48 hours since I stopped playing with it. I will probably give it a try to delete and re-install the integration to see if I can get those other entities. I can see the values for them in the data that is being processed in the logs.

nectur commented 2 years ago

@HAEdwin Looks like 1.2.12 fixes my missing entities issue. It was the EntityCategory enum change. I was about to make a pull request for it and noticed that it had already been merged in and my HACS pull of 1.2.11 did not have it. Thanks!

HAEdwin commented 2 years ago

@nectur I didn't expect you missed #62 anyway, great that Kyle merged and released 1.2.12. I'll close this issue. Should stability issues arise we can open a new issue.

HAEdwin commented 2 years ago

@nectur By the way, how is your ECU-R connected? WiFi or Ethernet, just checking.

nectur commented 2 years ago

@HAEdwin I have it on WiFi right now. Do you want me to test if it supports ethernet? I wouldn't mind getting a device off my WiFi if it might work :)

HAEdwin commented 2 years ago

@nectur If you want you could try but I don't expect you to test it. With the older "standard" ECU-R it only works via WiFi so I don't expect the ECU-R with pro firmware will work for this combination Ethernet and Integration.

dewi-ny-je commented 2 years ago

I'd be happy also to get rid of WiFi so I can test it.

Is there a way to change the IP address after setting up the integration? I renamed the solar panels and it's a lot of work to do again. I could add a second integration but then it would create new entities and it's not a "tidy" approach.

Otherwise you use the fork by tv3 which scrapes the web interface and works also on ethernet

TexTown commented 2 years ago

I did this by editing the core.config_entries file from HA, it contains a section for your ECUR (just search within the file for ecur) In that section, there is this part:

        "data": {
            "host": "192.168.2.99",
            "scan_interval": 300
        },

If you change those values, reboot HA and the new values should be used. Caution: editing these core config files might nuke your HA, so be carefull here. You find the file in the /config/.storage location

dewi-ny-je commented 1 year ago

I'd be happy also to get rid of WiFi so I can test it.

Is there a way to change the IP address after setting up the integration? I renamed the solar panels and it's a lot of work to do again. I could add a second integration but then it would create new entities and it's not a "tidy" approach.

Otherwise you use the fork by tv3 which scrapes the web interface and works also on ethernet

@HAEdwin it works just fine also on wired ethernet as long as I reboot every night. I tried tv3, which in theory should be more reliable with ECU-R PRO, but the current main branch doesn't even return (anymore?) the power of each panel, so I'll be back to this component, and actually I'll switch directly to your fork instead of ksheumaker, since you are the one currently active on the project (can you maybe enable "Issues" on your fork?).