Documentation: Status_Vendor

[ChristophCaina]

Hi all, Maybe, the documentation (Wiki) can be include some information about some of the different sensors and what they are for...

for example:

solaredge_status

the possible values for this sensor are: 1, 2, 3, 4, 5, 6, 7, 8

a template sensor can be created to convert the status code into readable text.

    solaredge_status_text:
      friendly_name: "Solaredge status text"
      value_template: >-
        {% if is_state('sensor.solaredge_modbus_status', '1') %}
          off
        {% elif is_state('sensor.solaredge_modbus_status', '2') %}
          sleeping (night mode)
        {% elif is_state('sensor.solaredge_modbus_status', '3') %}
          powering up
        {% elif is_state('sensor.solaredge_modbus_status', '4') %}
          production
        {% elif is_state('sensor.solaredge_modbus_status', '5') %}
          production throttled
        {% elif is_state('sensor.solaredge_modbus_status', '6') %}
          shutting down
        {% elif is_state('sensor.solaredge_modbus_status', '7') %}
          error
        {% elif is_state('sensor.solaredge_modbus_status', '8') %}
          maintenance
        {% else %}
          unknown
        {% endif %}

The same applies for "Status_Vendor" This sensor will show different error-information...

Single Phase Inverter Error Code	Message
31, 33	AC voltage too high
32, 41	AC voltage too low
34	AC freq. too high
35	AC freq. too low
44	No country selected
150,151	Arc fault detected
25	Isolation faults
27,153	Hardware error
17	Temperature to high

Three Phase Inverter Error Code	Message
64,65,66	AC voltage too high
61,62,63,67,68,69	AC voltage too low
79,80,81	AC freq. too high
82,83,84	AC freq. too low
44	No country selected
150,151	Arc fault detected
121	Isolation faults
95,106,120,125,126	Hardware error
104	Temperature to high

[mpredfearn]

Question: Why do we present these sensor values as numbers that have to be interpreted, rather than the sensors having human readable values? When I added the battery status, I made the sensor value the human readable interpretation of the value. Any reason we should not do the same with these inverter statuses, rather than fixing it in the documentation and people having to create their own templated sensors to interpret the values?

[WillCodeForCats]

It's already in there, as sensor.solaredge_i#_status for the code and sensor.solaredge_i#_status_text for the human readable name.

[WillCodeForCats]

Example from mine:

[mpredfearn]

@WillCodeForCats it's not there in upstream - maybe a change in your fork? We add an attribute to the sensor with the human readable name https://github.com/binsentsu/home-assistant-solaredge-modbus/blob/master/custom_components/solaredge_modbus/sensor.py#L193 But the sensor value is just the raw integer value:

[WillCodeForCats]

Look in const.py on line 267

[WillCodeForCats]

Maybe it got broken here when the battery stuff was added. I still see the code in sensor.py here.

The difference betwen my fork is I only have this:

   @property
    def extra_state_attributes(self):
        if self._key in ["status", "statusvendor"]:
            if self.state in DEVICE_STATUSSES:
                return {ATTR_STATUS_DESCRIPTION: DEVICE_STATUSSES[self.state]}
        return None

Upstream version here has all this:

    @property
    def extra_state_attributes(self):
        if self._key in ["status", "statusvendor"]:
            if self.state in DEVICE_STATUSSES:
                return {ATTR_STATUS_DESCRIPTION: DEVICE_STATUSSES[self.state]}
        elif "battery1" in self._key:
            if "battery1_attrs" in self._hub.data:
                return self._hub.data["battery1_attrs"]
        elif "battery2" in self._key:
            if "battery2_attrs" in self._hub.data:
                return self._hub.data["battery2_attrs"]
        return None

[mpredfearn]

Look in const.py on line 267

Yeah the status strings are there - but they are only added as attributes on the sensor - the sensor value is the raw integer

[mpredfearn]

The battery status sensor has the human readable value, if it is understood, otherwise the raw integer: https://github.com/binsentsu/home-assistant-solaredge-modbus/blob/master/custom_components/solaredge_modbus/__init__.py#L837

[mpredfearn]

I was wondering if there was a reason for adding the inverter status as an integer, with the human readable description as an attribute, rather than making the sensor value human readable? @binsentsu ?

[WillCodeForCats]

I have both values: status as number and status as human readable.

Something changed here broke that function or removed it.

[ChristophCaina]

hm... intesting... never checked the sensor attributes, to be honest

