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

Add modbus address selection #59

Merged
merged 10 commits into from
Aug 1, 2022

Conversation

cabeljunky
Copy link
Contributor

Not all the inverters are on the default address 1.
If a users has more device on the modbus network the mandatory address 1 is not the preferred and can mess up the network.

The change contains the option to select the address in the flow setup.
Then is is saved and is used in all the all the places where the unit was set.

Also this will also make it easier to configure multiple SolarEdge converters.

@WillCodeForCats
Copy link

This approach will only work for systems where every inverter has an individual modbus/tcp connection or for single inverter systems.

This will conflict with #12 or #47 where multiple inverters are read in sequence through a single modbus/tcp connection. Inverters chained through the RS-485 bus cannot be read by duplicating the integration since the solaredge inverter only allows one active modbus/tcp connection at a time, so eventually you will end up with overlapping connection attempts (and failures) trying poll a single IP. You could do a configurable base address and iterate upwards from that, but the only reliable way to read a chain of inverters is by iterating through on a single connection.

@cabeljunky
Copy link
Contributor Author

cabeljunky commented Oct 13, 2021

I agree that if you have daisy changed the inverters this will be a issue.
But if you have multiple inverters and also separated connection your solution of #12 and #47 will not work.

Also I think that this can be combined with the feature #12 and #47.
The main issue that you have there is that you also your fixed index of 1. And if your inverter is not on that address you can't use your module.

So if you look at the feature #12

def read_modbus_data_inverters(self):
        for inverter_index in range(self.number_of_inverters):
            inverter_prefix = "i" + str(inverter_index + 1) + "_"
            inverter_data = self.read_holding_registers(unit=inverter_index + 1, address=40071, count=38)

can than be changed to:

def read_modbus_data_inverters(self):
        for inverter_index in range(self.number_of_inverters):
            inverter_prefix = "i" + str(inverter_index + self._address) + "_"
            inverter_data = self.read_holding_registers(unit=inverter_index + self._address, address=40071, count=38)

@WillCodeForCats
Copy link

I will add this to my fork and try it.

WillCodeForCats added a commit to WillCodeForCats/home-assistant-solaredge-modbus that referenced this pull request Oct 18, 2021
WillCodeForCats added a commit to WillCodeForCats/home-assistant-solaredge-modbus that referenced this pull request Oct 18, 2021
@ctrl-alt-d
Copy link

You save my day! How to vote to this PR will be merged? It runs like a charm.

@cabeljunky
Copy link
Contributor Author

Hello @WillCodeForCats,

Did you had time to check the change?
Or do you still have concerns

@binsentsu binsentsu changed the base branch from master to feature/addressselection August 1, 2022 19:43
@binsentsu binsentsu merged commit a48bdec into binsentsu:feature/addressselection Aug 1, 2022
@WillCodeForCats
Copy link

Hello @WillCodeForCats,

Did you had time to check the change? Or do you still have concerns

@cabeljunky Sorry for the very delayed response. I am no longer using this integration, I've moved on to a different one.

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

Successfully merging this pull request may close these issues.

None yet

4 participants