mkaiser / Sungrow-SHx-Inverter-Modbus-Home-Assistant

Sungrow SH Integration for Home Assistant for SH3K6, SH4K6, SH5K-20, SH5K-V13, SH3K6-30, SH4K6-30, SH5K-30, SH3.RS, SH3.6RS, SH4.0RS, SH5.0RS, SH6.0RS, SH5.0RT, SH6.0RT, SH8.0RT, SH10RT, SH5.0RT-20, SH6.0RT-20, SH8.0RT-20, SH10RT-20, SH5.0RT-V112, SH6.0RT-V112, SH8.0RT-V112, SH10RT-V112, SH5.0RT-V122, SH6.0RT-V122, SH8.0RT-V122, SH10RT-V122, SH4.6R
310 stars 80 forks source link

Fix sg_battery_level_nom calculation #103

Closed stephan192 closed 1 year ago

stephan192 commented 1 year ago

In the current implementation some brackets are missing.

Current implementation example: soc_min = 5% soc_max = 95% soc_cur = 75% level_nom = 77% (wrong due to missing brackets)

Fixed implementation: soc_min = 5% soc_max = 95% soc_cur = 75% level_nom = 72,5%

dylan09 commented 1 year ago

I think the calculation without the additional braces is correct. Because you cut your available battery capacity with min=5% and max=95% by 10%. So you have only left 90% usable battery capacity. So the nominal (real) capacity has to be larger than the capacity reported by the inverter (except 100%).

Say you have a battery with 10000 Wh capacity. In your example you could only use 9000 Wh. 75% of 9000 Wh is 6750 Wh. 1000 Wh (reserved by SoC settings) + 6750 Wh is 77,5% nominal remaining capacity. The capacity you could use, when setting min_soc=0% and max_soc=100%.

Any other thougts on this?

stephan192 commented 1 year ago

I don't agree with you. I think you have an small error in your calculation. if soc_min=5% the reserved capacity is 500Wh not 1000Wh. 500Wh + 6750Wh is 7250Wh = 72,5%

stephan192 commented 1 year ago

So the nominal (real) capacity has to be larger than the capacity reported by the inverter (except 100%).

This is not true. For the given example 5% min 95% max the follwing applies. if inverter reports less then 50% the battery reports more than the inverter. If inverter reports 50% the battery also reports 50%. if inverter reports more then 50% the battery reports less than the inverter. It's just linear algebra.

dylan09 commented 1 year ago

You have set soc_min= 5% and soc_max= 95%. So you reserve 10% in my opinion:

But I will check this later when I have more time.

stephan192 commented 1 year ago

In my opinion, i don't use 10% but i just reserve 5% (only soc_min is relevant).

stephan192 commented 1 year ago

Beside this discussion my pull request was in first place originated by Jinja. In Jinja the vertical dash | binds tighter than * or +

The | round(1) applies just to (soc_cur / 100) and not to the whole term as expected. The value in my "wrong example" is 77 = 5 + (95-5) * 0,8

dylan09 commented 1 year ago

The | round(1) applies just to (soc_cur / 100) and not to the whole term as expected.

This bug must have crept in during a change in March. The | round(1) was inserted afterwards. And the brackets were forgotten.

mkaiser commented 1 year ago

thank you! :)

did not think about the operators preferences, when adding the rounding op