From abdcee450a74d1aae8997e94b97174370e03a79e Mon Sep 17 00:00:00 2001 From: Kaz Wolfe Date: Sat, 30 Mar 2024 10:08:37 -0700 Subject: [PATCH 1/8] feat: Add error code conversions --- .../mitsubishi_uart/muart_packet-derived.cpp | 19 ++++++++++++++++++- components/mitsubishi_uart/muart_packet.h | 5 ++++- 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/components/mitsubishi_uart/muart_packet-derived.cpp b/components/mitsubishi_uart/muart_packet-derived.cpp index fb5e687..84b3c0e 100644 --- a/components/mitsubishi_uart/muart_packet-derived.cpp +++ b/components/mitsubishi_uart/muart_packet-derived.cpp @@ -76,7 +76,7 @@ std::string ErrorStateGetResponsePacket::to_string() const { return ("Error State Response: " + Packet::to_string() + CONSOLE_COLOR_PURPLE + "\n ErrorCode: " + format_hex(getErrorCode()) - + " ShortCode: " + format_hex(getShortCode()) // TODO: This can be converted to a code string + + " ShortCode: " + getShortCode() + "(" + format_hex(getRawShortCode()) + ")" ); } std::string RemoteTemperatureSetRequestPacket::to_string() const { @@ -202,5 +202,22 @@ std::string ThermostatHelloRequestPacket::getThermostatVersionString() const { return buf; } +// ErrorStateGetResponsePacket functions +std::string ErrorStateGetResponsePacket::getShortCode() const { + const auto upperAlphabet = "AbEFJLPU"; + const auto lowerAlphabet = "0123456789ABCDEFOHJLPU"; + const auto errorCode = this->getRawShortCode(); + + auto lowBits = errorCode & 0x1F; + if (lowBits > 0x15) { + ESP_LOGW(PACKETS_TAG, "Error lowbits %x were out of range.", lowBits); + char buf[7]; + sprintf(buf, "ERR_%x", errorCode); + return buf; + } + + return {upperAlphabet[(errorCode & 0xE0) >> 5], lowerAlphabet[lowBits]}; +} + } // namespace mitsubishi_uart } // namespace esphome diff --git a/components/mitsubishi_uart/muart_packet.h b/components/mitsubishi_uart/muart_packet.h index 3b02878..331a639 100644 --- a/components/mitsubishi_uart/muart_packet.h +++ b/components/mitsubishi_uart/muart_packet.h @@ -257,7 +257,10 @@ class ErrorStateGetResponsePacket : public Packet { using Packet::Packet; public: uint16_t getErrorCode() const {return pkt_.getPayloadByte(4) << 8 | pkt_.getPayloadByte(5);} - uint8_t getShortCode() const {return pkt_.getPayloadByte(6);} + uint8_t getRawShortCode() const {return pkt_.getPayloadByte(6);} + std::string getShortCode() const; + + bool errorPresent() const { return getErrorCode() == 0x8000 && getRawShortCode() == 0x00; } std::string to_string() const override; }; From 7b38e9a3dfcda446f82663cc20e716a45c350a33 Mon Sep 17 00:00:00 2001 From: Kaz Wolfe Date: Sat, 30 Mar 2024 16:21:08 -0700 Subject: [PATCH 2/8] feat: Request error information if no MHK2 is detected --- components/mitsubishi_uart/mitsubishi_uart.cpp | 6 ++++++ components/mitsubishi_uart/mitsubishi_uart.h | 3 +++ components/mitsubishi_uart/muart_bridge.cpp | 2 +- components/mitsubishi_uart/muart_packet.h | 4 ++++ components/mitsubishi_uart/muart_rawpacket.h | 2 +- 5 files changed, 15 insertions(+), 2 deletions(-) diff --git a/components/mitsubishi_uart/mitsubishi_uart.cpp b/components/mitsubishi_uart/mitsubishi_uart.cpp index 33ac672..e588b75 100644 --- a/components/mitsubishi_uart/mitsubishi_uart.cpp +++ b/components/mitsubishi_uart/mitsubishi_uart.cpp @@ -107,6 +107,7 @@ be about `update_interval` late from their actual time. Generally the update in (default is 5seconds) this won't pose a practical problem. */ void MitsubishiUART::update() { + this->_updateLoopCounter += 1; // TODO: Temporarily wait 5 seconds on startup to help with viewing logs if (millis() < 5000) { @@ -133,6 +134,11 @@ void MitsubishiUART::update() { hp_bridge.sendPacket(GetRequestPacket::getStatusInstance()); hp_bridge.sendPacket(GetRequestPacket::getCurrentTempInstance()); ) + + // TODO: get this every 60 seconds instead of every n loops + if (this->_updateLoopCounter % 10 == 0 && this->ts_bridge != nullptr) { + IFACTIVE(hp_bridge.sendPacket(GetRequestPacket::getErrorInfoInstance());) + } } void MitsubishiUART::doPublish() { diff --git a/components/mitsubishi_uart/mitsubishi_uart.h b/components/mitsubishi_uart/mitsubishi_uart.h index e6addd5..8241149 100644 --- a/components/mitsubishi_uart/mitsubishi_uart.h +++ b/components/mitsubishi_uart/mitsubishi_uart.h @@ -136,6 +136,9 @@ class MitsubishiUART : public PollingComponent, public climate::Climate, public // Should we call publish on the next update? bool publishOnUpdate = false; + // used to track inconsistent updates + uint32_t _updateLoopCounter = 0; + // Preferences void save_preferences(); void restore_preferences(); diff --git a/components/mitsubishi_uart/muart_bridge.cpp b/components/mitsubishi_uart/muart_bridge.cpp index e1bd1d3..1cb6148 100644 --- a/components/mitsubishi_uart/muart_bridge.cpp +++ b/components/mitsubishi_uart/muart_bridge.cpp @@ -156,7 +156,7 @@ void MUARTBridge::classifyAndProcessRawPacket(RawPacket &pkt) const { case GetCommand::current_temp : processRawPacket(pkt, false); break; - case GetCommand::four : + case GetCommand::error_info : processRawPacket(pkt, false); break; case GetCommand::standby : diff --git a/components/mitsubishi_uart/muart_packet.h b/components/mitsubishi_uart/muart_packet.h index 331a639..3020c5b 100644 --- a/components/mitsubishi_uart/muart_packet.h +++ b/components/mitsubishi_uart/muart_packet.h @@ -180,6 +180,10 @@ class GetRequestPacket : public Packet { static GetRequestPacket INSTANCE = GetRequestPacket(GetCommand::status); return INSTANCE; } + static GetRequestPacket& getErrorInfoInstance() { + static GetRequestPacket INSTANCE = GetRequestPacket(GetCommand::error_info); + return INSTANCE; + } using Packet::Packet; private: diff --git a/components/mitsubishi_uart/muart_rawpacket.h b/components/mitsubishi_uart/muart_rawpacket.h index 4cdb51b..b8fddc9 100644 --- a/components/mitsubishi_uart/muart_rawpacket.h +++ b/components/mitsubishi_uart/muart_rawpacket.h @@ -33,7 +33,7 @@ enum class PacketType : uint8_t { enum class GetCommand : uint8_t { settings = 0x02, current_temp = 0x03, - four = 0x04, + error_info = 0x04, status = 0x06, standby = 0x09, a_9 = 0xa9 From aeb5e1fadf48401f0ec5f8e2f1698bb97008fc38 Mon Sep 17 00:00:00 2001 From: Kaz Wolfe Date: Sat, 30 Mar 2024 16:41:06 -0700 Subject: [PATCH 3/8] fix: Change test for detecting thermostat for error checks --- components/mitsubishi_uart/mitsubishi_uart.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/components/mitsubishi_uart/mitsubishi_uart.cpp b/components/mitsubishi_uart/mitsubishi_uart.cpp index e588b75..eef3c02 100644 --- a/components/mitsubishi_uart/mitsubishi_uart.cpp +++ b/components/mitsubishi_uart/mitsubishi_uart.cpp @@ -136,7 +136,7 @@ void MitsubishiUART::update() { ) // TODO: get this every 60 seconds instead of every n loops - if (this->_updateLoopCounter % 10 == 0 && this->ts_bridge != nullptr) { + if (this->_updateLoopCounter % 10 == 0 && !ts_bridge) { IFACTIVE(hp_bridge.sendPacket(GetRequestPacket::getErrorInfoInstance());) } } From a5a003619fa943a086552a6cdc6f00ba1a836bf7 Mon Sep 17 00:00:00 2001 From: Kaz Wolfe Date: Sat, 30 Mar 2024 15:30:44 -0700 Subject: [PATCH 4/8] feat: Add Error Sensor --- components/mitsubishi_uart/__init__.py | 12 +++++++--- .../mitsubishi_uart-packetprocessing.cpp | 24 +++++++++++++++---- .../mitsubishi_uart/mitsubishi_uart.cpp | 4 ++++ components/mitsubishi_uart/mitsubishi_uart.h | 2 ++ .../mitsubishi_uart/muart_packet-derived.cpp | 5 ++-- components/mitsubishi_uart/muart_packet.h | 2 +- 6 files changed, 39 insertions(+), 10 deletions(-) diff --git a/components/mitsubishi_uart/__init__.py b/components/mitsubishi_uart/__init__.py index 97521df..b2cb7cd 100644 --- a/components/mitsubishi_uart/__init__.py +++ b/components/mitsubishi_uart/__init__.py @@ -1,6 +1,6 @@ import esphome.codegen as cg import esphome.config_validation as cv -from esphome.components import climate, uart, sensor, binary_sensor, select, switch +from esphome.components import climate, uart, sensor, binary_sensor, text_sensor, select, switch from esphome.core import CORE from esphome.const import ( CONF_ID, @@ -19,14 +19,15 @@ ) from esphome.core import coroutine -AUTO_LOAD = ["climate", "select", "sensor", "binary_sensor", "switch"] -DEPENDENCIES = ["uart", "climate", "sensor", "binary_sensor", "select", "switch"] +AUTO_LOAD = ["climate", "select", "sensor", "binary_sensor", "text_sensor", "switch"] +DEPENDENCIES = ["uart", "climate", "sensor", "binary_sensor", "text_sensor", "select", "switch"] CONF_HP_UART = "heatpump_uart" CONF_TS_UART = "thermostat_uart" CONF_SENSORS = "sensors" CONF_SENSORS_THERMOSTAT_TEMP = "thermostat_temperature" +CONF_SENSORS_ERROR_CODE = "error_code" CONF_SELECTS = "selects" CONF_TEMPERATURE_SOURCE_SELECT = "temperature_source_select" # This is to create a Select object for selecting a source @@ -125,6 +126,11 @@ binary_sensor.binary_sensor_schema(), binary_sensor.register_binary_sensor ), + CONF_SENSORS_ERROR_CODE: ( + "Error Code", + text_sensor.text_sensor_schema(), + text_sensor.register_text_sensor + ) } SENSORS_SCHEMA = cv.All({ diff --git a/components/mitsubishi_uart/mitsubishi_uart-packetprocessing.cpp b/components/mitsubishi_uart/mitsubishi_uart-packetprocessing.cpp index d1af0c8..80401a5 100644 --- a/components/mitsubishi_uart/mitsubishi_uart-packetprocessing.cpp +++ b/components/mitsubishi_uart/mitsubishi_uart-packetprocessing.cpp @@ -285,13 +285,29 @@ void MitsubishiUART::processPacket(const StandbyGetResponsePacket &packet) { } //TODO: Not sure what AutoMode does yet -}; +} + void MitsubishiUART::processPacket(const ErrorStateGetResponsePacket &packet) { ESP_LOGV(TAG, "Processing %s", packet.to_string().c_str()); routePacket(packet); - // TODO: The MHK2 thermostat often asks for this, but the response is usually just all zeros. Could be be checking for - // errors / messages / warnings. Should check when e.g. the filter life runs out. -}; + + auto oldErrorCode = error_code_sensor->raw_state; + + // TODO: Include friendly text from JSON, somehow. + if (!packet.errorPresent()) { + error_code_sensor->raw_state = "No Error Reported"; + } else if (packet.getRawShortCode() != 0x00) { + error_code_sensor->raw_state = "Error " + packet.getShortCode(); + } else if (packet.getErrorCode() != 0x8000) { + error_code_sensor->raw_state = "Error " + to_string(packet.getErrorCode()); + } else { + // Logic bug :( + ESP_LOGW(TAG, "Packet indicated an error was present, but none of the error states matched. wat."); + } + + publishOnUpdate |= (oldErrorCode != error_code_sensor->state); +} + void MitsubishiUART::processPacket(const RemoteTemperatureSetRequestPacket &packet) { ESP_LOGV(TAG, "Processing %s", packet.to_string().c_str()); diff --git a/components/mitsubishi_uart/mitsubishi_uart.cpp b/components/mitsubishi_uart/mitsubishi_uart.cpp index eef3c02..8106ccd 100644 --- a/components/mitsubishi_uart/mitsubishi_uart.cpp +++ b/components/mitsubishi_uart/mitsubishi_uart.cpp @@ -163,6 +163,10 @@ void MitsubishiUART::doPublish() { ESP_LOGI(TAG, "Actual fan speed differs, do publish"); actual_fan_sensor->publish_state(actual_fan_sensor->raw_state); } + if (error_code_sensor && (error_code_sensor->raw_state != error_code_sensor->state)) { + ESP_LOGI(TAG, "Error code state differs, do publish"); + error_code_sensor->publish_state(error_code_sensor->raw_state); + } // Binary sensors automatically dedup publishes (I think) and so will only actually publish on change service_filter_sensor->publish_state(service_filter_sensor->state); diff --git a/components/mitsubishi_uart/mitsubishi_uart.h b/components/mitsubishi_uart/mitsubishi_uart.h index 8241149..e349326 100644 --- a/components/mitsubishi_uart/mitsubishi_uart.h +++ b/components/mitsubishi_uart/mitsubishi_uart.h @@ -69,6 +69,7 @@ class MitsubishiUART : public PollingComponent, public climate::Climate, public void set_defrost_sensor(binary_sensor::BinarySensor *sensor) {defrost_sensor = sensor;}; void set_hot_adjust_sensor(binary_sensor::BinarySensor *sensor) {hot_adjust_sensor = sensor;}; void set_standby_sensor(binary_sensor::BinarySensor *sensor) {standby_sensor = sensor;}; + void set_error_code_sensor(text_sensor::TextSensor *sensor) { error_code_sensor = sensor; }; // Select setters void set_temperature_source_select(select::Select *select) {temperature_source_select = select;}; @@ -153,6 +154,7 @@ class MitsubishiUART : public PollingComponent, public climate::Climate, public binary_sensor::BinarySensor *defrost_sensor = nullptr; binary_sensor::BinarySensor *hot_adjust_sensor = nullptr; binary_sensor::BinarySensor *standby_sensor = nullptr; + text_sensor::TextSensor *error_code_sensor = nullptr; // Selects select::Select *temperature_source_select; diff --git a/components/mitsubishi_uart/muart_packet-derived.cpp b/components/mitsubishi_uart/muart_packet-derived.cpp index 84b3c0e..a75aee8 100644 --- a/components/mitsubishi_uart/muart_packet-derived.cpp +++ b/components/mitsubishi_uart/muart_packet-derived.cpp @@ -1,5 +1,6 @@ #include "muart_packet.h" #include "muart_utils.h" +#include "mitsubishi_uart.h" namespace esphome { namespace mitsubishi_uart { @@ -75,7 +76,8 @@ std::string StatusGetResponsePacket::to_string() const { std::string ErrorStateGetResponsePacket::to_string() const { return ("Error State Response: " + Packet::to_string() + CONSOLE_COLOR_PURPLE - + "\n ErrorCode: " + format_hex(getErrorCode()) + + "\n Error State: " + (errorPresent() ? "Yes" : "No") + + " ErrorCode: " + format_hex(getErrorCode()) + " ShortCode: " + getShortCode() + "(" + format_hex(getRawShortCode()) + ")" ); } @@ -210,7 +212,6 @@ std::string ErrorStateGetResponsePacket::getShortCode() const { auto lowBits = errorCode & 0x1F; if (lowBits > 0x15) { - ESP_LOGW(PACKETS_TAG, "Error lowbits %x were out of range.", lowBits); char buf[7]; sprintf(buf, "ERR_%x", errorCode); return buf; diff --git a/components/mitsubishi_uart/muart_packet.h b/components/mitsubishi_uart/muart_packet.h index 3020c5b..be96161 100644 --- a/components/mitsubishi_uart/muart_packet.h +++ b/components/mitsubishi_uart/muart_packet.h @@ -264,7 +264,7 @@ class ErrorStateGetResponsePacket : public Packet { uint8_t getRawShortCode() const {return pkt_.getPayloadByte(6);} std::string getShortCode() const; - bool errorPresent() const { return getErrorCode() == 0x8000 && getRawShortCode() == 0x00; } + bool errorPresent() const { return getErrorCode() != 0x8000 || getRawShortCode() != 0x00; } std::string to_string() const override; }; From a0063bce1979abb09bf08896d764004e6541166a Mon Sep 17 00:00:00 2001 From: Kaz Wolfe Date: Sun, 31 Mar 2024 15:34:14 -0700 Subject: [PATCH 5/8] chore: Add in log validation for a bad error packet - Has no actual impact on anything, but good to know. --- .../mitsubishi_uart/mitsubishi_uart-packetprocessing.cpp | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/components/mitsubishi_uart/mitsubishi_uart-packetprocessing.cpp b/components/mitsubishi_uart/mitsubishi_uart-packetprocessing.cpp index 80401a5..211f4cc 100644 --- a/components/mitsubishi_uart/mitsubishi_uart-packetprocessing.cpp +++ b/components/mitsubishi_uart/mitsubishi_uart-packetprocessing.cpp @@ -296,12 +296,17 @@ void MitsubishiUART::processPacket(const ErrorStateGetResponsePacket &packet) { // TODO: Include friendly text from JSON, somehow. if (!packet.errorPresent()) { error_code_sensor->raw_state = "No Error Reported"; - } else if (packet.getRawShortCode() != 0x00) { + } else if (auto rawCode = packet.getRawShortCode() != 0x00) { + // Not that it matters, but good for validation I guess. + if ((rawCode & 0x1F) > 0x15) { + ESP_LOGW(TAG, "Error short code %x had invalid low bits. This is an IT protocol violation!", rawCode); + } + error_code_sensor->raw_state = "Error " + packet.getShortCode(); } else if (packet.getErrorCode() != 0x8000) { error_code_sensor->raw_state = "Error " + to_string(packet.getErrorCode()); } else { - // Logic bug :( + // Logic bug, should never happen. ESP_LOGW(TAG, "Packet indicated an error was present, but none of the error states matched. wat."); } From acbfd0171b952d155f0e90fedf76cbd68be01ea2 Mon Sep 17 00:00:00 2001 From: Kaz Wolfe Date: Sun, 31 Mar 2024 22:52:15 -0700 Subject: [PATCH 6/8] fix: Prevent possible UB caused by integer overflow While the loop counter overflowing shouldn't cause any particular problems, it's still probably best to just not let it overflow in the first place. 10,000 cycles should be enough for *any* scheduled event until there's a better system for infrequent events. --- components/mitsubishi_uart/mitsubishi_uart.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/components/mitsubishi_uart/mitsubishi_uart.cpp b/components/mitsubishi_uart/mitsubishi_uart.cpp index 8106ccd..3eee9e5 100644 --- a/components/mitsubishi_uart/mitsubishi_uart.cpp +++ b/components/mitsubishi_uart/mitsubishi_uart.cpp @@ -107,7 +107,8 @@ be about `update_interval` late from their actual time. Generally the update in (default is 5seconds) this won't pose a practical problem. */ void MitsubishiUART::update() { - this->_updateLoopCounter += 1; + // increment our update loop counter, and reset at 10,000 just to prevent overflow UB + if (++this->_updateLoopCounter >= 10000) this->_updateLoopCounter = 0; // TODO: Temporarily wait 5 seconds on startup to help with viewing logs if (millis() < 5000) { From b89e0e142a99c2f4390ea0764b3aceb065f01bd7 Mon Sep 17 00:00:00 2001 From: Kaz Wolfe Date: Sun, 31 Mar 2024 23:07:53 -0700 Subject: [PATCH 7/8] fix: Code Review Comments - Remove identified `auto` typing. - Simplify error packet else case to not care about logic flaws. - Check `raw_state` for publish checks, as that's what we're actually setting. --- .../mitsubishi_uart/mitsubishi_uart-packetprocessing.cpp | 9 +++------ components/mitsubishi_uart/muart_packet-derived.cpp | 8 ++++---- 2 files changed, 7 insertions(+), 10 deletions(-) diff --git a/components/mitsubishi_uart/mitsubishi_uart-packetprocessing.cpp b/components/mitsubishi_uart/mitsubishi_uart-packetprocessing.cpp index 211f4cc..46829dd 100644 --- a/components/mitsubishi_uart/mitsubishi_uart-packetprocessing.cpp +++ b/components/mitsubishi_uart/mitsubishi_uart-packetprocessing.cpp @@ -291,7 +291,7 @@ void MitsubishiUART::processPacket(const ErrorStateGetResponsePacket &packet) { ESP_LOGV(TAG, "Processing %s", packet.to_string().c_str()); routePacket(packet); - auto oldErrorCode = error_code_sensor->raw_state; + std::string oldErrorCode = error_code_sensor->raw_state; // TODO: Include friendly text from JSON, somehow. if (!packet.errorPresent()) { @@ -303,14 +303,11 @@ void MitsubishiUART::processPacket(const ErrorStateGetResponsePacket &packet) { } error_code_sensor->raw_state = "Error " + packet.getShortCode(); - } else if (packet.getErrorCode() != 0x8000) { - error_code_sensor->raw_state = "Error " + to_string(packet.getErrorCode()); } else { - // Logic bug, should never happen. - ESP_LOGW(TAG, "Packet indicated an error was present, but none of the error states matched. wat."); + error_code_sensor->raw_state = "Error " + to_string(packet.getErrorCode()); } - publishOnUpdate |= (oldErrorCode != error_code_sensor->state); + publishOnUpdate |= (oldErrorCode != error_code_sensor->raw_state); } void MitsubishiUART::processPacket(const RemoteTemperatureSetRequestPacket &packet) { diff --git a/components/mitsubishi_uart/muart_packet-derived.cpp b/components/mitsubishi_uart/muart_packet-derived.cpp index a75aee8..c7bb468 100644 --- a/components/mitsubishi_uart/muart_packet-derived.cpp +++ b/components/mitsubishi_uart/muart_packet-derived.cpp @@ -206,11 +206,11 @@ std::string ThermostatHelloRequestPacket::getThermostatVersionString() const { // ErrorStateGetResponsePacket functions std::string ErrorStateGetResponsePacket::getShortCode() const { - const auto upperAlphabet = "AbEFJLPU"; - const auto lowerAlphabet = "0123456789ABCDEFOHJLPU"; - const auto errorCode = this->getRawShortCode(); + const char* upperAlphabet = "AbEFJLPU"; + const char* lowerAlphabet = "0123456789ABCDEFOHJLPU"; + const uint8_t errorCode = this->getRawShortCode(); - auto lowBits = errorCode & 0x1F; + uint8_t lowBits = errorCode & 0x1F; if (lowBits > 0x15) { char buf[7]; sprintf(buf, "ERR_%x", errorCode); From b1f66f7c18792693de698ad7402b6fcab7ea6a63 Mon Sep 17 00:00:00 2001 From: Kaz Wolfe Date: Sun, 31 Mar 2024 23:16:03 -0700 Subject: [PATCH 8/8] fix: More aggressive error checking - Remove code to only check errors if the MHK isn't sending those packets. - Remove the every-n-ticks system. - Add TODO just in case this does cause issues later. --- components/mitsubishi_uart/mitsubishi_uart.cpp | 12 ++++-------- components/mitsubishi_uart/mitsubishi_uart.h | 3 --- 2 files changed, 4 insertions(+), 11 deletions(-) diff --git a/components/mitsubishi_uart/mitsubishi_uart.cpp b/components/mitsubishi_uart/mitsubishi_uart.cpp index 3eee9e5..233798b 100644 --- a/components/mitsubishi_uart/mitsubishi_uart.cpp +++ b/components/mitsubishi_uart/mitsubishi_uart.cpp @@ -107,9 +107,6 @@ be about `update_interval` late from their actual time. Generally the update in (default is 5seconds) this won't pose a practical problem. */ void MitsubishiUART::update() { - // increment our update loop counter, and reset at 10,000 just to prevent overflow UB - if (++this->_updateLoopCounter >= 10000) this->_updateLoopCounter = 0; - // TODO: Temporarily wait 5 seconds on startup to help with viewing logs if (millis() < 5000) { return; @@ -130,16 +127,15 @@ void MitsubishiUART::update() { IFACTIVE( // Request an update from the heatpump + // TODO: This isn't a problem *yet*, but sending all these packets every loop might start to cause some issues in + // certain configurations or setups. We may want to consider only asking for certain packets on a rarer cadence, + // depending on their utility (e.g. we dont need to check for errors every loop). hp_bridge.sendPacket(GetRequestPacket::getSettingsInstance()); // Needs to be done before status packet for mode logic to work hp_bridge.sendPacket(GetRequestPacket::getStandbyInstance()); hp_bridge.sendPacket(GetRequestPacket::getStatusInstance()); hp_bridge.sendPacket(GetRequestPacket::getCurrentTempInstance()); + hp_bridge.sendPacket(GetRequestPacket::getErrorInfoInstance()); ) - - // TODO: get this every 60 seconds instead of every n loops - if (this->_updateLoopCounter % 10 == 0 && !ts_bridge) { - IFACTIVE(hp_bridge.sendPacket(GetRequestPacket::getErrorInfoInstance());) - } } void MitsubishiUART::doPublish() { diff --git a/components/mitsubishi_uart/mitsubishi_uart.h b/components/mitsubishi_uart/mitsubishi_uart.h index e349326..97197ba 100644 --- a/components/mitsubishi_uart/mitsubishi_uart.h +++ b/components/mitsubishi_uart/mitsubishi_uart.h @@ -137,9 +137,6 @@ class MitsubishiUART : public PollingComponent, public climate::Climate, public // Should we call publish on the next update? bool publishOnUpdate = false; - // used to track inconsistent updates - uint32_t _updateLoopCounter = 0; - // Preferences void save_preferences(); void restore_preferences();