diff --git a/esphome/components/entity_transport/entity_transport.cpp b/esphome/components/entity_transport/entity_transport.cpp index 5da810695bc..5bc6b06d146 100644 --- a/esphome/components/entity_transport/entity_transport.cpp +++ b/esphome/components/entity_transport/entity_transport.cpp @@ -48,6 +48,8 @@ static constexpr uint32_t RETRY_DELAY_MS = 1000; static constexpr uint32_t RETRY_TIMER_ID = 0; static constexpr uint32_t TIMEOUT_CHECK_INTERVAL_MS = 1000; static constexpr uint32_t WARNING_INTERVAL_MS = 60000; +// How long the interface may refuse every packet before it is reported +static constexpr uint32_t REFUSED_WARNING_MS = 60000; static void put_uint32(uint8_t *buf, uint32_t value) { for (size_t i = 0; i != 4; i++) @@ -256,8 +258,31 @@ void EntityTransport::mark_updated_(LocalEntity &entity) { void EntityTransport::loop() { this->disable_loop(); // States are only sent once the dictionary that names them has been accepted - if ((this->dictionary_pending_ && !this->send_dictionary_()) || !this->send_updated_()) + if ((this->dictionary_pending_ && !this->send_dictionary_()) || !this->send_updated_()) { + this->sending_refused_(); this->set_timeout(RETRY_TIMER_ID, RETRY_DELAY_MS, [this] { this->enable_loop(); }); + return; + } + this->refusing_ = false; + if (this->status_has_warning()) { + ESP_LOGI(TAG, "Packets are accepted again"); + this->status_clear_warning(); + } +} + +// A link that is down or full refuses packets for a while; one that refuses them for long sends nothing at all +void EntityTransport::sending_refused_() { + uint32_t now = App.get_loop_component_start_time(); + if (!this->refusing_) { + this->refusing_ = true; + this->refused_since_ms_ = now; + return; + } + if (!this->status_has_warning() && now - this->refused_since_ms_ >= REFUSED_WARNING_MS) { + ESP_LOGW(TAG, "The packet interface has accepted no packets for %" PRIu32 "s, so no states are sent", + (now - this->refused_since_ms_) / 1000); + this->status_set_warning(); + } } size_t EntityTransport::write_header_(uint8_t type) const { @@ -285,8 +310,10 @@ bool EntityTransport::send_dictionary_() { bool new_section = type != section_type; if (length + (new_section ? SECTION_HEADER_SIZE : 0) + 1 + id_length > this->max_packet_size_) { buf[part_offset] = this->dictionary_part_ & PART_INDEX_MASK; - if (!this->interface_->send_to_interface(PacketBuffer(buf, length), {})) + if (!this->interface_->send_to_interface(PacketBuffer(buf, length), {})) { + ESP_LOGV(TAG, "Dictionary packet of %zu bytes not accepted, will retry", length); return false; + } this->dictionary_next_ = i; this->dictionary_part_++; length = header; @@ -306,8 +333,10 @@ bool EntityTransport::send_dictionary_() { } if (length != header) { buf[part_offset] = (this->dictionary_part_ & PART_INDEX_MASK) | PART_LAST; - if (!this->interface_->send_to_interface(PacketBuffer(buf, length), {})) + if (!this->interface_->send_to_interface(PacketBuffer(buf, length), {})) { + ESP_LOGV(TAG, "Dictionary packet of %zu bytes not accepted, will retry", length); return false; + } } this->dictionary_pending_ = false; this->dictionary_next_ = 0; @@ -436,8 +465,9 @@ void EntityTransport::receive_dictionary_(std::span packet, uint3 bool in_order = index == 0 || (this->rx_dictionary_id_ == dictionary_id && index == this->rx_next_part_); this->rx_dictionary_id_ = dictionary_id; this->rx_next_part_ = in_order ? (index + 1) & PART_INDEX_MASK : PART_NONE; - bool completed = in_order && (part & PART_LAST) != 0 && !this->dictionary_seen_(dictionary_id); - if (completed) { + bool completed = in_order && (part & PART_LAST) != 0; + bool first_time = completed && !this->dictionary_seen_(dictionary_id); + if (first_time) { this->seen_dictionaries_[this->seen_next_] = dictionary_id; this->seen_next_ = (this->seen_next_ + 1) % this->seen_dictionaries_.size(); } @@ -456,18 +486,22 @@ void EntityTransport::receive_dictionary_(std::span packet, uint3 pos += id_length; } } + // A provider may go back to a dictionary already seen, so every complete one is checked if (completed) - this->forget_unnamed_(provider, provider_length, dictionary_id); + this->forget_unnamed_(provider, provider_length, dictionary_id, first_time); } -void EntityTransport::forget_unnamed_(const char *provider, size_t provider_length, uint32_t dictionary_id) { +void EntityTransport::forget_unnamed_(const char *provider, size_t provider_length, uint32_t dictionary_id, + bool first_time) { // A remote entity that its provider's whole dictionary does not name would otherwise keep no or a stale state // silently #ifdef ENTITY_TRANSPORT_REMOTE_SENSOR_COUNT for (auto &remote : this->remote_sensors_) { if (!matches(remote.provider, provider, provider_length) || (remote.known && remote.dictionary_id == dictionary_id)) continue; - ESP_LOGW(TAG, "'%s' does not send '%s'", remote.provider, remote.remote_id); + if (remote.known || first_time) { + ESP_LOGW(TAG, "'%s' does not send '%s'", remote.provider, remote.remote_id); + } remote.known = false; if (remote.has_state) { remote.has_state = false; @@ -479,7 +513,9 @@ void EntityTransport::forget_unnamed_(const char *provider, size_t provider_leng for (auto &remote : this->remote_binary_sensors_) { if (!matches(remote.provider, provider, provider_length) || (remote.known && remote.dictionary_id == dictionary_id)) continue; - ESP_LOGW(TAG, "'%s' does not send '%s'", remote.provider, remote.remote_id); + if (remote.known || first_time) { + ESP_LOGW(TAG, "'%s' does not send '%s'", remote.provider, remote.remote_id); + } remote.known = false; if (remote.has_state) { remote.has_state = false; diff --git a/esphome/components/entity_transport/entity_transport.h b/esphome/components/entity_transport/entity_transport.h index b249c993714..2948264f517 100644 --- a/esphome/components/entity_transport/entity_transport.h +++ b/esphome/components/entity_transport/entity_transport.h @@ -53,7 +53,8 @@ namespace esphome::entity_transport { * Changed states are sent from loop(). The dictionary and all states are sent at boot, when a session layer reports * the peer connected, when the dictionary is requested, and every update interval. Unless the interface has both a * session and a reliability layer, the update interval defaults to DEFAULT_UPDATE_INTERVAL_MS. When the interface - * refuses a packet, sending is tried again after a delay. With a remote timeout, a remote entity loses its state when + * refuses a packet, sending is tried again after a delay, and a warning is set once it has refused every packet for a + * minute. With a remote timeout, a remote entity loses its state when * none arrives for it within the timeout. */ class EntityTransport : public Component { @@ -125,9 +126,11 @@ class EntityTransport : public Component { size_t id_length, uint32_t dictionary_id); void request_dictionary_(uint32_t dictionary_id); bool dictionary_seen_(uint32_t dictionary_id) const; - /// Warns about, and clears, each remote entity of \p provider that its complete dictionary does not name. - void forget_unnamed_(const char *provider, size_t provider_length, uint32_t dictionary_id); + /// Clears each remote entity of \p provider that its complete dictionary does not name, warning if the entity was + /// known or the dictionary is \p first_time seen, so a resent dictionary does not repeat the warning. + void forget_unnamed_(const char *provider, size_t provider_length, uint32_t dictionary_id, bool first_time); bool warning_due_(); + void sending_refused_(); void on_session_(bool connected); void clear_remote_states_(bool timed_out_only); @@ -148,6 +151,7 @@ class EntityTransport : public Component { uint32_t dictionary_id_{0}; uint32_t last_request_ms_{0}; uint32_t last_warning_ms_{0}; + uint32_t refused_since_ms_{0}; // when the interface began refusing every packet uint32_t rx_dictionary_id_{0}; // the dictionary whose parts are arriving // The last dictionary ids received in full, so a provider whose dictionary names none of our entities is not asked // again @@ -161,6 +165,7 @@ class EntityTransport : public Component { bool dictionary_pending_{true}; bool request_sent_{false}; bool warned_{false}; + bool refusing_{false}; }; } // namespace esphome::entity_transport diff --git a/tests/components/entity_transport/entity_transport.cpp b/tests/components/entity_transport/entity_transport.cpp index 4c2367c0a63..2bdd947f09f 100644 --- a/tests/components/entity_transport/entity_transport.cpp +++ b/tests/components/entity_transport/entity_transport.cpp @@ -184,6 +184,23 @@ TEST_F(EntityTransportTest, RefusedPacketSentAgain) { EXPECT_EQ(this->link_a_.sent.size(), 2); } +TEST_F(EntityTransportTest, LongRefusalWarns) { + this->link_a_.accept = false; + auto sender = this->make_sender_(); + { LoopBlockingGuard guard{nullptr, nullptr, 1000}; } + sender->loop(); + { LoopBlockingGuard guard{nullptr, nullptr, 60999}; } + sender->loop(); + EXPECT_FALSE(sender->status_has_warning()); + { LoopBlockingGuard guard{nullptr, nullptr, 61000}; } + sender->loop(); + EXPECT_TRUE(sender->status_has_warning()); + this->link_a_.accept = true; + sender->loop(); + EXPECT_FALSE(sender->status_has_warning()); + EXPECT_EQ(this->link_a_.sent.size(), 2); +} + TEST_F(EntityTransportTest, UnknownDictionaryRequested) { this->temp_.publish_state(5.0f); auto sender = this->make_sender_(); @@ -257,6 +274,31 @@ TEST_F(EntityTransportTest, EntityLeftOutOfNewDictionaryCleared) { EXPECT_FALSE(this->remote_door_.has_state()); } +TEST_F(EntityTransportTest, EntityLeftOutOfSeenDictionaryCleared) { + // The provider names only the sensor, then both entities, then goes back to naming only the sensor + sensor::Sensor temp; + MockInterface link; + auto make_temp_only = [&] { + auto transport = std::make_unique(&link); + transport->set_entity_count(1); + transport->add_sensor("temp", &temp); + transport->setup(); + return transport; + }; + temp.publish_state(4.0f); + auto receiver = this->make_receiver_(); + make_temp_only()->loop(); + link.deliver_to(this->link_b_); + this->door_.publish_state(true); + this->make_sender_()->loop(); + this->link_a_.deliver_to(this->link_b_); + EXPECT_TRUE(this->remote_door_.state); + make_temp_only()->loop(); + link.deliver_to(this->link_b_); + EXPECT_EQ(this->remote_temp_.get_state(), 4.0f); + EXPECT_FALSE(this->remote_door_.has_state()); +} + TEST_F(EntityTransportTest, LostDictionaryPartRequested) { // One entity in each dictionary packet MockInterface link(5 + 2 + strlen(DEVICE_NAME) + 3 + 5);