Is this something you are actively working on? I believe (better) SMA support would be generally nice to have in OpenEMS.

Implementation of SMA STP 10.0 Hybrid ESS.
The implementation is split into Battery, Inverter and DC Charger. The inverter only works with that battery, but the implementation had to be split up as the devices use different Modbus Unit IDs.

It sounds like bad abstraction if they only work in this combination anyway. If Modbus Unit-ID is your only problem, we should be able to find an alternative - see e.g. https://github.com/OpenEMS/openems/blob/develop/io.openems.edge.ess.byd.container/src/io/openems/edge/ess/byd/container/Config.java#L23-L30

Is this something you are actively working on? I believe (better) SMA support would be generally nice to have in OpenEMS.

Implementation of SMA STP 10.0 Hybrid ESS.
The implementation is split into Battery, Inverter and DC Charger. The inverter only works with that battery, but the implementation had to be split up as the devices use different Modbus Unit IDs.

It sounds like bad abstraction if they only work in this combination anyway. If Modbus Unit-ID is your only problem, we should be able to find an alternative - see e.g. https://github.com/OpenEMS/openems/blob/develop/io.openems.edge.ess.byd.container/src/io/openems/edge/ess/byd/container/Config.java#L23-L30

Yes, we are actively working on this, and we have such an inverter in use on a productive system.
I agree that the abstraction is not very good, and I was trying to get around it too. We definitely need both Unit IDs. Under one unit ID (3 by default) you'll find a protocol needed for write access (similar to the one of ESS Sunny Island), and under the other unit ID (126 by default), there is a SunSpec protocol, as in the SMA PV inverter.
I couldn't find a way of having two unit IDs in one component, as the unit ID is part of the AbstractOpenemsModbusComponent. In the example you give, there are two different modbus bridges (so I assume two different IP addresses), but only one unit ID.
I was also thinking that instead of combining the inverter and the battery to a GenericManagedEss, you have a EssSma combining them, but in the end I didn't see the benefit of it.
I would be glad if you or anyone else has a suggestion of how to deal with different unit IDs in the same component. I saw at least two other modbus devices that use different unit IDs (though in both these cases, one unit ID can be ignored, as the registers are not too relevant).

Best regards,
Thomas

Codecov Report

❌ Patch coverage is 67.15928% with 200 lines in your changes missing coverage. Please review.

@@              Coverage Diff              @@
##             develop    #2618      +/-   ##
=============================================
+ Coverage      59.45%   59.51%   +0.07%     
  Complexity       112      112              
=============================================
  Files           2914     2922       +8     
  Lines         125438   126028     +590     
  Branches        9392     9410      +18     
=============================================
+ Hits           74566    74995     +429     
- Misses         48038    48183     +145     
- Partials        2834     2850      +16     

Backported changes we had done, mainly:

1. Adaption to new SunSpec implementation
2. A "Smart Mode" similar to GoodWe was introduced

Regarding the problem with the two different Unit-IDs, I have a suggestion but neither time to implement it nor do I know if it's really a good idea. It is as follows: The Unit-ID should not be part of the Modbus component, but rather be part of the Modbus task, as it is really part of the request, like you see here. So the constructor of the FC3ReadRegistersTask for instance would not only have a start address, a priority and Modbus elements, but also a Unit-ID.
Now this would of course be a huge change in a very central component, but to me it would make a bit more sense this way.

This PR has been automatically marked as stale due to inactivity. It will be closed in 7 days if no further activity occurs.

I would like to add a similar setup to my OpenEMS setup, so I wanted to ask what is missing for this PR to be merged, apart from additional test coverage. Maybe I could find the time to help.