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

Proof of concept - Low temperature heating without modifying MHI-AC-Ctrl using only the internal temperature sensor #83

Open
LeonPhilips opened this issue Mar 2, 2024 · 4 comments · May be fixed by #87

Comments

@LeonPhilips
Copy link

Hi Ginkage,

Thank you so much for your hard work making this ESPHome variant of the MHI AC Ctrl. I've combined your code with the latest version of absalom-muc/MHI-AC-Ctrl@c6592d3.

The approach in my fork master...TerminatorNL:MHI-AC-Ctrl-ESPHome-TempRange:master allows reading the internal temperature sensor and feeds that value back into the AC unit as external room temperature. I've had this running for a week now, and experienced no issues.

I decided to not make a PR because my code style might not line up with yours as I'm relatively new to C++ and ESPHome programming. But you are more than welcome to take inspiration (read: copy) from my approach and spread the love.

Here's some of my findings:

  • If you give the AC unit an external room temperature (troom), the next time you read the internal temperature sensor, it will return low values. Speculation: this might indicate a temperature offset compared to troom, instead of the actual room temperature value.
  • If you wait one frame before reading the internal temperature sensor, after giving it an troom of 0xff, you'll get the absolute room temperature reading back.
  • The AC unit does not react instantly to changes in temperature, so I'm letting it read its internal room temperature sensor for one frame and give it back that temperature with an offset in all subsequent ones.

You can see the most important change here: https://github.com/TerminatorNL/MHI-AC-Ctrl-ESPHome-TempRange/blob/f5e0fb8a48fd069097ba1d6a01fcc0edbbac20ac/mhi_ac_ctrl.h#L145-L194

The internal temperature sensor actually updates quite quickly. I'm quite confident we could do away with an external temperature sensor all together if we just add one additional offset as a calibration parameter, but that was out of scope for me for now.

I hope this helps you and others.

@ginkage
Copy link
Owner

ginkage commented Mar 2, 2024

Thank you for the contribution!
Please do file a PR! That way, either me or (more likely) @RobertJansen1 will be able to review and/or suggest changes to the code while preserving the attribution (read: give credit where it's due).
The code style is less of an issue, I don't really mind, as long as it is a meaningful, useful and working change.

@LeonPhilips
Copy link
Author

Thanks for the lovely reply! My free time is currently limited, but I'll pick this up in the future if someone else hasn't already.

@ervee ervee linked a pull request Apr 3, 2024 that will close this issue
@RobertJansen1 RobertJansen1 linked a pull request Apr 5, 2024 that will close this issue
@TGgrimreaper
Copy link

Is there a update to this so we can run this too?

@Dennis-Q
Copy link

Dennis-Q commented Oct 4, 2024

Just my 2 cents on this, but I do wonder if your readings @TerminatorNL are influenced by an offset setting on the AC. More details can be found on this issue.

#65

This can also be found in official documentation of the AC-units, although it's not always described the same way. It seems by default it corrects the temperature with 2 degrees to 'compensate' for additional heat that is caused by the location of the aircondition (high and near ceilings). You can disable this offset and make it more accurate (maybe). I imagine this setting has impact on your readings and the AC-behavior.

Also, I'm not sure if your code changes are a good thing (yet) for people using an external sensor like me. It seems to cause some issues (see my comment in your PR).

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

Successfully merging a pull request may close this issue.

4 participants