status_description: Sleeping (auto-shutdown) – Night mode

does this also apply for the status_vendor? for this one, I could not see any additional attribute

[WillCodeForCats]

[ChristophCaina]

since I am using the latest 1.5.1 release of this addon, I can confirm that the status_text isn't available by the integration itself. Instead, I created a sensor_template for this

Not sure, if a human readable text would be allow a translation - or if it would require another sensor_template where I translate the status_text again. In this case, the integer value would make more sense in my eyes.

[mpredfearn]

I don't see from the code how the human readable sensor is being created...

home-assistant-solaredge-modbus$ grep -r "_text"
home-assistant-solaredge-modbus$ 

Interestingly the multiple inverter patches have all broken adding the human readable text as an attribute on the sensor, because this pattern: if self._key in ["status", "statusvendor"]: does not match - self._key includes the inverter number now - i.e the correct pattern to look for would be i1_status, i2_status etc.

@ChristophCaina The vendor status is not interpreted to an attribute, right now.

[WillCodeForCats]

Yeah I really don't know... this is into "exceeds my python experience" territory again.

My fork is less me developing new things and more me copy/pasting from here and simple changes. Like all the control stuff you did I have no idea how to modify it to work together with PR #12. I really don't even like the Python language to be honest (my go-to languages are C or Perl) so outside of being forced to use it because HA does, I don't have much experience with Python.

[mpredfearn]

Yeah I really don't know... this is into "exceeds my python experience" territory again.

My fork is less me developing new things and more me copy/pasting from here and simple changes. Like all the control stuff you did I have no idea how to modify it to work together with PR #12. I really don't even like the Python language to be honest (my go-to languages are C or Perl) so outside of being forced to use it because HA does, I don't have much experience with Python.

:-) Yeah my primary (what I get paid for) language is C but I do rather like Python too.

@binsentsu I think it's worth thinking about why we present the inverter status sensors as numbers that have to be interpreted, rather than the sensors having human readable values? Is there a good reason, or would it be an idea to change the sensor value to a human readable one? Admittedly changing the sensor could break peoples automations etc which might rely on the current integers - but it seems to me it would be much more friendly to make these human readable.

[WillCodeForCats]

Oh, I think I understand now... extra_state_attributes() is some kind of callback HA is making rather than something called in the integration.

[WillCodeForCats]

So then would simply adding:

self.data[inverter_prefix + "status_text"] = DEVICE_STATUSES[status]

Do the trick?

[mpredfearn]

Oh, I think I understand now... extra_state_attributes() is some kind of callback HA is making rather than something called in the integration.

Yeah exactly

So then would simply adding:

self.data[inverter_prefix + "status_text"] = DEVICE_STATUSES[status]

Do the trick?

You'd want to check that status is a valid key in the dictionary, otherwise you'll get an KeyError Exception, like this:

if status in DEVICE_STATUSES:
    self.data[inverter_prefix + "status_text"] = DEVICE_STATUSES[status]
else:
     self.data[inverter_prefix + "status_text"] = status

That wouldn't by itself create the sensor though, you'd need to define the additional sensor that looked for the inverter_prefix + "status_text" key in self.data for it's value.

[mpredfearn]

Not sure, if a human readable text would be allow a translation - or if it would require another sensor_template where I translate the status_text again. In this case, the integer value would make more sense in my eyes.

Ah interesting - yeah probably the correct way to do this would be to provide translations for the states as documented by HA https://developers.home-assistant.io/docs/internationalization/core/... In which case the sensor value (I think) can remain an integer, and the translation file would provide the localised human readable version.

[ChristophCaina]

in addition to this: is state_class: measurement the correct state class for the status & status_vendor sensors?

In fact, they aren't any measurement devices...

[WillCodeForCats]

Okay sorry for the confusion on my end. Updated my fork now that I understand a little more about HA integrations.

I don't think a human readable sensor needs any state class, does it?

[mpredfearn]

This is a pretty kludgy hack, just to see if I could get it to work, but this seems to add a translated version of the human readable status to the sensor

commit e99c90e53d967113abb1077666f0ceaa0a668994 (HEAD -> master)
Author: Matt Redfearn <matt.redfearn@gmail.com>
Date:   Wed Oct 20 20:28:56 2021 +0100

    Add WIP sensor translation

