Add support for multiple inverters

[mpredfearn]

This is basically a slight rework of #12, rebased on current master.

Inverters must (for now) be numbered 1,2,3 etc on RS485-1 bus, with the primary inverter being ID1.

All inverter sensor names are now prefixed by the inverter number.

[mpredfearn]

@willcodeforcats please could you test this? @binsentsu any blockers to getting this upstream?

[binsentsu]

No blockers for me. Can't test it myself though. @WillCodeForCats Have you got feedback on this one?

[WillCodeForCats]

The pace on changes is too fast for me to keep up with everything else I've got going on in life, and I don't have any batteries so I can't really test that aspect. But I see the control part is not adapted for multiple inverters, so probably having only some of the code multi-inverter aware other parts single-only is probably not a good idea.

I would just say compare it to my fork since that's what I am using and confirmed working for what I need (which is multiple inverters, no batteries, no control). Ideally multiple inverter support should have been added before significantly expanding it to include unit control, rather than now trying to work backwards. I see in the docs that multiple units and multiple battery is possible, so I'd say anything from here on out should include multiple inverter awareness or stick to single unit systems, otherwise I think this runs the risk of a problem if multiple units are set to conflicting or mismatching control modes. I'm not really willing to try setting conflicting control modes on my hardware outside of a lab environment where possible damage to the hardware is unknown.

[binsentsu]

@mpredfearn Is my understanding correct?: As all entities are now prefixed with the inverter number. This PR introduces a breaking change as all entities for people with just one inverter are now renamed with the prefix. Just a thought: Would it be backwards compatible if we would omit the prefix in case numberOfInverters = 1 ?

[mpredfearn]

@mpredfearn Is my understanding correct?: As all entities are now prefixed with the inverter number. This PR introduces a breaking change as all entities for people with just one inverter are now renamed with the prefix. Just a thought: Would it be backwards compatible if we would omit the prefix in case numberOfInverters = 1 ?

Correct, in it's current form it is a breaking change since all sensors become prefixed by the inverter number. I could eliminate that as you suggest, by only prefixing the sensor name if the configured number of inverters is > 1, which makes sense, but is a bit inconsistent with how meters are named, since they are always prefixed by meter number even if only a single meter is configured. What would you prefer?

[mpredfearn]

@binsentsu Updated the PR such that inverter sensor names are only prefixed with an inverter number if multiple inverters are configured. If only a single inverter is configured then the sensor names are unchanged from previous versions.

[rzulian]

@mpredfearn I would love to test the support for multiple inverters, but I'm not really sure I'm understanding the current status. Which version should I test? Or should I test your fork?

[mpredfearn]

@rzulian your assistance in testing this PR would be hugely appreciated! My system has only a single inverter so I cannot test it unfortunately :-/ nevertheless I think that this is an important feature to get merged, so if you could test it that would be awesome :+1: The current status is, with this MR branch, you should get sensors created for each inverter. With a single inverter system, the names of the sensors are just sensor.solaredge_modbus_accurrent, sensor.solaredge_modbus_acpower, etc. But if you set "number of inverters" to a value other than 1 when configuring the integration, you will get sensors prefixed with the inverter number i.e. sensor.solaredge_modbus_i1_accurrent, sensor.solaredge_modbus_i2_accurrent, sensor.solaredge_modbus_i1_acpower, sensor.solaredge_modbus_i2_acpower, etc. Please do test it using this branch and report back - also any improvements you can think of like maybe the integration should create the sensor.solaredge_modbus_acpower sensor, but make it the sum of all configured inverters? Things like that.

[rzulian]

@mpredfearn I'm happy to help on this. I will submit a PR to allow non consecutive inverters IDs. My primary inverter, connected to the battery and the meter, has the ID 1, the other connected to the panels has ID 3. The meter has ID 2. I've managed to let it work in the meantime.

I'm trying to clarify the different sensors that I'm reading from the different threads, here and in the community.

This is my current understanding using your sensors and running this branch, using also the solaredge app to double check:

   power_battery_charging:
        friendly_name: "Power - Battery Charging"
        unit_of_measurement: "W"
        value_template: "{{ max(states('sensor.solaredge_battery1_power') | float,0) | abs() }}"
    power_battery_discharging:
        friendly_name: "Power - Battery Discharging"
        unit_of_measurement: "W"
        value_template: "{{ min(states('sensor.solaredge_battery1_power') | float,0) | abs() }}"

Those are correct

      power_battery_charging_ac:
        friendly_name: "Power - Battery Charging from AC"
        unit_of_measurement: "W"
        value_template: "{{ min(states('sensor.solaredge_i1_ac_power') | float,0) | abs() }}"

I've noriced that when the battery is charging (i.e power_battery_charging is >0), sensor.solaredge_i1_ac_power is more or less equal to power_battery_charging. Please note that I'm not charging the battery from the grid, but it should probably be if sensor.solaredge_i3_ac_power == 0 and power_battery_charging >0 then power_battery_charging, which is similar to

  - name: "Solar Grid To Battery W"
        unique_id: solar_grid_to_battery_w
        unit_of_measurement: "W"
        icon: mdi:battery-positive
        state: >
          {% if (is_state('sensor.solaredge_ac_power', '0')) and ((states('sensor.solaredge_battery1_power') | float(0)) > 0) %}
            {{(states('sensor.solaredge_battery1_power') | float(0))}}
          {% else %}
            0
          {% endif %}

