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

State Code add? #358

Open
RafAustralia opened this issue Oct 1, 2024 · 10 comments
Open

State Code add? #358

RafAustralia opened this issue Oct 1, 2024 · 10 comments

Comments

@RafAustralia
Copy link
Contributor

As per inverter states method using code line checking inverter state and removing the need for (default=0), should there be a line of code added to here as well to perform same function?

@Gnarfoz
Copy link
Contributor

Gnarfoz commented Oct 1, 2024

You mean to amend the "unknown" state so it outputs the actual value so it can be reported by someone who encounters it?
That seems like a good idea to me.

@RafAustralia
Copy link
Contributor Author

what I mean... is this:

(default=0) was flooding the system logs ok - I get it, and I understand need to apply the status checker - works fine for me.
but beyond the inverter status and device type checker:

       Unknown - should not see me! {{ (states('sensor.system_state') |int) }}
        Unknown device code: {{ '%0x' % (states('sensor.sungrow_device_type_code') |int)  }}

There are few other places which have the same situation:
these are:

  - name: Battery forced charge discharge cmd
    unique_id: sg_battery_forced_charge_discharge_cmd
    availability: "{{ not is_state('sensor.battery_forced_charge_discharge_cmd_raw', 'unavailable') }}"
    # TODO: test state_class with enum
    # state_class: measurement
    device_class: enum
    state: >-
      {% if ((states('sensor.battery_forced_charge_discharge_cmd_raw') |int) == 0x00AA) %}
        Forced charge
      {% elif ((states('sensor.battery_forced_charge_discharge_cmd_raw') |int)  == 0x00BB) %}
        Forced discharge
      {% elif ((states('sensor.battery_forced_charge_discharge_cmd_raw') |int)  == 0x00CC) %}
        Stop (default)
      {% else %}
        Unknown - should not see me!
      {% endif %}

  - name: Export power limit mode
    unique_id: sg_export_power_limit_mode
    availability: "{{ not is_state('sensor.export_power_limit_mode_raw', 'unavailable') }}"
    # TODO: test state_class with enum
    # state_class: measurement
    device_class: enum
    state: >-
      {% if ((states('sensor.export_power_limit_mode_raw') |int) == 0x00AA) %}
        Enabled
      {% elif ((states('sensor.export_power_limit_mode_raw') |int)  == 0x0055) %}
        Disabled
      {% else %}
        Unknown - should not see me!
      {% endif %}

  - name: EMS mode selection
    unique_id: sg_ems_mode_selection
    availability: "{{ not is_state('sensor.ems_mode_selection_raw', 'unavailable') }}"
    # TODO: test state_class with enum with enum
    # state_class: measurement
    device_class: enum
    state: >-
      {% if ((states('sensor.ems_mode_selection_raw') |int) == 0) %}
        Self-consumption mode (default)
      {% elif ((states('sensor.ems_mode_selection_raw') |int) == 2) %}
        Forced mode
      {% elif ((states('sensor.ems_mode_selection_raw') |int) == 3) %}
        External EMS
      {% elif ((states('sensor.ems_mode_selection_raw') |int) == 4) %}
        VPP
      {% elif ((states('sensor.ems_mode_selection_raw') |int) == 8) %}
        MicroGrid
      {% else %}
        Unknown - should not see me!
      {% endif %}

My logic was: since (default=0) is removed then a checker should be placed to check what is the status of these.
That's all.

@mkaiser
Copy link
Owner

mkaiser commented Oct 2, 2024

you are right - printing the actual value (that could not be parsed / value causing the step into the else path) would improve the code / error handling.

Do you wan't to make a PR for the 3 sensors above?

@RafAustralia
Copy link
Contributor Author

RafAustralia commented Oct 2, 2024

Good evening mkaiser... I do love a challenge - might take me a short while but happy to try...

right now desperately trying to sort out why export slider is acting weird, not just for me but for few others... it is my mission to get curtailment fully operational, which is not happening yet, not quite as I would like...

and I would love it renamed to something more clear, as many assume it was a curtailment tool... e.g.

Production limit

@Gnarfoz
Copy link
Contributor

Gnarfoz commented Oct 2, 2024

As the sensors say, the limit is currently disabled. You have to enable it. Why is yours a sensor? The default dashboard includes a dropdown there to enable / disable it.
https://github.com/mkaiser/Sungrow-SHx-Inverter-Modbus-Home-Assistant/blob/main/dashboards/_DefaultDashboard_mkaiser/dashboard.yaml#L274

@RafAustralia
Copy link
Contributor Author

RafAustralia commented Oct 2, 2024