diff --git a/custom_components/solaredge_modbus/sensor.py b/custom_components/solaredge_modbus/sensor.py
index d1b97bad62b..d326a41a23a 100644
--- a/custom_components/solaredge_modbus/sensor.py
+++ b/custom_components/solaredge_modbus/sensor.py
@@ -143,6 +143,9 @@ class SolarEdgeSensor(SensorEntity):
         self._icon = icon
         self._device_info = device_info
         self._attr_state_class = STATE_CLASS_MEASUREMENT
+
+        if key in ["i1_status", "i2_status", "i3_status"]:
+            self._attr_device_class = "solaredge_modbus__status"
         if self._unit_of_measurement == ENERGY_KILO_WATT_HOUR:
             self._attr_state_class = STATE_CLASS_TOTAL_INCREASING
             self._attr_device_class = DEVICE_CLASS_ENERGY
diff --git a/custom_components/solaredge_modbus/translations/sensor.en.json b/custom_components/solaredge_modbus/translations/sensor.en.json
new file mode 100644
index 00000000000..708f7e8de87
--- /dev/null
+++ b/custom_components/solaredge_modbus/translations/sensor.en.json
@@ -0,0 +1,15 @@
+{
+  "state": {
+    "solaredge_modbus__status": {
+      "1": "Off",
+      "2": "Sleeping (Night Mode)",
+      "3": "Powering Up",
+      "4": "Production",
+      "5": "Production Throttled",
+      "6": "Shutting Down",
+      "7": "Error",
+      "8": "Maintenance",
+      "_": "Unknown"
+    }
+  }
+}

[WillCodeForCats]

Okay I see where I was going wrong... I had added status_text in my local version that I never uploaded to my fork on github. I also added the if/else statement suggested by @mpredfearn

[WillCodeForCats]

My fault for being stupid. sorry everyone.

[WillCodeForCats]

So now after making sure what I have in my fork matches what I have running in my real HA install, this the result:

I quickly added the status_vendor_text suggestion too because I learned something today. I also dumped the extra_state_attributes() function. So now at this point my fork will have original stuff in it instead of just copy/pasting from upstream.

[ChristophCaina]

here's the "official" translation of the status provided from SolarEdge:

[WillCodeForCats]

Yes, I'm familiar with the spec doc, but some of those descriptions are just too long (my opinion) and don't show up nicely in my dashboard. So I changed them to something shorter that has the same meaning.

"Production" means the same thing as "Inverter is ON and producing power" but with one word instead of 6 words. And besides, it says "Production (curtailed)" for state 5 (inverter clipping) which is a modifier on state 4 so making the two production states "Production" and "Production (Curtailed)" ties them together nicely.

Same with just saying "Sleeping" as one word instead of all the extra words that mean effectively the same thing.

I took out "wake-up" from 3 because I've never seen my inverters in that state. They go straight from 2 to 4 at start of production. The only time they go to state 3 is after a grid event during the monitoring wait time to decide to go back into production.

If a different language translation needs the extra words that's OK to translate them more literally. Or create another sensor for "long literal text from the PDF" and the short version. But I like the short versions I'm using better for English.

DEVICE_STATUSES = {
    1: "Off",
    2: "Sleeping (Auto-Shutdown)",
    3: "Grid Monitoring",
    4: "Production",
    5: "Production (Curtailed)",
    6: "Shutting Down",
    7: "Fault",
    8: "Maintenance",
}

[ChristophCaina]

I know :) just added this for documentation purpose, if the inverter_status_text will be implemented as readable text in the translation, I would suggest to use the official wording by default.

If required, it can still be changed with another template or something like that.

[WillCodeForCats]

The document says that column is titled "description". In English "description" just means a representation, like an explanation to clarify what something means. I would say the official wording is actually the "parameter" column.

[WillCodeForCats]

I would suggest this then if official wording is preferred:

DEVICE_STATUSES = {
    1: "I_STATUS_OFF",
    2: "I_STATUS_SLEEPING",
    3: "I_STATUS_STARTING",
    4: "I_STATUS_MPPT",
    5: "I_STATUS_THROTTLED",
    6: "I_STATUS_SHUTTING_DOWN",
    7: "I_STATUS_FAULT",
    8: "I_STATUS_STANDBY",
}

Then localize that with language translations.

[binsentsu]

Yeah I really don't know... this is into "exceeds my python experience" territory again. My fork is less me developing new things and more me copy/pasting from here and simple changes. Like all the control stuff you did I have no idea how to modify it to work together with PR #12. I really don't even like the Python language to be honest (my go-to languages are C or Perl) so outside of being forced to use it because HA does, I don't have much experience with Python.

