diff --git a/esphome/components/nextion/nextion.cpp b/esphome/components/nextion/nextion.cpp index a5ac2325369..21303566ce5 100644 --- a/esphome/components/nextion/nextion.cpp +++ b/esphome/components/nextion/nextion.cpp @@ -1,11 +1,13 @@ #include "nextion.h" #include +#include #include #include "esphome/core/application.h" #include "esphome/core/helpers.h" #include "esphome/core/log.h" +#include "esphome/core/progmem.h" #include "esphome/core/string_ref.h" #include "esphome/core/util.h" @@ -18,6 +20,18 @@ static bool is_sleep_wake_command(const NextionComponentBase *component) { return component->get_queue_type() == NextionQueueType::NO_RESULT && component->get_variable_name() == "sleep_wake"; } +// The display answers a raw "get" with a value instead of 0x01. Its queue entry is named RAW_GET_NAME so that +// value removes it. An entity with this variable name would clash when it writes its state; none is expected. +static constexpr const char *RAW_GET_NAME = "raw_get"; + +static bool is_raw_get_command(const NextionComponentBase *component) { + return component->get_queue_type() == NextionQueueType::NO_RESULT && component->get_variable_name() == RAW_GET_NAME; +} + +static const char *raw_command_name(const char *command, const char *name) { + return ESPHOME_strncmp_P(command, ESPHOME_PSTR("get "), 4) == 0 ? RAW_GET_NAME : name; +} + // Nextion command terminator: three consecutive 0xFF bytes (per Nextion Instruction Set v1.1). static constexpr uint8_t COMMAND_DELIMITER[3] = {0xFF, 0xFF, 0xFF}; static constexpr size_t DELIMITER_SIZE = sizeof(COMMAND_DELIMITER); @@ -287,7 +301,7 @@ bool Nextion::send_command(const char *command) { if ((!this->is_setup() && !this->connection_state_.ignore_is_setup_) || this->is_sleeping()) return false; - this->add_no_result_to_queue_with_command_("command", command); + this->add_no_result_to_queue_with_command_(raw_command_name(command, "command"), command); return true; } @@ -305,7 +319,7 @@ bool Nextion::send_command_printf(const char *format, ...) { return false; } - this->add_no_result_to_queue_with_command_("command_printf", buffer); + this->add_no_result_to_queue_with_command_(raw_command_name(buffer, "command_printf"), buffer); return true; } @@ -448,6 +462,75 @@ bool Nextion::remove_from_q_(bool report_empty) { return true; } +void Nextion::deliver_queue_response_(const LogString *response_name, const NextionQueueResponse &resp) { + auto it = this->nextion_queue_.begin(); + while (it != this->nextion_queue_.end()) { + NextionQueue *nb = *it; + if (nb == nullptr || nb->component == nullptr) { + ESP_LOGE(TAG, "Invalid queue entry"); + if (nb != nullptr) { + this->release_queue_entry_(nb); + } + it = this->nextion_queue_.erase(it); + continue; + } + NextionComponentBase *component = nb->component; + NextionQueueType qtype = component->get_queue_type(); + const bool raw_get = is_raw_get_command(component); + + if (!nb->pending_command.empty()) { + ESP_LOGV(TAG, "%s: skipping unsent '%s'", LOG_STR_ARG(response_name), component->get_variable_name().c_str()); + ++it; + continue; + } + + if (qtype == NextionQueueType::NO_RESULT && !raw_get) { + // Leave in queue -- its 0x01 ACK will remove it via remove_from_q_(). + ESP_LOGV(TAG, "%s: skipping NO_RESULT '%s'", LOG_STR_ARG(response_name), component->get_variable_name().c_str()); + ++it; + continue; + } + + // The display answers queries in order, so the response belongs to this query even when the type does + // not match. Consume only this entry and never hand the value to a later query. + if (raw_get) { + // A raw "get" from send_command() has no entity to update + ESP_LOGV(TAG, "%s for raw get dropped", LOG_STR_ARG(response_name)); + } else if (!resp.is_string && (qtype == NextionQueueType::SENSOR || qtype == NextionQueueType::BINARY_SENSOR || + qtype == NextionQueueType::SWITCH)) { + ESP_LOGN(TAG, "Numeric: %s type %d:%s val %d", component->get_variable_name().c_str(), qtype, + component->get_queue_type_string(), resp.int_value); + component->set_state_from_int(resp.int_value, true, false); + } else if (resp.is_string && qtype == NextionQueueType::TEXT_SENSOR) { + ESP_LOGN(TAG, "String resp: '%s' id: %s type: %s", resp.str_value->c_str(), + component->get_variable_name().c_str(), component->get_queue_type_string()); + component->set_state_from_string(*resp.str_value, true, false); + } else { + ESP_LOGE(TAG, "%s but '%s' invalid type %d", LOG_STR_ARG(response_name), component->get_variable_name().c_str(), + qtype); + } + + this->release_queue_entry_(nb); + this->nextion_queue_.erase(it); + return; + } + ESP_LOGE(TAG, "%s but no sent query in queue", LOG_STR_ARG(response_name)); +} + +void Nextion::set_numeric_return_(int value) { + NextionQueueResponse resp{}; + resp.is_string = false; + resp.int_value = value; + this->deliver_queue_response_(LOG_STR("Numeric return"), resp); +} + +void Nextion::set_string_return_(const std::string &value) { + NextionQueueResponse resp{}; + resp.is_string = true; + resp.str_value = &value; + this->deliver_queue_response_(LOG_STR("String return"), resp); +} + void Nextion::process_serial_() { // Read all available bytes in batches to reduce UART call overhead. size_t avail = this->available(); @@ -653,28 +736,7 @@ void Nextion::process_nextion_commands_() { break; } - NextionQueue *nb = this->nextion_queue_.front(); - if (!nb || !nb->component) { - ESP_LOGE(TAG, "Invalid queue entry"); - if (nb != nullptr) { - this->release_queue_entry_(nb); - } - this->nextion_queue_.pop_front(); - return; - } - NextionComponentBase *component = nb->component; - - if (component->get_queue_type() != NextionQueueType::TEXT_SENSOR) { - ESP_LOGE(TAG, "String return but '%s' not text sensor", component->get_variable_name().c_str()); - } else { - ESP_LOGN(TAG, "String resp: '%s' id: %s type: %s", to_process.c_str(), component->get_variable_name().c_str(), - component->get_queue_type_string()); - component->set_state_from_string(to_process, true, false); - } - - this->release_queue_entry_(nb); - this->nextion_queue_.pop_front(); - + this->set_string_return_(to_process); break; } // 0x71 0x01 0x02 0x03 0x04 0xFF 0xFF 0xFF @@ -696,31 +758,7 @@ void Nextion::process_nextion_commands_() { int value = static_cast(encode_uint32(to_process[3], to_process[2], to_process[1], to_process[0])); - NextionQueue *nb = this->nextion_queue_.front(); - if (!nb || !nb->component) { - ESP_LOGE(TAG, "Invalid queue"); - if (nb != nullptr) { - this->release_queue_entry_(nb); - } - this->nextion_queue_.pop_front(); - return; - } - NextionComponentBase *component = nb->component; - - if (component->get_queue_type() != NextionQueueType::SENSOR && - component->get_queue_type() != NextionQueueType::BINARY_SENSOR && - component->get_queue_type() != NextionQueueType::SWITCH) { - ESP_LOGE(TAG, "Numeric return but '%s' invalid type %d", component->get_variable_name().c_str(), - component->get_queue_type()); - } else { - ESP_LOGN(TAG, "Numeric: %s type %d:%s val %d", component->get_variable_name().c_str(), - component->get_queue_type(), component->get_queue_type_string(), value); - component->set_state_from_int(value, true, false); - } - - this->release_queue_entry_(nb); - this->nextion_queue_.pop_front(); - + this->set_numeric_return_(value); break; } @@ -1154,6 +1192,8 @@ void Nextion::add_no_result_to_queue_with_command_(const std::string &variable_n if ((!this->is_setup() && !this->connection_state_.ignore_is_setup_) || command.empty()) return; + ESP_LOGV(TAG, "Queue NORESULT '%s': %s", variable_name.c_str(), command.c_str()); + if (this->send_command_(command)) { this->add_no_result_to_queue_(variable_name); #ifdef USE_NEXTION_COMMAND_SPACING diff --git a/esphome/components/nextion/nextion.h b/esphome/components/nextion/nextion.h index 320aec048a0..0e28b844411 100644 --- a/esphome/components/nextion/nextion.h +++ b/esphome/components/nextion/nextion.h @@ -1066,6 +1066,7 @@ class Nextion final : public NextionBase, public PollingComponent, public uart:: /** * Manually send a raw command to the display. + * A "get" command is accepted, but the returned value is dropped. Use a Nextion sensor or text sensor to read values. * @param command The pcommand, like "page 0" * @return Whether the send was successful. */ @@ -1073,6 +1074,7 @@ class Nextion final : public NextionBase, public PollingComponent, public uart:: /** * Manually send a raw formatted command to the display. + * A "get" command is accepted, but the returned value is dropped. Use a Nextion sensor or text sensor to read values. * @param format The printf-style command format, like "vis %s,0" * @param ... The format arguments * @return Whether the send was successful. @@ -1470,6 +1472,21 @@ class Nextion final : public NextionBase, public PollingComponent, public uart:: void release_queue_entry_(NextionQueue *nb); NextionQueue *make_no_result_entry_(const std::string &variable_name); + /// Payload for deliver_queue_response_(): either a numeric (0x71) or string (0x70) return value. + struct NextionQueueResponse { + bool is_string; + int int_value; + const std::string *str_value; + }; + /// Deliver a 0x70/0x71 response to the first query in the queue that was sent and expects a value, then + /// remove that query. NO_RESULT entries are skipped (their own 0x01 ACK removes them), as are entries whose + /// command is still waiting to be sent. Invalid entries are dropped. If the first query has the wrong type + /// (for example a numeric return for a TEXT_SENSOR), it is removed without a state update. A raw "get" sent + /// with send_command() counts as a query, and its value is dropped. + void deliver_queue_response_(const LogString *response_name, const NextionQueueResponse &resp); + void set_numeric_return_(int value); + void set_string_return_(const std::string &value); + /** * @brief Status flags for Nextion display state management * diff --git a/tests/components/nextion/__init__.py b/tests/components/nextion/__init__.py new file mode 100644 index 00000000000..b1b932e4d2c --- /dev/null +++ b/tests/components/nextion/__init__.py @@ -0,0 +1,12 @@ +from tests.testing_helpers import ComponentManifestOverride + + +def override_manifest(manifest: ComponentManifestOverride) -> None: + # nextion.h includes esphome/components/display/display.h and + # esphome/components/uart/uart.h. Declare both as test dependencies + # so their headers are in the include path during C++ unit test builds. + deps = list(manifest.dependencies or []) + for dep in ("display", "uart"): + if dep not in deps: + deps.append(dep) + manifest.dependencies = deps diff --git a/tests/components/nextion/deliver_queue_response_test.cpp b/tests/components/nextion/deliver_queue_response_test.cpp new file mode 100644 index 00000000000..166db4bffc0 --- /dev/null +++ b/tests/components/nextion/deliver_queue_response_test.cpp @@ -0,0 +1,243 @@ +#include + +#include +#include +#include +#include +#include + +#include "esphome/components/nextion/nextion.h" +#include "esphome/components/uart/uart_component.h" +#include "esphome/core/helpers.h" + +namespace esphome::nextion::testing { + +// Explicit instantiation skips access checks, which lets the test reach the queue and the response +// handlers without making them public on the component. +template Nextion::*Q, void (Nextion::*N)(int), void (Nextion::*S)(const std::string &), + void (Nextion::*R)(NextionQueue *), bool NextionBase::*U> +struct NextionAccess { + friend std::list &queue(Nextion &nextion) { return nextion.*Q; } + friend void numeric_return(Nextion &nextion, int value) { (nextion.*N)(value); } + friend void string_return(Nextion &nextion, const std::string &value) { (nextion.*S)(value); } + friend void release_queue_entry(Nextion &nextion, NextionQueue *entry) { (nextion.*R)(entry); } + friend void mark_setup(Nextion &nextion) { nextion.*U = true; } +}; +std::list &queue(Nextion &nextion); +void numeric_return(Nextion &nextion, int value); +void string_return(Nextion &nextion, const std::string &value); +void release_queue_entry(Nextion &nextion, NextionQueue *entry); +void mark_setup(Nextion &nextion); +template struct NextionAccess<&Nextion::nextion_queue_, &Nextion::set_numeric_return_, &Nextion::set_string_return_, + &Nextion::release_queue_entry_, &Nextion::is_setup_>; + +/// Accepts the commands the component writes; the display never answers +class FakeUART : public uart::UARTComponent { + public: + void write_array(const uint8_t *data, size_t len) override {} + bool peek_byte(uint8_t *data) override { return false; } + bool read_array(uint8_t *data, size_t len) override { return false; } + size_t available() override { return 0; } + uart::UARTFlushResult flush() override { return uart::UARTFlushResult::UART_FLUSH_RESULT_SUCCESS; } +#if defined(USE_ESP8266) || defined(USE_ESP32) + void load_settings(bool dump_config) override {} +#endif // defined(USE_ESP8266) || defined(USE_ESP32) + + protected: + void check_logger_conflict() override {} +}; + +/// Stands in for a sensor, switch or text sensor and records the value it receives +class MockComponent : public NextionComponentBase { + public: + MockComponent(const std::string &name, NextionQueueType type) : type_(type) { this->set_variable_name(name); } + + NextionQueueType get_queue_type() const override { return this->type_; } + void set_state_from_int(int state_value, bool publish, bool send_to_nextion) override { + this->received = true; + this->int_value = state_value; + } + void set_state_from_string(const std::string &state_value, bool publish, bool send_to_nextion) override { + this->received = true; + this->str_value = state_value; + } + + bool received{false}; + int int_value{0}; + std::string str_value; + + protected: + NextionQueueType type_; +}; + +class DeliverQueueResponse : public ::testing::Test { + protected: + void SetUp() override { + this->nextion_.set_uart_parent(&this->uart_); + mark_setup(this->nextion_); + } + + void TearDown() override { + for (auto *entry : queue(this->nextion_)) { + if (entry != nullptr) + release_queue_entry(this->nextion_, entry); + } + queue(this->nextion_).clear(); + } + + /// Queues a query the way add_to_get_queue() does. An unsent query still waits for command spacing. + MockComponent *add_query_(const std::string &name, NextionQueueType type, bool sent = true) { + auto *component = this->components_.emplace_back(std::make_unique(name, type)).get(); + auto *entry = RAMAllocator().allocate(1); + new (entry) NextionQueue; + entry->component = component; + if (!sent) + entry->pending_command = "get " + name; + queue(this->nextion_).push_back(entry); + return component; + } + + /// Names of the queued entries, front first. send_command() names its entries "command", or "raw_get" for a "get". + std::vector queued_() { + std::vector names; + for (auto *entry : queue(this->nextion_)) { + if (entry == nullptr || entry->component == nullptr) { + names.emplace_back(""); + } else { + names.push_back(entry->component->get_variable_name()); + } + } + return names; + } + + FakeUART uart_; + Nextion nextion_; + std::vector> components_; +}; + +// The regression: a numeric return must not consume a command that is still waiting for its ACK +TEST_F(DeliverQueueResponse, NumericSkipsNoResult) { + this->nextion_.send_command("page 0"); + auto *sensor = this->add_query_("temp", NextionQueueType::SENSOR); + + numeric_return(this->nextion_, 42); + + EXPECT_TRUE(sensor->received); + EXPECT_EQ(sensor->int_value, 42); + EXPECT_EQ(this->queued_(), std::vector{"command"}); +} + +TEST_F(DeliverQueueResponse, StringSkipsNoResult) { + this->nextion_.send_command("page 0"); + auto *text = this->add_query_("status", NextionQueueType::TEXT_SENSOR); + + string_return(this->nextion_, "online"); + + EXPECT_TRUE(text->received); + EXPECT_EQ(text->str_value, "online"); + EXPECT_EQ(this->queued_(), std::vector{"command"}); +} + +TEST_F(DeliverQueueResponse, NumericGoesToBinarySensorAndSwitch) { + auto *binary = this->add_query_("bin", NextionQueueType::BINARY_SENSOR); + auto *sw = this->add_query_("sw", NextionQueueType::SWITCH); + + numeric_return(this->nextion_, 1); + EXPECT_TRUE(binary->received); + EXPECT_EQ(binary->int_value, 1); + EXPECT_FALSE(sw->received); + + numeric_return(this->nextion_, 0); + EXPECT_TRUE(sw->received); + EXPECT_EQ(sw->int_value, 0); + EXPECT_TRUE(this->queued_().empty()); +} + +// With command spacing, a query can be queued before its command is sent; the display cannot be answering it yet +TEST_F(DeliverQueueResponse, NumericSkipsUnsentQuery) { + auto *unsent = this->add_query_("unsent", NextionQueueType::SENSOR, false); + auto *sent = this->add_query_("sent", NextionQueueType::SENSOR); + + numeric_return(this->nextion_, 7); + + EXPECT_FALSE(unsent->received); + EXPECT_TRUE(sent->received); + EXPECT_EQ(this->queued_(), std::vector{"unsent"}); +} + +TEST_F(DeliverQueueResponse, NumericWithoutSentQueryLeavesQueue) { + this->nextion_.send_command("page 0"); + auto *unsent = this->add_query_("temp", NextionQueueType::SENSOR, false); + + numeric_return(this->nextion_, 99); + + EXPECT_FALSE(unsent->received); + EXPECT_EQ(this->queued_(), (std::vector{"command", "temp"})); +} + +// The display answers queries in order, so a wrong-type return still belongs to the first sent query. +// It must consume only that query and never hand the value to a later one. +TEST_F(DeliverQueueResponse, WrongTypeConsumesOnlyFirstQuery) { + auto *text = this->add_query_("status", NextionQueueType::TEXT_SENSOR); + auto *sensor = this->add_query_("temp", NextionQueueType::SENSOR); + + numeric_return(this->nextion_, 100); + + EXPECT_FALSE(text->received); + EXPECT_FALSE(sensor->received); + EXPECT_EQ(this->queued_(), std::vector{"temp"}); +} + +TEST_F(DeliverQueueResponse, StringWrongTypeConsumesOnlyFirstQuery) { + auto *sensor = this->add_query_("temp", NextionQueueType::SENSOR); + auto *text = this->add_query_("status", NextionQueueType::TEXT_SENSOR); + + string_return(this->nextion_, "online"); + + EXPECT_FALSE(sensor->received); + EXPECT_FALSE(text->received); + EXPECT_EQ(this->queued_(), std::vector{"status"}); +} + +TEST_F(DeliverQueueResponse, InvalidEntriesAreDropped) { + auto *no_component = RAMAllocator().allocate(1); + new (no_component) NextionQueue; + queue(this->nextion_).push_back(nullptr); + queue(this->nextion_).push_back(no_component); + auto *sensor = this->add_query_("temp", NextionQueueType::SENSOR); + + numeric_return(this->nextion_, 5); + + EXPECT_TRUE(sensor->received); + EXPECT_TRUE(this->queued_().empty()); +} + +// The display answers a raw "get" with a value instead of 0x01, so that value removes it. +// The value must not reach a later query, and no orphan entry may stay behind for later ACKs to remove. +TEST_F(DeliverQueueResponse, RawGetTakesItsOwnValue) { + this->nextion_.send_command("get n0.val"); + this->nextion_.send_command_printf("get %s", "t0.txt"); + auto *sensor = this->add_query_("temp", NextionQueueType::SENSOR); + auto *text = this->add_query_("status", NextionQueueType::TEXT_SENSOR); + EXPECT_EQ(this->queued_(), (std::vector{"raw_get", "raw_get", "temp", "status"})); + + numeric_return(this->nextion_, 3); + string_return(this->nextion_, "raw"); + + EXPECT_FALSE(sensor->received); + EXPECT_FALSE(text->received); + EXPECT_EQ(this->queued_(), (std::vector{"temp", "status"})); +} + +// A raw "get" is found by its entry name; a query from a user entity with the same name is still a normal query +TEST_F(DeliverQueueResponse, QueryNamedRawGetIsNotRawGet) { + auto *sensor = this->add_query_("raw_get", NextionQueueType::SENSOR); + + numeric_return(this->nextion_, 8); + + EXPECT_TRUE(sensor->received); + EXPECT_EQ(sensor->int_value, 8); + EXPECT_TRUE(this->queued_().empty()); +} + +} // namespace esphome::nextion::testing