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

Latest HA 2023.9.0 breaks SolArk Integration #40

Closed
poldim opened this issue Sep 7, 2023 · 30 comments
Closed

Latest HA 2023.9.0 breaks SolArk Integration #40

poldim opened this issue Sep 7, 2023 · 30 comments

Comments

@poldim
Copy link
Contributor

poldim commented Sep 7, 2023

Everything worked fine until I updated to 2023.9.0, reverting to 2023.8.4 allows it to work again.

This error originated from a custom integration.

Logger: custom_components.solark_modbus.hub
Source: custom_components/solark_modbus/hub.py:114
Integration: SolArk Inverter Modbus (documentation, issues)
First occurred: 8:28:47 PM (51 occurrences)
Last logged: 8:32:57 PM

Reading realtime data failed! Inverter is unreachable.

I've scraped the changelog for anything modbus related, two stand out:

  • Modbus switch, allow restore “unknown” (@janiversen - #99533) (modbus docs) (beta fix)
  • use write_registers also for target temp (@brunoenten - #97475) (modbus docs)
  • Bump pymodbus v3.4.1. (@janiversen - #97612) (modbus docs)
  • modbus: use pb not pymodbus consistently as name. (@janiversen - #97780) (modbus docs)
  • Fix Flexit mypy error in pymodbus (@janiversen - #97799) (flexit docs)
  • Handle explicit Modbus NaN values (@joanwa - #90800) (modbus docs)
  • Replace Float ‘nan’ with None for modbus floats (@String-656 - #93832) (modbus docs)
  • modbus: remove unused constants and get 100% coverage. (@janiversen - #97779) (modbus docs)
  • Move all used modbus constants to Stiebel (@joostlek - #98044) (stiebel_eltron docs)
  • Make changes in modbus trigger a full CI run (@emontnemery - #98055)
  • Modbus: set state_class etc in slaves. (@janiversen - #98332) (modbus docs)
  • Address late modbus review (@janiversen - #99123) (modbus docs)
  • Add possibility to have multiple values for every modbus hvac mode (@escoand - #98829) (modbus docs)
  • Add modbus test for configuration errors (@janiversen - #98697) (modbus docs)
  • Modbus switch, allow restore “unknown” (@janiversen - #99533) (modbus docs) (beta fix)
@janiversen
Copy link

You need to ask the custom component to update.

@legomind
Copy link

legomind commented Sep 7, 2023

can confirm this problem exists in 2023.9.0. Redownloading from HACS does NOT fix the issue.

@pbix
Copy link
Owner

pbix commented Sep 7, 2023

You need to ask the custom component to update.

What update is required? I do not see what exactly has broken.

I saw all these modbus related changes in the changelog for HA and decided to wait a bit before upgrading my system even to 2023.8.4 (many modbus related regressions had to be addressed in the changelog I saw due to changes in 8.0 it appeared to me).

Anyway I have not yet had a chance to investigate and would welcome any input.

@legomind
Copy link

legomind commented Sep 7, 2023

@pbix is there any way to get the details of the exception logged?

The exception being raised is caught here: https://github.com/pbix/HA-solark-PV/blob/9df26260e0f3f14939fff97151d359bc6b952b3a/custom_components/solark/hub.py#L113C10-L113C10

But the underlying error is not actually logged, as far as I can see.

@pbix
Copy link
Owner

pbix commented Sep 7, 2023

Possible regression in 2023.9.0. Please avoid updating for now and let's see what becomes of this upstream.

home-assistant/core#99784

@pbix
Copy link
Owner

pbix commented Sep 8, 2023

Looks like regression #1758 in pymodbus 3.5.0 is responsible for this.

Home Assistant 2023.9.1 is slated to move to v3.5.1 to address this.
home-assistant/core#99940

I think there will be a new HA release soon so please hang on a little longer.

A confirmation by anyone upgrading to 2029.9.1 or later in this thread would be appreciated so I can close this issue.

@dscowan
Copy link

dscowan commented Sep 9, 2023

I upgraded to 2023.9.1 and it is still broken

@EiNSTeiN-
Copy link
Contributor

Same, running 2023.9.1 and I'm still getting a Inverter is unreachable exception at solark_modbus/hub.py:114

@legomind
Copy link

legomind commented Sep 9, 2023

Wait for 2023.9.2. It has the actual fix in it.

@alexdelprete
Copy link

Hi @pbix,

sorry to chime in, but I thought it might help: for my component the problem was byteorder=Endian.Big. Had to change all those to byteorder=Endian.BIG.

The change in pymodbus 3.5.0 was documented here: https://github.com/pymodbus-dev/pymodbus/blob/dev/API_changes.rst

image

Hope it helps.

@pbix
Copy link
Owner

pbix commented Sep 10, 2023

Hi @alexdelprete,

Thanks for the tip.

Did you have to version detect to use lower case for versions prior to 3.5.0 and upper case for later versions?

What a pain.

@alexdelprete
Copy link

alexdelprete commented Sep 10, 2023

Hi Paul,

Did you have to version detect to use lower case for versions prior to 3.5.0 and upper case for later versions?

No, I decided to force the use of HA 2023.9.1 and pymodbus 3.5.1 so I could immediately release a working version. I hope to not cause too many problems to users. I'll wait for feedbacks and then, if needed, I might implement what you suggested. I saw your code, and I noticed you already check pymodbus version for other things, it's a good idea.

What a pain.

Absolutely, I spent many hours debugging this (mainly because I'm not really a developer) and I found the solution only because I clicked on the right link and when debugging, examining the exception I was receiving, it contained the string "Big". So I associated Big to that Endian case change, and that was definitely it. A little bit of luck and years of experience in troubleshooting IT issues. But you're right: "what a pain..." :)

@pbix
Copy link
Owner

pbix commented Sep 10, 2023

Update to Main branch allows this integration to operate despite the breaking changes introduced without warning into the pymodbus library. The SolArk integration should now work with a releases of HA.

Thanks to @alexdelprete for pointing out one of the breaking changes saving me many hours if debug no doubt. Your integration may benefit from my resolution method as it works with all versions of HA. Just avoid using the pymodbus Endian module altogether.

Would love to hear from other users that they met success after updating to this release.

@EiNSTeiN-
Copy link
Contributor

I've just updated (redownloaded from HACS to be sure) and the integration still seems broken after restarting HA

@alexdelprete
Copy link

Thanks to @alexdelprete for pointing out one of the breaking changes saving me many hours if debug no doubt.

glad to help. I hope that's the only issue we're facing: I have 3 users reporting it worked, but now 1 user tells me it's still not working. :(

@pbix
Copy link
Owner

pbix commented Sep 10, 2023

I've just updated (redownloaded from HACS to be sure) and the integration still seems broken after restarting HA

Do you have anything in your logs to report?
It is working for me using a TCP connection and HA 2023.9.1.

What type of connections are you using?

@EiNSTeiN-
Copy link
Contributor

EiNSTeiN- commented Sep 10, 2023

I added a log of the ConnectionException that is raised on hub.py:114 and it only says [Connection] Failed to connect[ModbusTcpClient(solark.local:502)] which isn't super helpful, I can confirm that connecting to solark.localdomain/solark.local port 502 works on my local network though...

@EiNSTeiN-
Copy link
Contributor

Just to be sure I tried adding the device in HA by IP address to rule out any DNS resolution issue and I confirmed mbusb is running on port 502 on the host that is connected to SolArk (this was working and hasn't changed)

I'll wait for other users to report before digging more

@pbix
Copy link
Owner

pbix commented Sep 11, 2023

I made another update. Try again.

@EiNSTeiN-
Copy link
Contributor

9d17f3e worked, thanks!

@legomind
Copy link

can confirm it works on 2023.9.1

@alexdelprete
Copy link

I made another update. Try again.

Paul, so for the pymodbus issues you dumped the endian constants and then did an explicit connect(), correct?

I already had the explicit connect() in my code, so after fixing the Endian, I guess there's no other things to fix. Correct me if I'm wrong.

Thanks.

@pbix
Copy link
Owner

pbix commented Sep 11, 2023

Alex,

You are correct. I was not doing an explicit connect before as it was always automatically done.
A correction for this is in v3.5.2 but I did not want to wait.
pymodbus-dev/pymodbus#1772

For the Endian problem, which was an undocumented breaking change BTW. I dumped the Endian module and just used the native '>' & '<' characters so as to avoid any backwards compatibility issues.

Hopefully we are set until the next breaking change comes our way.

@legomind
Copy link

update: all sensors changed to unknown after about 30 minutes. Rolling back home assistant to 2012.8.4 for now.

@alexdelprete
Copy link

alexdelprete commented Sep 11, 2023

Paul,

thanks a lot. For my next major version, I'll be moving from modbus to REST/json. The ABB/FIMER inverters use a card that does Modbus, but we discovered it also supports REST calls and provide json data with all necessary information. Unfortunately the vendor told me the REST API is not documented / official, so they wouldn't provide assistance/information, but luckily we managed to find out how it works.

So long to modbus for me...it was fun learning new things and all the register map parsing, but I don't like to depend on pymodbus etc. This last incident was the classic straw that broke the camel's back. :)

Now I need to find a good component that does REST calls and parses json, so I can adapt my component.

Thanks for your advices.

@pbix
Copy link
Owner

pbix commented Sep 12, 2023

update: all sensors changed to unknown after about 30 minutes. Rolling back home assistant to 2012.8.4 for now.

@legomind

I have tested both with v2023.9.1 and v2023.8.4 with the latest on github and am unable to reproduce your results. To simulate a communications error I disconnected and reconnected my serial link cable. The both version recovered and not errors were shown in the logs. I am using a TCP connection.

Are you using a serial or TCP connection?

I will need help from you to document a reproducible scenario for me.

@legomind
Copy link

I am using a tcp connection. I will update once more tonight and let you know the results & include logs & configs.

@legomind
Copy link

legomind commented Sep 12, 2023

Ok exactly 15 minutes after upgrade, the following message is logged, and all sensors changed to unavailable.

I am using a modbus rtu gateway connected via Ethernet cable. What else do you need from me to troubleshoot?

Logger: custom_components.solark_modbus.hub
Source: custom_components/solark_modbus/hub.py:116
Integration: SolArk Inverter Modbus ([documentation](https://github.com/pbix/HA-solark-PV), [issues](https://github.com/pbix/HA-solark-PV/issues))
First occurred: 8:29:01 PM (92 occurrences)
Last logged: 8:36:36 PM

Reading realtime data failed! Inverter is unreachable.

@legomind
Copy link

Working overnight on 2023.9.2

@pbix
Copy link
Owner

pbix commented Sep 24, 2023

Resolved at v 1.3.0

@pbix pbix closed this as completed Sep 24, 2023
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