:-) Yeah my primary (what I get paid for) language is C but I do rather like Python too.

@binsentsu I think it's worth thinking about why we present the inverter status sensors as numbers that have to be interpreted, rather than the sensors having human readable values? Is there a good reason, or would it be an idea to change the sensor value to a human readable one? Admittedly changing the sensor could break peoples automations etc which might rely on the current integers - but it seems to me it would be much more friendly to make these human readable.

@mpredfearn Reason for numeric was for straightforward automations purposes. The textual values are in my opinion too verbose for usage in automations. eg. 'Sleeping (auto-shutdown) – Night mode' . Changing this would indeed imply a breaking change.

[ChristophCaina]

hm... ok, I don't think, that we should change the status sensor. the numeric value is ok for automatisations - and since the status text is available as attribute, it can easily be used ( i wasn't aware of that fact)...

If you want to translate the status text - or use your own one, it can be done by a template sensor... that's fine.

But I would appreciate, to get the status_vendor text somehow - at least, as an attribute, too. Yesterday, I've seen that status vendor switched to "86" for a short period of time - which isn't documented in the list above... therefore, I am trying to get some more information that might help out :)

[ChristophCaina]

so, comming back to this after a year... :)

I am trying to implement the status_vendor text as an attribute to the status_vendor sensor (just as it has been done for the status itself)...

I am not very deeply into the code of this integration, but I think, that's the area where the change needs to be done:

sensor.py

    @property
    def extra_state_attributes(self):
        if self._key in ["status", "statusvendor"]:
            if self.state in DEVICE_STATUSSES:
                return {ATTR_STATUS_DESCRIPTION: DEVICE_STATUSSES[self.state]}
        elif "battery1" in self._key:
            if "battery1_attrs" in self._hub.data:
                return self._hub.data["battery1_attrs"]
        elif "battery2" in self._key:
            if "battery2_attrs" in self._hub.data:
                return self._hub.data["battery2_attrs"]
        return None

in the init.py there is the following:

            status = decoder.decode_16bit_int()
            self.data["status"] = status
            statusvendor = decoder.decode_16bit_int()
            self.data["statusvendor"] = statusvendor

const.py

DEVICE_STATUSSES = {
    1: "Off",
    2: "Sleeping (auto-shutdown) – Night mode",
    3: "Grid Monitoring/wake-up",
    4: "Inverter is ON and producing power",
    5: "Production (curtailed)",
    6: "Shutting down",
    7: "Fault",
    8: "Maintenance/setup",
}

I guess, I need to create a "VENDOR_STATUS" in the const.py and extend the if/else in the sensor.py into something like this:

const.py

VENDOR_STATUSES = {
    0:  "No Error",
    17: "Temperature too high",
    25: "Isolation faults",
    27: "Hardware error",
    31: "AC voltage too high",
    32: "AC voltage too low",
    33: "AC voltage too high",
    34: "AC freq. too high",
    35: "AC freq. too low",
    41: "AC voltage too low",
    44: "No country selected",
    61: "AC voltage too low",
    62: "AC voltage too low",
    63: "AC voltage too low",
    64: "AC voltage too high",
    65: "AC voltage too high",
    66: "AC voltage too high",
    67: "AC voltage too low",
    68: "AC voltage too low",
    69: "AC voltage too low",
    79: "AC freq. too high",
    80: "AC freq. too high",
    81: "AC freq. too high",
    82: "AC freq. too low",
    83: "AC freq. too low",
    84: "AC freq. too low",
    95: "Hardware error",
    104: "Temperature too high",
    106: "Hardware error",
}

sensor.py

def extra_state_attributes(self):
        if self._key in ["status"]:
            if self.state in DEVICE_STATUSSES:
                return {ATTR_STATUS_DESCRIPTION: DEVICE_STATUSSES[self.state]}
        elif self._key in ["statusvendor"]:
            if self.statusvendor in VENDOR_STATUSSES:
                return {ATTR_STATUS_DESCRIPTION: VENDOR_STATUSSES[self.statusvendor]}
        elif "battery1" in self._key:
            if "battery1_attrs" in self._hub.data:
                return self._hub.data["battery1_attrs"]
        elif "battery2" in self._key:
            if "battery2_attrs" in self._hub.data:
                return self._hub.data["battery2_attrs"]
        return None

But atm I am not sure with the IF ELSE structure in the sensor.py - so maybe, someone can confirm or provide more information on how I need to change that?