diff --git a/esphome/components/mitsubishi_cn105/mitsubishi_cn105.cpp b/esphome/components/mitsubishi_cn105/mitsubishi_cn105.cpp index 2a173997f3f..56f1ee1b3f4 100644 --- a/esphome/components/mitsubishi_cn105/mitsubishi_cn105.cpp +++ b/esphome/components/mitsubishi_cn105/mitsubishi_cn105.cpp @@ -8,7 +8,7 @@ namespace esphome::mitsubishi_cn105 { static const char *const TAG = "mitsubishi_cn105.driver"; -static constexpr uint32_t WRITE_TIMEOUT_MS = 2000; +static constexpr uint32_t RESPONSE_TIMEOUT_MS = 2000; static constexpr uint8_t TARGET_TEMPERATURE_ENC_A_OFFSET = 31; @@ -91,25 +91,31 @@ static constexpr auto CONNECT_PACKET = make_packet(PACKET_TYPE_CONNECT_REQUEST, void MitsubishiCN105::initialize() { this->set_state_(State::CONNECTING); } bool MitsubishiCN105::update() { - if (const auto start = this->status_update_start_ms_) { - if (this->pending_updates_.any()) { - this->status_update_wait_credit_ms_ = std::min(this->update_interval_ms_, get_loop_time_ms() - *start); - this->cancel_waiting_and_transition_to_(State::APPLYING_SETTINGS); - return false; - } + switch (this->state_) { + case State::WAITING_FOR_SCHEDULED_STATUS_UPDATE: + if (this->pending_updates_.any()) { + this->status_update_wait_credit_ms_ = + std::min(this->update_interval_ms_, get_loop_time_ms() - this->operation_start_ms_); + this->set_state_(State::APPLYING_SETTINGS); + return false; + } + if (this->has_timed_out_(this->update_interval_ms_)) { + this->set_state_(State::UPDATING_STATUS); + return false; + } + break; - if ((get_loop_time_ms() - *start) >= this->update_interval_ms_) { - this->cancel_waiting_and_transition_to_(State::UPDATING_STATUS); - return false; - } - } + case State::CONNECTING: + case State::UPDATING_STATUS: + case State::APPLYING_SETTINGS: + if (this->has_timed_out_(RESPONSE_TIMEOUT_MS)) { + this->set_state_(State::READ_TIMEOUT); + return false; + } + break; - if (const auto start = this->write_timeout_start_ms_; start && (get_loop_time_ms() - *start) >= WRITE_TIMEOUT_MS) { - this->write_timeout_start_ms_.reset(); - this->frame_parser_.reset(); - this->status_update_wait_credit_ms_ = 0; - this->set_state_(State::READ_TIMEOUT); - return false; + default: + break; } return this->frame_parser_.read_and_parse(this->device_, [this](uint8_t type, const uint8_t *payload, size_t len) { @@ -171,7 +177,6 @@ void MitsubishiCN105::did_transition_(State to) { break; case State::CONNECTED: - this->write_timeout_start_ms_.reset(); this->current_status_msg_type_ = STATUS_MSG_SETTINGS; this->set_state_(State::UPDATING_STATUS); break; @@ -181,7 +186,6 @@ void MitsubishiCN105::did_transition_(State to) { break; case State::STATUS_UPDATED: { - this->write_timeout_start_ms_.reset(); if (this->pending_updates_.any() && this->is_status_initialized()) { this->set_state_(State::APPLYING_SETTINGS); } else if (this->current_status_msg_type_ == STATUS_MSG_SETTINGS && this->should_request_room_temperature_()) { @@ -194,7 +198,7 @@ void MitsubishiCN105::did_transition_(State to) { } case State::SCHEDULE_NEXT_STATUS_UPDATE: - this->status_update_start_ms_ = get_loop_time_ms() - this->status_update_wait_credit_ms_; + this->operation_start_ms_ = get_loop_time_ms() - this->status_update_wait_credit_ms_; this->status_update_wait_credit_ms_ = 0; this->current_status_msg_type_ = STATUS_MSG_SETTINGS; this->set_state_(State::WAITING_FOR_SCHEDULED_STATUS_UPDATE); @@ -205,11 +209,12 @@ void MitsubishiCN105::did_transition_(State to) { break; case State::SETTINGS_APPLIED: - this->write_timeout_start_ms_.reset(); this->set_state_(State::SCHEDULE_NEXT_STATUS_UPDATE); break; case State::READ_TIMEOUT: + this->frame_parser_.reset(); + this->status_update_wait_credit_ms_ = 0; this->set_state_(State::CONNECTING); break; @@ -233,7 +238,7 @@ bool MitsubishiCN105::should_request_room_temperature_() const { void MitsubishiCN105::send_packet_(const uint8_t *packet, size_t len) { FrameParser::dump_buffer_vv("TX", packet, len); this->device_.write_array(packet, len); - this->write_timeout_start_ms_ = get_loop_time_ms(); + this->operation_start_ms_ = get_loop_time_ms(); } void MitsubishiCN105::update_status_() { @@ -241,11 +246,6 @@ void MitsubishiCN105::update_status_() { this->send_packet_(make_packet(PACKET_TYPE_STATUS_REQUEST, payload)); } -void MitsubishiCN105::cancel_waiting_and_transition_to_(State state) { - this->status_update_start_ms_.reset(); - this->set_state_(state); -} - bool MitsubishiCN105::process_rx_packet_(uint8_t type, const uint8_t *payload, size_t len) { switch (type) { case PACKET_TYPE_CONNECT_RESPONSE: diff --git a/esphome/components/mitsubishi_cn105/mitsubishi_cn105.h b/esphome/components/mitsubishi_cn105/mitsubishi_cn105.h index 52b78efccda..60ca81cf9e0 100644 --- a/esphome/components/mitsubishi_cn105/mitsubishi_cn105.h +++ b/esphome/components/mitsubishi_cn105/mitsubishi_cn105.h @@ -124,9 +124,9 @@ class MitsubishiCN105 { bool parse_status_room_temperature_(const uint8_t *payload, size_t len); void send_packet_(const uint8_t *packet, size_t len); void update_status_(); - void cancel_waiting_and_transition_to_(State state); bool should_request_room_temperature_() const; void apply_settings_(); + bool has_timed_out_(uint32_t timeout) const { return ((get_loop_time_ms() - this->operation_start_ms_) >= timeout); } void set_remote_temperature_half_deg_(uint8_t temperature_half_deg); template void send_packet_(const T &packet) { this->send_packet_(packet.data(), packet.size()); } static bool should_transition(State from, State to); @@ -135,9 +135,8 @@ class MitsubishiCN105 { uart::UARTDevice &device_; uint32_t update_interval_ms_{1000}; uint32_t status_update_wait_credit_ms_{0}; + uint32_t operation_start_ms_{0}; uint32_t room_temperature_min_interval_ms_{60000}; - std::optional write_timeout_start_ms_; - std::optional status_update_start_ms_; std::optional last_room_temperature_update_ms_; Status status_{}; State state_{State::NOT_CONNECTED}; diff --git a/tests/components/mitsubishi_cn105/climate/mitsubishi_cn105_tests.cpp b/tests/components/mitsubishi_cn105/climate/mitsubishi_cn105_tests.cpp index 7615b62d03f..db2fbced1c3 100644 --- a/tests/components/mitsubishi_cn105/climate/mitsubishi_cn105_tests.cpp +++ b/tests/components/mitsubishi_cn105/climate/mitsubishi_cn105_tests.cpp @@ -16,13 +16,13 @@ TEST(MitsubishiCN105Tests, InitSendsConnectPacket) { ctx.sut.set_current_time(123); EXPECT_EQ(ctx.sut.state_, TestableMitsubishiCN105::State::NOT_CONNECTED); EXPECT_TRUE(ctx.uart.tx.empty()); - EXPECT_FALSE(ctx.sut.write_timeout_start_ms_.has_value()); + EXPECT_EQ(ctx.sut.operation_start_ms_, 0); ctx.sut.initialize(); EXPECT_EQ(ctx.sut.state_, TestableMitsubishiCN105::State::CONNECTING); EXPECT_THAT(ctx.uart.tx, ::testing::ElementsAre(0xFC, 0x5A, 0x01, 0x30, 0x02, 0xCA, 0x01, 0xA8)); - EXPECT_EQ(ctx.sut.write_timeout_start_ms_, std::optional{123}); + EXPECT_EQ(ctx.sut.operation_start_ms_, 123); } TEST(MitsubishiCN105Tests, ConnectAndUpdateStatus) { @@ -32,8 +32,7 @@ TEST(MitsubishiCN105Tests, ConnectAndUpdateStatus) { ctx.uart.tx.clear(); // Remove first connect packet bytes EXPECT_EQ(ctx.sut.state_, TestableMitsubishiCN105::State::CONNECTING); - EXPECT_EQ(ctx.sut.write_timeout_start_ms_, std::optional{0}); - EXPECT_FALSE(ctx.sut.status_update_start_ms_.has_value()); + EXPECT_EQ(ctx.sut.operation_start_ms_, 0); // Connect response ctx.uart.push_rx({0xFC, 0x7A, 0x01, 0x30, 0x00, 0x55}); @@ -47,8 +46,7 @@ TEST(MitsubishiCN105Tests, ConnectAndUpdateStatus) { EXPECT_EQ(ctx.sut.state_, TestableMitsubishiCN105::State::UPDATING_STATUS); EXPECT_THAT(ctx.uart.tx, ::testing::ElementsAre(0xFC, 0x42, 0x01, 0x30, 0x10, 0x02, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x7B)); - EXPECT_EQ(ctx.sut.write_timeout_start_ms_, std::optional{200}); - EXPECT_FALSE(ctx.sut.status_update_start_ms_.has_value()); + EXPECT_EQ(ctx.sut.operation_start_ms_, 200); // Clear TX bytes. ctx.uart.tx.clear(); @@ -77,8 +75,7 @@ TEST(MitsubishiCN105Tests, ConnectAndUpdateStatus) { EXPECT_EQ(ctx.sut.state_, TestableMitsubishiCN105::State::UPDATING_STATUS); EXPECT_THAT(ctx.uart.tx, ::testing::ElementsAre(0xFC, 0x42, 0x01, 0x30, 0x10, 0x03, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x7A)); - EXPECT_EQ(ctx.sut.write_timeout_start_ms_, std::optional{300}); - EXPECT_FALSE(ctx.sut.status_update_start_ms_.has_value()); + EXPECT_EQ(ctx.sut.operation_start_ms_, 300); // Clear TX bytes. ctx.uart.tx.clear(); @@ -101,8 +98,7 @@ TEST(MitsubishiCN105Tests, ConnectAndUpdateStatus) { EXPECT_TRUE(ctx.uart.tx.empty()); EXPECT_EQ(ctx.sut.state_, TestableMitsubishiCN105::State::WAITING_FOR_SCHEDULED_STATUS_UPDATE); - EXPECT_FALSE(ctx.sut.write_timeout_start_ms_.has_value()); - EXPECT_EQ(ctx.sut.status_update_start_ms_, std::optional{400}); + EXPECT_EQ(ctx.sut.operation_start_ms_, 400); } TEST(MitsubishiCN105Tests, NoResponseTriggersReconnect) { @@ -115,21 +111,21 @@ TEST(MitsubishiCN105Tests, NoResponseTriggersReconnect) { ASSERT_FALSE(ctx.sut.update()); EXPECT_EQ(ctx.sut.state_, TestableMitsubishiCN105::State::CONNECTING); EXPECT_TRUE(ctx.uart.tx.empty()); - EXPECT_EQ(ctx.sut.write_timeout_start_ms_, std::optional{0}); + EXPECT_EQ(ctx.sut.operation_start_ms_, 0); // Still no response after 1999ms, no retry yet ctx.sut.set_current_time(1999); ASSERT_FALSE(ctx.sut.update()); EXPECT_EQ(ctx.sut.state_, TestableMitsubishiCN105::State::CONNECTING); EXPECT_TRUE(ctx.uart.tx.empty()); - EXPECT_EQ(ctx.sut.write_timeout_start_ms_, std::optional{0}); + EXPECT_EQ(ctx.sut.operation_start_ms_, 0); // Stop waiting after 2s and retry connect ctx.sut.set_current_time(2000); ASSERT_FALSE(ctx.sut.update()); EXPECT_EQ(ctx.sut.state_, TestableMitsubishiCN105::State::CONNECTING); EXPECT_THAT(ctx.uart.tx, ::testing::ElementsAre(0xFC, 0x5A, 0x01, 0x30, 0x02, 0xCA, 0x01, 0xA8)); - EXPECT_EQ(ctx.sut.write_timeout_start_ms_, std::optional{2000}); + EXPECT_EQ(ctx.sut.operation_start_ms_, 2000); } TEST(MitsubishiCN105Tests, RxWatchdogLimitsProcessingPerUpdate) { @@ -233,15 +229,12 @@ TEST(MitsubishiCN105Tests, NextStatusUpdateAfterUpdateIntervalMilliseconds) { ctx.sut.set_update_interval(2000); ctx.sut.set_current_time(80000); - // No scheduled status update - EXPECT_FALSE(ctx.sut.status_update_start_ms_.has_value()); - // Status update completed, schedule next status update ctx.sut.state_ = TestableMitsubishiCN105::State::STATUS_UPDATED; ctx.sut.set_state(TestableMitsubishiCN105::State::SCHEDULE_NEXT_STATUS_UPDATE); EXPECT_EQ(ctx.sut.state_, TestableMitsubishiCN105::State::WAITING_FOR_SCHEDULED_STATUS_UPDATE); - EXPECT_EQ(ctx.sut.status_update_start_ms_, std::optional{80000}); + EXPECT_EQ(ctx.sut.operation_start_ms_, 80000); // Wait for update_interval (ms) before doing another status update ASSERT_FALSE(ctx.sut.update()); @@ -257,7 +250,7 @@ TEST(MitsubishiCN105Tests, NextStatusUpdateAfterUpdateIntervalMilliseconds) { ASSERT_FALSE(ctx.sut.update()); EXPECT_FALSE(ctx.uart.tx.empty()); EXPECT_EQ(ctx.sut.state_, TestableMitsubishiCN105::State::UPDATING_STATUS); - EXPECT_FALSE(ctx.sut.status_update_start_ms_.has_value()); + EXPECT_EQ(ctx.sut.operation_start_ms_, 82000); } TEST(MitsubishiCN105Tests, DecodeStatusSettingsPackageTempEncodedA) { @@ -382,14 +375,14 @@ TEST(MitsubishiCN105Tests, WriteInterruptsWaitingForNextStatusUpdate) { ctx.sut.state_ = TestableMitsubishiCN105::State::STATUS_UPDATED; ctx.sut.set_state(TestableMitsubishiCN105::State::SCHEDULE_NEXT_STATUS_UPDATE); EXPECT_EQ(ctx.sut.state_, TestableMitsubishiCN105::State::WAITING_FOR_SCHEDULED_STATUS_UPDATE); - EXPECT_EQ(ctx.sut.status_update_start_ms_, std::optional{5000}); + EXPECT_EQ(ctx.sut.operation_start_ms_, 5000); EXPECT_EQ(ctx.sut.status_update_wait_credit_ms_, 0); // Nothing to do in update (rx empty, no timeout) ctx.sut.set_current_time(5500); ASSERT_FALSE(ctx.sut.update()); EXPECT_TRUE(ctx.uart.tx.empty()); - EXPECT_EQ(ctx.sut.status_update_start_ms_, std::optional{5000}); + EXPECT_EQ(ctx.sut.operation_start_ms_, 5000); EXPECT_EQ(ctx.sut.status_update_wait_credit_ms_, 0); // Write new values @@ -402,7 +395,6 @@ TEST(MitsubishiCN105Tests, WriteInterruptsWaitingForNextStatusUpdate) { // Waiting for next status update must be interrupted and new values send to AC ctx.sut.set_current_time(6000); ASSERT_FALSE(ctx.sut.update()); - EXPECT_FALSE(ctx.sut.status_update_start_ms_.has_value()); EXPECT_EQ(ctx.sut.status_update_wait_credit_ms_, 1000); EXPECT_EQ(ctx.sut.state_, TestableMitsubishiCN105::State::APPLYING_SETTINGS); EXPECT_THAT(ctx.uart.tx, ::testing::ElementsAre(0xFC, 0x41, 0x01, 0x30, 0x10, 0x01, 0x0F, 0x00, 0x00, 0x01, 0x00, @@ -414,7 +406,7 @@ TEST(MitsubishiCN105Tests, WriteInterruptsWaitingForNextStatusUpdate) { ctx.sut.set_current_time(6500); ASSERT_FALSE(ctx.sut.update()); EXPECT_EQ(ctx.sut.state_, TestableMitsubishiCN105::State::WAITING_FOR_SCHEDULED_STATUS_UPDATE); - EXPECT_EQ(ctx.sut.status_update_start_ms_, std::optional{6500 - 1000}); + EXPECT_EQ(ctx.sut.operation_start_ms_, 6500 - 1000); EXPECT_EQ(ctx.sut.status_update_wait_credit_ms_, 0); } @@ -502,7 +494,7 @@ TEST(MitsubishiCN105Tests, WriteTimeoutClearsStatusUpdateWaitCreditOnReconnect) ctx.sut.state_ = TestableMitsubishiCN105::State::STATUS_UPDATED; ctx.sut.set_state(TestableMitsubishiCN105::State::SCHEDULE_NEXT_STATUS_UPDATE); ASSERT_EQ(ctx.sut.state_, TestableMitsubishiCN105::State::WAITING_FOR_SCHEDULED_STATUS_UPDATE); - ASSERT_EQ(ctx.sut.status_update_start_ms_, std::optional{5000}); + ASSERT_EQ(ctx.sut.operation_start_ms_, 5000); ASSERT_EQ(ctx.sut.status_update_wait_credit_ms_, 0); // Interrupt that wait with a write so credit is accumulated. @@ -514,7 +506,7 @@ TEST(MitsubishiCN105Tests, WriteTimeoutClearsStatusUpdateWaitCreditOnReconnect) ctx.sut.set_current_time(6000); ASSERT_FALSE(ctx.sut.update()); ASSERT_EQ(ctx.sut.state_, TestableMitsubishiCN105::State::APPLYING_SETTINGS); - ASSERT_FALSE(ctx.sut.status_update_start_ms_.has_value()); + ASSERT_EQ(ctx.sut.operation_start_ms_, 6000); ASSERT_EQ(ctx.sut.status_update_wait_credit_ms_, 1000); // Do not ACK the write. Advance time far enough to force timeout/reconnect @@ -522,8 +514,8 @@ TEST(MitsubishiCN105Tests, WriteTimeoutClearsStatusUpdateWaitCreditOnReconnect) ctx.sut.set_current_time(36000); ASSERT_FALSE(ctx.sut.update()); EXPECT_NE(ctx.sut.state_, TestableMitsubishiCN105::State::APPLYING_SETTINGS); + ASSERT_EQ(ctx.sut.operation_start_ms_, 36000); EXPECT_EQ(ctx.sut.status_update_wait_credit_ms_, 0); - EXPECT_FALSE(ctx.sut.status_update_start_ms_.has_value()); } TEST(MitsubishiCN105Tests, SetOutOfRangeRemoteRoomTempIsIgnored) { diff --git a/tests/components/mitsubishi_cn105/common.h b/tests/components/mitsubishi_cn105/common.h index d0fdca1ea5a..73e09d6c84b 100644 --- a/tests/components/mitsubishi_cn105/common.h +++ b/tests/components/mitsubishi_cn105/common.h @@ -44,8 +44,7 @@ class TestableMitsubishiCN105 : public MitsubishiCN105 { using MitsubishiCN105::State; using MitsubishiCN105::UpdateFlag; using MitsubishiCN105::state_; - using MitsubishiCN105::write_timeout_start_ms_; - using MitsubishiCN105::status_update_start_ms_; + using MitsubishiCN105::operation_start_ms_; using MitsubishiCN105::use_temperature_encoding_b_; using MitsubishiCN105::status_update_wait_credit_ms_; using MitsubishiCN105::pending_updates_;