ksheumaker / homeassistant-apsystems_ecur

Home Assistant custom component for local querying of APSystems ECU-R Solar System
Apache License 2.0
166 stars 42 forks source link

Temperature "-100 °C" #86

Closed dewi-ny-je closed 1 year ago

dewi-ny-je commented 1 year ago

In https://github.com/ksheumaker/homeassistant-apsystems_ecur/blob/69b808cb2997c68a3c395c9fb580c6f16f9e27be/custom_components/apsystems_ecur/APSystemsSocket.py#L322

the temperature is calculated as offset by 100, since the ECU sends data with 0 = -100 °C up to 255 = 155 °C.

As result, when no data is available because the panel is offline, the HA and Grafana graphs are nonsense:

image

The component should filter out values of "-100 °C", so that HA and Grafana will omit the measurement point, which is what is right, since... the panel is offline! why a fake value?

I'm not sure how to replace a number by "unavailable" or how to avoid adding that measurement point altogether. Maybe replacing:

inv["temperature"] = self.aps_int(data, location) - 100

by:

if self.aps_int(data, location) > 0:
  inv["temperature"] = self.aps_int(data, location) - 100

but I don't know python, I just looked up "python if then else".

HAEdwin commented 1 year ago

I currently use https://github.com/HAEdwin/homeassistant-apsystems_ecur/blob/main/custom_components/apsystems_ecur/APSystemsSocket.py and get zero values when the inverter is offline. This keeps the temperature graph somewhat inbound. image You can imagine that replacing a nummeric value for a text value for graphs won't work.

Edit: I was looking to the graph based on the Gauge which has a minimum value defined hence the zero values.

dewi-ny-je commented 1 year ago

No it wouldn't work but then the value should be omitted entirely, since that's what actually happens: the panels are offline and no value is reported by the inverters (which are powered by the panels).

In winter 0°C is a perfectly reasonable value for daily operation of the panels.

HAEdwin commented 1 year ago

Would you rather have it produce the graph like the red line is showing? image I think, if the value is ommited entirely, the sensor produces an invalid data statement from within Home Assistant since the value is not being updated anymore. Option is to keep the latest value and jump to the new value when the inverter goes online but I don't think that is a neat solution. Just go to zero like all values is the best solution imho.

dewi-ny-je commented 1 year ago

I'm not at home but I can tell you that when data is missing home assistant simply omits the line, it doesn't connect the latest value with the newest one. It may also depend whether the previous value has an expiration or is set to be maintained until a new one arrives?

12christiaan commented 1 year ago

I confirm the same problem. during night the temperture drops to -100. this should not happen.

dewi-ny-je commented 1 year ago

Screenshot_20220810-080950_Home Assistant

This sensor has connection issues (TCP modbus) and clearly has holes in the graph where data is missing or errors are returned.

I know from ESPhome that sensors can be made to expire: https://esphome.io/components/sensor/index.html

But I searched the HA docs and I found nothing.

dewi-ny-je commented 1 year ago

https://community.home-assistant.io/t/is-it-possible-to-mark-a-sensor-as-unavailable/448865/3

How to make the sensor "unavailable" is beyond my capabilities, but it is possible and it is IMHO the correct choice whenever theECU provides "0" as temperature.

HAEdwin commented 1 year ago

The inverter temperature sensor is made unavailable by making it None if the inverter is offline. This results in a non-numeric value for users who use gauges to display the inverter temperature (like I do). image To bypass the non-numeric state you have to create a value template in configuration.yaml like this example:

sensor:
  - platform: template
    sensors:
      temperature_non_numeric_806000111111:
        value_template: "{{ states('sensor.inverter_806000111111_temperature')|float(0) }}"
        unit_of_measurement: "°C"

The gauge on this sensor now shows 0 when the inverter is offline and state is non-numeric image

None represents a non-fictitious value and does eventually create gaps in the graph but that is only the case when state becomes a value again. Meanwhile a gap is shown in the graph. 0 in the gauge represents a fictitious temperature.

HAEdwin commented 1 year ago

@dewi-ny-je and @12christiaan: Did the update https://github.com/HAEdwin/homeassistant-apsystems_ecur/commit/a8867f2787680f24880f04d24049833a6a152d76 solve your issue?

12christiaan commented 1 year ago

No, the update did not solve the problem. i think the unit is still online, while the value is already dropping to -100

HAEdwin commented 1 year ago

Noticed the issue due to unexpected behavior :) will look into it.

12christiaan commented 1 year ago

perhaps check if "frequency" > 0 than update "temperature" else "temperature" = none?

HAEdwin commented 1 year ago

creative idea, I'm going to find out why the online parameter is apparently not correct.

12christiaan commented 1 year ago

I like to help, but am new to HASS. i have started a TCPdump, so we will see tonight what happens on the communication flow. at least we can see what the system send, etc.

HAEdwin commented 1 year ago

Error was in the boolean evaluation, data was right. Solved it and evaluating... https://github.com/HAEdwin/homeassistant-apsystems_ecur/commit/35b9372df6f7b5bdf73a802801a72f986cb6abc4

12christiaan commented 1 year ago

i apply the change to my local script. let's see tonight what happens

HAEdwin commented 1 year ago

As expected, the "None" value produces a flat line until the value is no longer "None" and will then create a gap in the graph data. Here the temperature graph remains at 24 degrees Celsius and after the inverter went online it leaves a gap. image image

dewi-ny-je commented 1 year ago

When saving data to an external database such as Influxdb having a gap may be the intended behaviour. Using 0 as gap filling may well mean that in winter there is overlap between fake and valid data, in some locations (corner case I would say... it's uncommon). I would prefer the flat line which becomes a gap as soon as a new valid datum arrives,but other users may have other priorities.

HAEdwin commented 1 year ago

I get it but this does leave a challenge for those who use a gauge to indicate the temperature. To accommodate non-numeric values ​​in the gauge, None must be replaced by 0.

I'm struggling with a template myself but can't figure it out. value_template: "{{ 0 if states('sensor.inverter_806000011111_temperature') == none }} does not work

So I asked the community for help. It's a jinja thing :)

dewi-ny-je commented 1 year ago

Can you have an option in the settings? Besides IP address and polling time.

"When offline, report inverter temperature as: (leave blank for not reporting values, which may cause troubles with gauges)" Then a field with 0 as default.

HAEdwin commented 1 year ago

I also thought of that solution, but it can also be concluded with a clear comment that a value template must be used (https://github.com/ksheumaker/homeassistant-apsystems_ecur/issues/86#issuecomment-1213542652). I was already given a solution that works. As a result, the gauge then represents a fictitious temperature so it's better to keep the default None. I modified it in the repository. https://github.com/ksheumaker/homeassistant-apsystems_ecur/pull/84/commits/17d6fc086142d323f93da2079f8f9018d2bf038e

HAEdwin commented 1 year ago

Guess this issue can be closed? I asked @ksheumaker if he got some spare time left to do a merge of the pull requests so that we're more up to date.

12christiaan commented 1 year ago

i confirm the 0 is fine for me (100 times better than -100 :-) ) also here the change is working fine.