As the sensors say, the limit is currently disabled. You have to enable it. Why is yours a sensor? The default dashboard includes a dropdown there to enable / disable it. https://github.com/mkaiser/Sungrow-SHx-Inverter-Modbus-Home-Assistant/blob/main/dashboards/_DefaultDashboard_mkaiser/dashboard.yaml#L274

Hey - at some point I will create a separate issue about this, but...

I do have full sliders and dropdowns... (you will just have to trust me its quite an advanced setup)
Example below:
(ignore unknown state - that is solved with your new inverter states added, which I did not have at the time)

FULL SLIDERS PRODUCTION

(I have quite a few different dashboards), but the point being, these sliders are adjusting how much inverter produces... so even if it had 20kw coming from the roof... if you slide it to 4kw, then inverter will output 4kw only.

Firstly, many people think it is export to grid - it is NOT. just cuts inverter output, regardless where power goes.

Second issue is - it does not function properly under different modes, I have been testing it extensively with @purcell-lab and we observed a strange behaviour in that slider - it is chaos. Sometimes it just cuts inverter down to ZERO, without any way to stop it... it works in some modes but not the others...

so as part of my work on curtailment for Sungrow inverters - making this slider work properly is quite critical, and I am in process of trying to determine - is it the code or is it Sungrow functionality issue

se below examples of what I am talking about
Please note - control is first DISABLED and it works, slider open (30000)

PRODUCTION CONTROL DISABLED

then ENABLED for first inverter but left open (30000)... so inverter should still produce... but first inverter dies off,

PRODUCTION CONTROL ENABLED DISABLED

and then both ENABLED, and both die... slider open (30000)

PRODUCTION CONTROL ENABLED ENABLED

This should not be the case.
Conclusion:
Enabling this feature should not cut production - the slider adjustment should.

Keen to hear your comments on this issue... Im sure you will know what the issue may be -:)

@Gnarfoz
Copy link
Contributor

Gnarfoz commented Oct 3, 2024

One thing stands out:
The value at the slider labeled "Set export power limit" (the input value) and the value labeled "Export power limit" (the readout value) should match. They don't. That's not correct and it explains why the inverter "dies off". The power limit is set to 0 W, you enable the power limit, and the inverter obeys.

Do note that this whole project only reads and writes simple values from and to the inverters. If a register you write to does not do what it's expected to do, that can only be the inverter's fault.
Except: if it's not the correct register. ;-)
The SHxT series is also still new-ish and has various known firmware issues. Perhaps this is among them.

Also, multi-inverter setups are far less tested. Out of the box, only entities for a single inverter are available. So you probably had to do all the duplication work by hand. Perhaps something slipped through and was not duplicated correctly?

Do you even need to set an export limit on both inverters if they're in a primary/secondary setup?
iSolarCloud has these helpful tooltips that seem to say "enable the power limit on both, but only set a value on the host, and make sure you set it as both an actual value and a percentage"?!
image
image

Lastly, at least on the SHxRT series, there are two power limits. A static one and a dynamic one. Only the dynamic one is being manipulated here. Not sure if the SHxT series also has this. The static one would need to be set up via iSolarCloud.
I think it's documented as "feed-in limitation - wide range", register 31222 (address 31221).
image

@RafAustralia
Copy link
Contributor Author

RafAustralia commented Oct 3, 2024

@Gnarfoz

Hey.

so point by point... there is a lot to cover.

YES AGREED - they should match... slider goes down and the figure should follow.
It works in some modes not others.
EG. Forced charging YES, but self-consume NO, forced discharge NO.
And it produces interesting issue also.
Its behaviour makes no logical sense...
the moment it is applied the inverter stops charging battery - it goes to grid and house only...
I think you should see it in action and happy to show you - you could see it live... it has some problems - I have not yet found where... Mark Purcell and I have been looking for ages and hassling sungrow also...

As for Sungrow limitations - these do not work for me cos I am in open loop pass through mode... so ignoring EMS from logger.

Work in progress.

@korttoma
Copy link

korttoma commented Oct 3, 2024

Hi All, not sure if it has anything to do with your challenges as my system is completely different but I observed this related to the export power limit in my system -> #302

@RafAustralia
Copy link
Contributor Author

Hi All, not sure if it has anything to do with your challenges as my system is completely different but I observed this related to the export power limit in my system -> #302

Hey - it is always good to know what other issues there may be - I am not sure if these are related but what I am saying this feature needs work as it is currently not doing what it is meant to do. Period.
In saying that - I have failed to solve it yet but I will get there eventually.
I think this feature needs more detailed variables to account for whole lot more scenarios / running modes.
Will hopefully help all others.

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

No branches or pull requests

4 participants