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

Terminator nl master #87

Open
wants to merge 25 commits into
base: master
Choose a base branch
from

Conversation

RobertJansen1
Copy link
Collaborator

Needs some testing, especially on the api room temp part and the offset / low temperature stuff from @Terminator-NL

@ervee
Copy link

ervee commented Apr 3, 2024

Thanks for the continuing effort! I could have missed the context for a lot of these changes, but:

  • The external room temperature sensor capability is removed in favor of a user configurable temperature offset or simulated room temperature?
  • We are changing the .h and .cpp files now instead of copying them as-is from the MHI-AC-Ctrl project? Can we still easily pull changes from the MHI-AC-Ctrl project?
  • What is the low temperature heating?

If I know what to look for I can test these changes and provide feedback here.

Edit: I found #83 :)

@RobertJansen1
Copy link
Collaborator Author

Hi @ervee thanks for replying and wanting to help.

the changes are indeed for issue #83 and the nice work of @TerminatorNL

  • the external room temperature sensor capability was removed by @TerminatorNL but i readded it, so it should have both. as i don't use both yet, it is not easy for me to test this through.
  • the files MHI-AC-Ctrl-core.cpp and MHI-AC-Ctrl-core.h are fetched from the MHI-AC-CTRl, and the changes here are cosmetic changes that came from "upstream". the file MHI-AC-Ctrl.h is "ours" and contains all logic, and is the part that was extended
  • The units normaly don't allow for heating under 18 degrees celsius, or cooling above 30 degrees i think, this should allow for heating to for example 16 degrees

in future, we can extend it further to allow half degree setting of temperature (which currently is ignored)

@RobertJansen1 RobertJansen1 linked an issue Apr 5, 2024 that may be closed by this pull request
@RobertJansen1 RobertJansen1 linked an issue Apr 5, 2024 that may be closed by this pull request
@Dennis-Q
Copy link

Dennis-Q commented Oct 4, 2024

So I have been using this build for a while, but it seems to influence the set_api_room_temperature option since temperature wasn't stable. Reverted back to the original code and all is working fine again.

With my earlier tests during summer, it looked ok (testing for short period), but I didn't actually use it for heating until a few days ago and it seems strange things happen (temperature is not stable and I see the temperature of the climate entity changing to high numbers (e.g. 27 degrees) sometimes when in reality it was around 21... (as given through the API).

Since this build, I haven't changed any setup on my side (except that I actually started heating again), so I think it really an issue with this code change. Flashing the ESP with the latest code of the master branch resolved the issue, but maybe others with external sensors can test too...

I imagine this implementation might be an improvement over the default's behaviour, but I think the external temperature sensor is superior and simple to implement. Temperature in multiple rooms with this setup is rock solid, I just think not many people actually use it :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants