wills106 / homeassistant-solax-modbus

SolaX Power Modbus custom_component for Home Assistant (Supports some Ginlong Solis, Growatt, Sofar Solar, TIGO TSI & Qcells Q.Volt Hyb)
296 stars 88 forks source link

Some X3 Gen4 hybrids does not show export_control_user_limit correctly #58

Closed infradom closed 2 years ago

infradom commented 2 years ago

This issue was reported by @mudshurko in the HA community SolaX app and device display shows value e.g. 9000 In the solaxcloud webpage, it shows 900 Our integration in HA shows also 900 image

Serial number prefix: H34T10H Device versions:

Firmware Version Inverter Master: 15
Firmware Version Manager: 14
Firmware Version Modbus TCP Major: 0
Firmware Version Modbus TCP Minor: 6
ARM: 2.03
DSP: 2.07

Other info from the thread:

tried now to set 9000 (developer tools) then I get in the inverter 90.000 image

As the cloud webpage suffers from the same problem, it is unsure if the error is in our code, but we fear that other users may encounter the same problem

wills106 commented 2 years ago

I don't this this is just Gen4 limited. I think there might be a Gen3 X3 doing the same #11 or at least similar.

We might need to add the ability to "Configure" the Integration once installed to allow users to change the scaling if they are getting incorrect figures. Seems odd how it only affects "export_control_user_limit" though!

https://developers.home-assistant.io/docs/config_entries_options_flow_handler

wills106 commented 2 years ago

I think a good example of where an Integration needs to reconfigure itself is the Solcast Integration that I use.

https://github.com/oziee/ha-solcast-solar

Some users were having an issue where there Solar forecast was 4 or 6 hours out from the displayed graph. So a TZ offset was introduced. Where you could click the "Configure" option on the main Integration page and change the offset.

We could possibly use the same approach to add a user multiplier (10 for example) to "export_control_user_limit" so it read correctly.

infradom commented 2 years ago

OK, thanks, I was not aware that the GEN3 versions also behave differently. If you want, I can try to implement such a feature, but it wont be for this week ..

wills106 commented 2 years ago

It only seems to be these two Inverters at the moment, unless no one has looked? I guess it's not a value people would change very often if at all.

I'm going to work on removing all the unused Tick Boxes first and then I can have a look at adding in an adjustable configuration for the offset.

infradom commented 2 years ago

Today, I realize that my Gen4 inverter also suffers from this export_control_user_limit issue. I was convinced that this was not the case earlier this year, but I might be wrong. As I moved to a day-ahead-pricing model, injection prices can be negative, so I need to be able to modify the export_control_user limit in an automation, but it will either be zero or some high value.

Looking at our code, It seems like this number selector is only available in the integration for Gen4, the parameter seems to exist on other generations also. Is my perception wrong ?

   SolaXModbusSensorEntityDescription(
        name="Export Control User Limit",
        key="export_control_user_limit",
        native_unit_of_measurement=POWER_WATT,
        entity_registry_enabled_default=False,
        allowedtypes=ALLDEFAULT,
    ),
    SolaxModbusNumberEntityDescription( name = "Export Control User Limit",
        key = "export_control_user_limit", 
        register = 0x42,
        fmt = "i",
        min_value = 0,
        max_value = 60000,
        step = 500,
        unit_of_measurement = POWER_WATT,
        allowedtypes = GEN4,
    ),
wills106 commented 2 years ago

The Gen2 & Gen3 also supports this. There has been a request to add this, but I haven't got round to it yet.

https://community.home-assistant.io/t/solax-inverter-by-modbus-no-pocket-wifi-now-a-custom-component/140143/566?u=wills106

infradom commented 2 years ago

Ok, I have issued a pull request that resolves issue #58 . My fork also contains the untested support for gen3/2

infradom commented 2 years ago

I will issue yet another PR for supporting gen3 and gen2 (and fix an error in previous PR)

wills106 commented 2 years ago

Is this now resolved? Just want to check if I can close the issue down.

infradom commented 2 years ago

On my Gen4, the issue has disappeared (or has been solved by our previous changes). I also recently allowed my Solax installer to perform a sofware upgrade, I think the problem was solved already before that upgrade.

wills106 commented 2 years ago

Ok, I'll close it down then.