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

Connection time out #57

Closed
rzulian opened this issue Jul 12, 2022 · 18 comments · Fixed by #143
Closed

Connection time out #57

rzulian opened this issue Jul 12, 2022 · 18 comments · Fixed by #143
Labels
help wanted Extra attention is needed

Comments

@rzulian
Copy link

rzulian commented Jul 12, 2022

I'm getting a lot of connection time out using a polling interval below 10 secs. I was getting the same problems with other component for modbus-solaredge.
I think that this is related to the new HA version 7.2, and I was hoping that this component wasn't affected using a much more recent pymodbus library.

Screenshot 2022-07-12 at 14 12 30

Do you have the same problems?

@WillCodeForCats
Copy link
Owner

Tested on 2022.7.3 working OK with 1 second interval for 2 inverters and 1 meter.

Video:
https://user-images.githubusercontent.com/48533968/178546814-91d76a4e-bb93-4e49-a188-7f911437a3c9.mp4

@rzulian
Copy link
Author

rzulian commented Jul 12, 2022

I need to investigate what's wrong on my side then ...
Thanks

@rzulian rzulian closed this as completed Jul 12, 2022
@rzulian
Copy link
Author

rzulian commented Jul 13, 2022

I was suspecting that I had a performance problem in my HA in a VM. So I've tested a dev container with only this component, but I'm still unable to set the polling to less than 10 secs. @WillCodeForCats Any idea on what I can check?

@WillCodeForCats
Copy link
Owner

How long did you wait after changing the time? Let it wait for 2 minutes and see if it's just an issue with the inverter thinking the old connection is still open.

@rzulian
Copy link
Author

rzulian commented Jul 14, 2022

I've tried your suggestion and I've also restarted the inverter, without success. I've restored HA Core 2022.6.7, which, to my knowledge, was working fine, but it isn't.
I've tried to use ```pymodbus==3.0.0dev3", which is actually working without problem. Only an annoying Warning

2022-07-14 13:23:22.667 WARNING (MainThread) [asyncio] Executing <Task finished name='Task-2317' coro=<SolarEdgeModbusMultiHub.async_refresh_modbus_data() done, defined at /workspaces/core/config/custom_components/solaredge_modbus_multi/hub.py:190> result=None created at /workspaces/core/homeassistant/core.py:452> took 0.316 seconds

Screenshot 2022-07-14 at 15 23 11

@WillCodeForCats WillCodeForCats added the help wanted Extra attention is needed label Jul 16, 2022
@WillCodeForCats
Copy link
Owner

PR #59 might help with the immediate connection error after reconfigure.

@rzulian
Copy link
Author

rzulian commented Jul 21, 2022

Are you going to merge also the data_update_coordinator. I would test the new main with also the battery support

@WillCodeForCats
Copy link
Owner

Are you going to merge also the data_update_coordinator. I would test the new main with also the battery support

No, I'm not.

@rzulian
Copy link
Author

rzulian commented Jul 21, 2022

BTW, digging into a very slow trigger issue, I was discussing about the possibility that the solaredge_modbus can be athe problem. In the discord channel they suggested me to "It looks like that package does the same thing. It should be an easy fix, you can just change line 187 there to use an executor job like "update_result = await self._hass.async_add_executor_job(self._read_modbus_data)"
But the better way to do it is to use the asyncio methods which the pymodbus library seems to have".

@WillCodeForCats
Copy link
Owner

I have been looking at converting it to use the async client of pymodbus. The first step is to use the data update coordinator then probably async pymodbus later. The branch is there but it doesn't work yet. Once I get it to work with sync then I will work on async.

Because it currently uses sync if you have a fast polling interval it can halt all home assistant execution while reading or waiting for a modbus timeout, so fast polling is not advised.

@WillCodeForCats
Copy link
Owner

I will also consider the async_add_executor_job wrapper if it's faster to implement

@WillCodeForCats
Copy link
Owner

The data-update-coordinator branch is working now, you could try it out if you want. It has not been tested, I've only done enough work so that it loads and runs.

@WillCodeForCats
Copy link
Owner

I should have all of the sync modbus calls covered with async_add_executor_job as of commit d8e3254

@WillCodeForCats WillCodeForCats linked a pull request Jul 29, 2022 that will close this issue
@WillCodeForCats WillCodeForCats removed a link to a pull request Jul 29, 2022
@rzulian
Copy link
Author

rzulian commented Jul 29, 2022

I can test it early next week

@WillCodeForCats
Copy link
Owner

I've created a pre-release v2.1.0-pre.1

@WillCodeForCats
Copy link
Owner

I've created a pre-release for the latest pymodbus dev version: v2.1.2-pymodbusdev.1

@WillCodeForCats
Copy link
Owner

pymodbus has released v3.0.0

I have updated the pymodbus-dev branch to support the latest release version. You can download the branch to try it manually: https://github.com/WillCodeForCats/solaredge-modbus-multi/archive/refs/heads/pymodbus-dev.zip

@WillCodeForCats
Copy link
Owner

pymodbus v3.0.0 is now in Release v2.2.6-pre.3

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants