diff --git a/esphome/components/entity_transport/entity_transport.cpp b/esphome/components/entity_transport/entity_transport.cpp index 40d1eb8a99f..f1eacbe1c99 100644 --- a/esphome/components/entity_transport/entity_transport.cpp +++ b/esphome/components/entity_transport/entity_transport.cpp @@ -298,6 +298,10 @@ void EntityTransport::receive_dictionary_(std::span packet, uint3 size_t pos = HEADER_SIZE; if (pos == packet.size() || pos + 1 + packet[pos] > packet.size()) return; + if (!this->dictionary_seen_(dictionary_id)) { + this->seen_dictionaries_[this->seen_next_] = dictionary_id; + this->seen_next_ = (this->seen_next_ + 1) % this->seen_dictionaries_.size(); + } size_t provider_length = packet[pos++]; const auto *provider = reinterpret_cast(packet.data() + pos); pos += provider_length; @@ -345,24 +349,30 @@ void EntityTransport::learn_(const char *provider, size_t provider_length, uint8 #endif } +bool EntityTransport::dictionary_seen_(uint32_t dictionary_id) const { + return std::find(this->seen_dictionaries_.begin(), this->seen_dictionaries_.end(), dictionary_id) != + this->seen_dictionaries_.end(); +} + void EntityTransport::receive_data_(std::span packet, uint32_t dictionary_id) { bool known = false; - bool unresolved = false; + bool has_remotes = false; #ifdef ENTITY_TRANSPORT_REMOTE_SENSOR_COUNT for (const auto &remote : this->remote_sensors_) { known |= remote.known && remote.dictionary_id == dictionary_id; - unresolved |= !remote.known; + has_remotes = true; } #endif #ifdef ENTITY_TRANSPORT_REMOTE_BINARY_SENSOR_COUNT for (const auto &remote : this->remote_binary_sensors_) { known |= remote.known && remote.dictionary_id == dictionary_id; - unresolved |= !remote.known; + has_remotes = true; } #endif if (!known) { - // The packet may come from the provider of an entity whose number is not yet known - if (unresolved) + // A dictionary not yet received may name our entities, or be a provider's new one; one already received named + // none of them, so asking again would only make that provider resend everything + if (has_remotes && !this->dictionary_seen_(dictionary_id)) this->request_dictionary_(dictionary_id); return; } diff --git a/esphome/components/entity_transport/entity_transport.h b/esphome/components/entity_transport/entity_transport.h index b1c0a88c91f..2bec0dadffe 100644 --- a/esphome/components/entity_transport/entity_transport.h +++ b/esphome/components/entity_transport/entity_transport.h @@ -1,5 +1,6 @@ #pragma once +#include #include #include "esphome/components/packet_interface/packet_interface.h" @@ -39,7 +40,8 @@ namespace esphome::entity_transport { * - sensor records: number (1 byte), then the state as a 4 byte float; * - binary sensor records, to the end: number (1 byte), then 0, 1, or 2 for no state. * - * Dictionary request packet, sent by a device that receives data with a dictionary id it does not know: + * Dictionary request packet, sent by a device that receives data with a dictionary id it has not received a dictionary + * for: * - type: 1 byte, PACKET_REQUEST; * - the dictionary id wanted: 4 bytes. * @@ -116,6 +118,7 @@ class EntityTransport : public Component { void learn_(const char *provider, size_t provider_length, uint8_t type, uint8_t number, const char *id, size_t id_length, uint32_t dictionary_id); void request_dictionary_(uint32_t dictionary_id); + bool dictionary_seen_(uint32_t dictionary_id) const; void on_session_(bool connected); void clear_remote_states_(bool timed_out_only); @@ -135,6 +138,9 @@ class EntityTransport : public Component { uint32_t remote_timeout_ms_{0}; uint32_t dictionary_id_{0}; uint32_t last_request_ms_{0}; + // The last dictionary ids received, so a provider whose dictionary names none of our entities is not asked again + std::array seen_dictionaries_{}; + uint8_t seen_next_{0}; uint8_t sensor_count_{0}; uint8_t binary_sensor_count_{0}; size_t dictionary_next_{0}; // the first entity of the next dictionary packet to send diff --git a/tests/components/entity_transport/entity_transport.cpp b/tests/components/entity_transport/entity_transport.cpp index bf585f051e4..2bd89fcfd0f 100644 --- a/tests/components/entity_transport/entity_transport.cpp +++ b/tests/components/entity_transport/entity_transport.cpp @@ -186,6 +186,32 @@ TEST_F(EntityTransportTest, UnknownDictionaryRequested) { EXPECT_EQ(this->remote_temp_.get_state(), 5.0f); } +TEST_F(EntityTransportTest, ChangedDictionaryRequested) { + this->temp_.publish_state(5.0f); + auto sender = this->make_sender_(); + auto receiver = this->make_receiver_(); + sender->loop(); + this->link_a_.deliver_to(this->link_b_); + EXPECT_EQ(this->remote_temp_.get_state(), 5.0f); + // The provider restarts with other entities, and its new dictionary is lost + sensor::Sensor temp; + MockInterface link; + TestTransport changed(&link); + changed.set_entity_count(1); + changed.add_sensor("temp", &temp); + changed.setup(); + temp.publish_state(6.0f); + changed.loop(); + link.sent.erase(link.sent.begin()); + link.deliver_to(this->link_b_); + ASSERT_EQ(this->link_b_.sent.size(), 1); + EXPECT_EQ(this->link_b_.sent[0], header(PACKET_REQUEST, changed.get_dictionary_id())); + this->link_b_.deliver_to(link); + changed.loop(); + link.deliver_to(this->link_b_); + EXPECT_EQ(this->remote_temp_.get_state(), 6.0f); +} + TEST_F(EntityTransportTest, RequestsLimited) { auto receiver = this->make_receiver_(); Packet data = header(PACKET_DATA, 0x12345678); @@ -203,6 +229,8 @@ TEST_F(EntityTransportTest, OtherProviderAndOwnPacketsIgnored) { auto packets = this->link_a_.sent; this->link_a_.deliver_to(this->link_b_); EXPECT_FALSE(this->remote_temp_.has_state()); + // The dictionary named none of the receiver's entities, so it is not asked for again + EXPECT_TRUE(this->link_b_.sent.empty()); for (auto &packet : packets) this->link_a_.inject(packet); this->link_a_.inject(header(PACKET_REQUEST, 0x12345678));