search

absalom-muc / MHI-AC-Ctrl

Reads and writes data (e.g. power, mode, fan status etc.) from/to a Mitsubishi Heavy Industries (MHI) air conditioner (AC) via SPI controlled by MQTT
MIT License
270 stars 61 forks source link

Added enhancing Tsetpoint resolution when using .5 degrees #123

Closed glsf91 closed 1 year ago

glsf91 commented 1 year ago

Added #define ENHANCED_RESOLUTION to enhance the resolution of Tsetpoint when using .5 degrees. To Troom an offset will be added for compensating a higher setpoint if needed. Basically the Troom send to the airco will be increased with an offset (0.5 degrees) when a setpoint x.5 degrees is used, to compensate for the setpoint +1 degrees. Based on the discussion in issue #81

Also added #define DISABLE_FILTER_TROOM to show also Troom delta > 0.0 instead of 0.25 degrees. Based on the discussion in issue #82

glsf91 commented 1 year ago

I did not adjust the documentation (except adding the new topic) because I don't know how you want this be documented.

glsf91 commented 1 year ago

Maybe also add to the documentation that this feature is not possible when using the internal room sensor of the airco.

absalom-muc commented 1 year ago

Thank you @glsf91 , some questions:

glsf91 commented 1 year ago

@absalom-muc

  • Why have you added the line //#define DISABLE_FILTER_TROOM true to MHI-AC-Ctrl-core.h and not to support.h? Not sure if it is a good idea to have configuration on different places.

I have added //#define DISABLE_FILTER_TROOM true to MHI-AC-Ctrl-core.h instead of support.h because support.h is not included in MHI-AC-Ctrl-core.h. I can do this but I wasn't sure you want this because there is now a (big) separation between MHI-AC-Ctrl-core. and the rest. MHI-AC-Ctrl-core. is just interacting with the AC.

  • The usage of the MQTT topic "Troom_offset" becomes not clear in the description, what has the user to do with this topic? Could you add an explanation? Maybe separated depending if Troom comes via DS18x20 or via MQTT.

I have added a description to SW-Configuration.md. Separation if possible of course but makes not much sense I think. It is displaying the offset which is applied to the Troom which is sent to the AC. If Troom is coming from set/Troom topic or DS18x20, makes no difference, it is the same Troom sent to the AC. Only the source will be different.

  • You added "Troom_offset|r|0.00 .. 0.50|Offset in °C for Troom used when ENHANCED RESOLUTION is used" to SW-Configuration.md - is really only reading planned? You used r, not r/w.

At this moment it is indeed only for reading. Of course this can change in future if there is functionality for it.

absalom-muc commented 1 year ago

@absalom-muc

  • Why have you added the line //#define DISABLE_FILTER_TROOM true to MHI-AC-Ctrl-core.h and not to support.h? Not sure if it is a good idea to have configuration on different places.

I have added //#define DISABLE_FILTER_TROOM true to MHI-AC-Ctrl-core.h instead of support.h because support.h is not included in MHI-AC-Ctrl-core.h. I can do this but I wasn't sure you want this because there is now a (big) separation between MHI-AC-Ctrl-core. and the rest. MHI-AC-Ctrl-core. is just interacting with the AC.

I see your point. We have the problem only, because the filter is implemented at the wrong place (in MHI-AC-Ctrl-core.cpp). A better place would be MHI-AC-Ctrl.ino. So if you don't mind I would move the filtering to MHI-AC-Ctrl.ino, then we can use support.h for the configuration switch. Are you o.k. with this way?

  • The usage of the MQTT topic "Troom_offset" becomes not clear in the description, what has the user to do with this topic? Could you add an explanation? Maybe separated depending if Troom comes via DS18x20 or via MQTT.

I have added a description to SW-Configuration.md. Separation if possible of course but makes not much sense I think. It is displaying the offset which is applied to the Troom which is sent to the AC. If Troom is coming from set/Troom topic or DS18x20, makes no difference, it is the same Troom sent to the AC. Only the source will be different.

  • You added "Troom_offset|r|0.00 .. 0.50|Offset in °C for Troom used when ENHANCED RESOLUTION is used" to SW-Configuration.md - is really only reading planned? You used r, not r/w.

At this moment it is indeed only for reading. Of course this can change in future if there is functionality for it.

Thank you for the clarification. According to my understanding the user doesn't need to know Troom_offset. I wouldn't publish it via MQTT to reduce complexity. Are you o.k. to remove the publishing part?

glsf91 commented 1 year ago

I see your point. We have the problem only, because the filter is implemented at the wrong place (in MHI-AC-Ctrl-core.cpp). A better place would be MHI-AC-Ctrl.ino. So if you don't mind I would move the filtering to MHI-AC-Ctrl.ino, then we can use support.h for the configuration switch. Are you o.k. with this way?

Ok for me.

According to my understanding the user doesn't need to know Troom_offset. I wouldn't publish it via MQTT to reduce complexity. Are you o.k. to remove the publishing part?

I can agree on this. I will make a new commit this evening or tomorrow.

glsf91 commented 1 year ago

I removed the Troom_offset topic and adjusted SW-Configuration.md accordingly.

absalom-muc commented 1 year ago

Thank you for your efforts @glsf91 .

Unfortunatelly I added my changes related to the Troom filtering before I've seen that you already adapted your pull request. This created some conflicts. However, I've added your program code manually to v2.6R4 and therefore don't consider the pull request. I would appreciate when you could test v2.6R4 to ensure that I haven't overseen something when implementing your code.

glsf91 commented 1 year ago

It is working fine.

I saw 1 thing:

#define TEMP_MEASURE_PERIOD 60
//#define ROOM_TEMP_DS18X20

TEMP_MEASURE_PERIOD should be 0 if external sensor is not used :-) Previous version it was 0.

absalom-muc commented 1 year ago

o.k. thank you

TEMP_MEASURE_PERIOD is fixed to 0.

glsf91 commented 1 year ago

@absalom-muc In SW-Configuration.md the part over ENHANCED_RESOLUTION is not right. The topic Troom_offset is there still mentioned. I think you missed my last edit. (https://github.com/absalom-muc/MHI-AC-Ctrl/pull/123/commits/a600083feb061b5f4afb21b9025cd3c215b60549) It should be:

If you now send x.5 degrees as setpoint, still the setpoint on the AC will be (x+1). But when sending the received Troom (from MQTT or the external temperature sensor) to the AC, Troom with an offset of .5 degrees will be send to the AC. This way the AC will increase the temperature in the room with .5 degrees instead of 1 degree. The MQTT topic Troom will show (like before) the Troom received by the AC (including the offset). For example: when setpoint is 20.5. When Troom 19.5 is received (from MQTT or DS18x20), Troom sent to the AC will be 20.0. Topic Troom will also show 20.0.

absalom-muc commented 1 year ago

o.k. fixed, thank you.