power_grid_import:
        friendly_name: "Power - Grid Import"
        unit_of_measurement: "W"
        value_template: "{{ ( min(states('sensor.solaredge_m1_ac_power') | float ,0) | abs()) - (states('sensor.power_battery_charging_ac') | float) }}"
      power_grid_export:
        friendly_name: "Power - Grid Export"
        unit_of_measurement: "W"
        value_template: "{{ max(states('sensor.solaredge_m1_ac_power') | float,0) | abs() }}"

I don't think we should subtract (states('sensor.power_battery_charging_ac') | float) as the meter is between the grid and the inverters.

      power_solar_generation:
        friendly_name: "Power - Solar Generation"
        unit_of_measurement: "W"
        value_template: "{{ (max(states('sensor.solaredge_i3_ac_power') | float ,0)) + (states('sensor.power_battery_charging_solar') | float) - (states('sensor.power_battery_discharging') | float) }}"

Why are you subtracting power_battery_discharging?

[mpredfearn]

Hi @rzulian Getting the power sensors right was a massive pain because the raw values from the inverter are quite difficult to work with. I charge my battery from AC overnight with cheap electricity, but the "inverter AC power" is never negative. But when the battery is being discharged, the "inverter AC power" includes the power coming from the battery. The same is true of the energy sensors. The energy from the inverter always goes up, including when the battery is discharging, but it never goes down when the battery is being charged from the grid. This asymmetry gave rise to most of my issues getting the sensors right. The other major issue for me is that the panel power is not reported separately - the panel and battery power gets lumped together because of the single inverter. Therefore I need some calculations to separate the power from the panels from that from the battery.

As you have 2 inverters, one for the panels and one for the battery, it sounds like your sensor set up may be simpler, because inverter 1's AC power matches up to what the battery is doing and inverter 2 is what the panels are doing. Please feel free to edit the wiki page and add your sensors set up, but the sensors I've listed are correct for my set up with a single inverter, single battery and single meter.

This one: power_grid_import: friendly_name: "Power - Grid Import" unit_of_measurement: "W" value_template: "{{ ( min(states('sensor.solaredge_m1_ac_power') | float ,0) | abs()) - (states('sensor.power_battery_charging_ac') | float) }}"

I don't think we should subtract (states('sensor.power_battery_charging_ac') | float) as the meter is between the grid and the inverters.

I am calculating the power used by the house loads which is coming from the grid. So I take the power coming from the grid, measured by the meter, and subtract what is being directed to the battery. That leaves the flow to house loads.

power_solar_generation: friendly_name: "Power - Solar Generation" unit_of_measurement: "W" value_template: "{{ (max(states('sensor.solaredge_i3_ac_power') | float ,0)) + (states('sensor.power_battery_charging_solar') | float) - (states('sensor.power_battery_discharging') | float) }}"

Why are you subtracting power_battery_discharging?

Because I am calculating the power coming from the panels. Since on my single inverter set up , "inverter AC power" includes both power coming from panels and battery, I subtract the flows to and from the battery from the "inverter AC power" to get (an approximation of) the DC power from the panels. The panel power is not presented by the inverter, so I had to calculate it. Maybe as you have 2 inverters, one for panels and one for the battery, you don't have that problem because your 2 inverter powers are separate?

[mpredfearn]

@rzulian If you have to make further changes on top of my commit to add the basic multiple inverters, please do and we can either get this merged first followed by yours, or I can close this and merge your PR including the support for non consecutive IDs.

[rzulian]

@mpredfearn I've submitted a PR on your fork. I was thinking to create a set of sensors based on different scenarios (e.g 1 inverter, 1 battery, 1 meter, or 2 inverters, 1battery, 1 meter). But there are so many different layouts ( see this https://ressupply.com/documents/solaredge/StorEdge_Applications_Connection_and_Configuration_Guide.pdf ), that it's really difficult to validate the scenarios. And also I don't know if this would be faster than using templates. I will add my sensors to the wiki, and maybe we could align the names to a standard, using for example the tesla naming (e.g grid_to_house, generation_to_battery)

[thargy]

Has this been abandoned? I have 2 inverters, each with a battery and 2 strings of PV. My modbus ids are also non-consecutive, in fact, I believe that slave inverters cannot be numbered 1 or 2, and so have to start at 3 (from issues we had during setup and a call to solaredge).

Interested in testing this, or getting it working if it's been abandoned.

[mpredfearn]

Has this been abandoned. I have 2 inverters, each with a battery and 2 strings of PV. My modbus ids are also non-consecutive, in fact, I believe that slave inverters cannot be numbered 1 or 2, and so have to start at 3 (from issues we had during setup and a call to solaredge).

Interested in testing this, or getting it working if it's been abandoned.

If you have multiple inverters, you should use the integration over at https://github.com/WillCodeForCats/solaredge-modbus-multi

[thargy]

Has this been abandoned. I have 2 inverters, each with a battery and 2 strings of PV. My modbus ids are also non-consecutive, in fact, I believe that slave inverters cannot be numbered 1 or 2, and so have to start at 3 (from issues we had during setup and a call to solaredge). Interested in testing this, or getting it working if it's been abandoned.

If you have multiple inverters, you should use the integration over at WillCodeForCats/solaredge-modbus-multi

Thank you!!!!