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

Future recommendations / wishes #38

Open
foltymat opened this issue Nov 4, 2022 · 1 comment
Open

Future recommendations / wishes #38

foltymat opened this issue Nov 4, 2022 · 1 comment
Labels
enhancement New feature or request

Comments

@foltymat
Copy link

foltymat commented Nov 4, 2022

Hi @rsciriano,

I wanted to take time to write down the couple things I would like to see improved in your new external component. I love how you are implementing this, but there are couple of quality of life improvements that I think would help a lot.

1. PID calculations only on temperature change

In my eyes, this should be happening either on a state change of the PID controller, or on a set loop (i.e. every 10 seconds).
This is mainly due to the fact that I'm using Zigbee temperature sensors around the house and they are very slow to update.
And can stay on a same level for a long time. This way, when you start feeling chilly, you have to wait for the change to happen.

2. Interlocking that PID controller and Heating water elements.

I know you've already talked about is in issue #27 but I would like to emphasise that.
Letting the user be responsible for turning on/off the heating element is not really a good idea (even though it can be solved by a simple automation.
What I would like to say is keeping the water flowing through the radiators even after the flame is off is a GOOD IDEA.
Yes, the boiler is used to these temperatures, but it's so much better for the chamber to not keep heating up from that residual heat. If you can circulate the water for let's say 10 minutes, that's a win in my book!

3. Non-valid states reported as -1

The main reason being, -1 is a valid state Home Assistant understands. That means you get these spikes in the graph (shown below).
I tried increasing the polling time in component.h file, which helped but only marginally.
HA 2022.11 introduces a very simple Statistics sensor, which can easily give you a min/max values for a given day.
That would be perfect, but my minimum is now -1 because of that :)
Not really sure how best to implement this, but the best way in my opinion would be to have the sensor go Unavailable in HA.
That way, you would just get a gap in the data, rather than a wrong value.

image

4. Prepare for HA statistics on the external temperature sensor

This is a "problem" I don't even consider a problem but ... Home Assistant has a great databse for long-term statistics, which your sketch does not utilize, as you don't have a state_class and device_class defined. It is VERY easy to add by anyone but I did have to search for the solution as it was not obvious why there are no statistics for the sensor :) Here's how that looks:

    - name: "External Temperature"
      unit_of_measurement: °C
      accuracy_decimals: 1
      state_class: measurement
      device_class: temperature

I'll update the the thread if I come up with something else.

@rsciriano rsciriano added the enhancement New feature or request label Dec 16, 2022
@Chuckame
Copy link

Just one thing I'm thinking about, but instead of reporting as unavailable or -1, maybe it's better to just wait for the next valid poll before send the value, with like a retry mechanism, and when it's over, the sensor is reported as unavailable ?

I'm too new to esphome libraries, so I don't know if it's do-able! But for sure I'll contribute since I just received my own diyless master bridge.. And also a developer !

ShurikenGitHub added a commit to ShurikenGitHub/ESPHome-OpenTherm that referenced this issue Feb 12, 2023
…iriano#38 (point nr 3) and rsciriano#43

Values used in the code are -1, if you use NAN home assistant will understand it's a wrong value
rsciriano pushed a commit that referenced this issue Oct 21, 2023
 (#52)

Values used in the code are -1, if you use NAN home assistant will understand it's a wrong value

Probably fixing issues: #17, #25, #38 (point nr 3) and #43 (#52)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants