github/esphome
2
0
Fork 0
mirror of https://github.com/esphome/esphome.git synced 2026-08-24 07:06:20 +00:00
Code Issues Packages Projects Releases Wiki Activity

Restore the discovery/MTU overlap and latch streamer abort causes

Browse Source 
The split serialized service discovery behind the MTU exchange: the
backend reported connected only at CFG_MTU_EVT and the wrapper started
discovery on that report, costing one ATT round trip per uncached
connection. The backend now pre-starts the search at OPEN_EVT (it knows
the connection type) and completes the consumer's discover_services()
from it: SEARCH_CMPL latches silently until requested, the flush after
the connected report delivers in the same event drain, and a refused
pre-start falls back to the serialized path. Net RAM cost is zero (the
flags pack into one bitfield byte plus a status byte, replacing the two
existing bools).

Also from review: SEARCH_CMPL no longer clobbers a teardown in progress
with ESTABLISHED, the five streamer abort sites latch pending_error_ so
HA sees the real Bluedroid status instead of a generic HCI reason, and a
completed re-discovery frees a materialized service table before the
counts its offsets derive from move.
This commit is contained in:
J. Nick Koston
2026-08-09 09:52:58 -05:00
parent 4a9e5f5da9
commit 084ff9d6e3
2 changed files with 93 additions and 16 deletions
Download Patch File Download Diff File Expand all files Collapse all files
esphome/components/bluetooth_connection/bluetooth_connection_bluedroid.cpp
+79 -13
View File
@@ -64,10 +64,18 @@ void BluedroidGattClient::loop() {
// Do not wait for REG_EVT; a dropped event must not wedge the slot.
this->set_state(ClientState::IDLE);
} else if (st == ClientState::IDLE) {
// The loop only drives the bootstrap and the disconnect safety timeout.
// The loop only drives the bootstrap, the disconnect safety timeout and
// the pre-started-search flush.
this->disable_loop();
} else if (st == ClientState::DISCONNECTING &&
millis() - this->disconnecting_started_ > ble_device_base::GATT_DISCONNECT_TIMEOUT_MS) {
} else if (st != ClientState::DISCONNECTING) {
// Only the pre-started-search flush can need the loop here (a consumer
// requesting the finished search outside an event drain). Settle again -
// unless delivering it just started a teardown that needs the timer.
this->deliver_pending_search_();
if (this->state() != ClientState::DISCONNECTING) {
this->disable_loop();
}
} else if (millis() - this->disconnecting_started_ > ble_device_base::GATT_DISCONNECT_TIMEOUT_MS) {
ESP_LOGE(TAG, "[%d] Timeout waiting for CLOSE_EVT, forcing IDLE", this->connection_index_);
// Release before idling: unconditional disconnect does not release, and a
// lost CLOSE/DISCONNECT would otherwise leak the table and the cache.
@@ -176,8 +184,22 @@ int BluedroidGattClient::discover_services() {
if (this->conn_id_ == UNSET_CONN_ID) {
return ble_device_base::GATT_ERR_NOT_CONNECTED;
}
return this->check_and_log_error_("esp_ble_gattc_search_service",
esp_ble_gattc_search_service(this->gattc_if_, this->conn_id_, nullptr));
this->search_requested_ = true;
if (this->search_prestarted_) {
// Completion comes from the pre-started search: the pending SEARCH_CMPL,
// or - when it already landed - the flush after the connected report
// (loop() covers a request made outside that event drain).
if (this->search_done_) {
this->enable_loop();
}
return 0;
}
int err = this->check_and_log_error_("esp_ble_gattc_search_service",
esp_ble_gattc_search_service(this->gattc_if_, this->conn_id_, nullptr));
if (err != 0) {
this->search_requested_ = false;
}
return err;
}
int BluedroidGattClient::read_characteristic(uint16_t handle) {
@@ -419,6 +441,10 @@ bool BluedroidGattClient::check_addr_(const esp_bd_addr_t &addr) const {
void BluedroidGattClient::set_idle_() {
this->set_state(ClientState::IDLE);
this->conn_id_ = UNSET_CONN_ID;
this->search_prestarted_ = false;
this->search_done_ = false;
this->search_requested_ = false;
this->search_status_ = 0;
}
void BluedroidGattClient::set_disconnecting_() {
@@ -453,7 +479,12 @@ void BluedroidGattClient::log_gattc_warning_(const char *operation, int code) {
// ---- service streaming ----
void BluedroidGattClient::handle_search_cmpl_() {
int BluedroidGattClient::handle_search_cmpl_() {
#ifdef USE_BLE_GATT_SERVICE_TABLE
// A completed re-discovery moves the counts table_view_() derives its
// offsets from; a table built from the old ones must not survive it.
this->free_service_table_();
#endif
// Step down from the fast discovery params.
this->update_conn_params_(MEDIUM_MIN_CONN_INTERVAL, MEDIUM_MAX_CONN_INTERVAL, 0, MEDIUM_CONN_TIMEOUT, "medium");
uint16_t primary = 0;
@@ -466,11 +497,20 @@ void BluedroidGattClient::handle_search_cmpl_() {
// A failed count must not become an authoritative empty database.
auto status = primary_status != ESP_GATT_OK ? primary_status : secondary_status;
this->log_gattc_warning_("esp_ble_gattc_get_attr_count", status);
this->listener_->on_service_discovery_done(status);
return;
return status;
}
this->service_total_ = primary + secondary;
this->listener_->on_service_discovery_done(0);
return 0;
}
// Reports a completed search once its consumer has asked for it; the
// pre-started search must stay silent until then.
bool BluedroidGattClient::deliver_pending_search_() {
if (!this->search_requested_ || !this->search_done_)
return false;
this->search_requested_ = false;
this->listener_->on_service_discovery_done(this->search_status_);
return true;
}
#ifdef USE_BLUETOOTH_PROXY
@@ -508,11 +548,13 @@ void BluedroidGattClient::stream_service_batch(BluetoothConnection &conn) {
while (conn.send_service_ < this->service_total_) {
esp_gattc_service_elem_t service_result;
uint16_t svc_count = 1;
if (esp_ble_gattc_get_service(this->gattc_if_, this->conn_id_, nullptr, &service_result, &svc_count,
conn.send_service_) != ESP_GATT_OK ||
svc_count == 0) {
esp_gatt_status_t svc_status = esp_ble_gattc_get_service(this->gattc_if_, this->conn_id_, nullptr, &service_result,
&svc_count, conn.send_service_);
if (svc_status != ESP_GATT_OK || svc_count == 0) {
ESP_LOGE(TAG, "[%d] [%s] Service walk failed (service %d), aborting stream", conn.connection_index_,
conn.address_str_, conn.send_service_);
// Latch the real cause for the disconnect report.
conn.pending_error_ = svc_status != ESP_GATT_OK ? svc_status : ESP_GATT_NOT_FOUND;
conn.send_service_ = DONE_SENDING_SERVICES;
conn.disconnect();
return;
@@ -523,6 +565,7 @@ void BluedroidGattClient::stream_service_batch(BluetoothConnection &conn) {
service_result.start_handle, service_result.end_handle, 0, &total_char_count);
if (char_count_status != ESP_GATT_OK) {
this->log_gattc_warning_("esp_ble_gattc_get_attr_count", char_count_status);
conn.pending_error_ = char_count_status;
conn.send_service_ = DONE_SENDING_SERVICES;
conn.disconnect();
return;
@@ -556,6 +599,7 @@ void BluedroidGattClient::stream_service_batch(BluetoothConnection &conn) {
if (char_status != ESP_GATT_OK || cc == 0) {
if (char_status != ESP_GATT_OK) {
this->log_gattc_warning_("esp_ble_gattc_get_all_char", char_status);
conn.pending_error_ = char_status;
conn.send_service_ = DONE_SENDING_SERVICES;
conn.disconnect();
return;
@@ -576,6 +620,7 @@ void BluedroidGattClient::stream_service_batch(BluetoothConnection &conn) {
// Abort rather than stream the characteristic descriptor-less: a
// missing CCCD in a cached database breaks notifications for good.
this->log_gattc_warning_("esp_ble_gattc_get_attr_count", desc_count_status);
conn.pending_error_ = desc_count_status;
conn.send_service_ = DONE_SENDING_SERVICES;
conn.disconnect();
return;
@@ -594,6 +639,7 @@ void BluedroidGattClient::stream_service_batch(BluetoothConnection &conn) {
if (desc_status != ESP_GATT_OK || dc == 0) {
if (desc_status != ESP_GATT_OK) {
this->log_gattc_warning_("esp_ble_gattc_get_all_descr", desc_status);
conn.pending_error_ = desc_status;
conn.send_service_ = DONE_SENDING_SERVICES;
conn.disconnect();
return;
@@ -666,6 +712,16 @@ void BluedroidGattClient::handle_open_evt_(esp_ble_gattc_cb_param_t *param) {
// Settled; set_disconnecting_() re-enables the loop for the net.
this->disable_loop();
}
} else {
// Discovery-bound connection: start the search now so it overlaps the
// MTU exchange. On a refusal fall back to the serialized path - the
// consumer's own discover_services() call retries the real search.
auto ret = esp_ble_gattc_search_service(this->gattc_if_, this->conn_id_, nullptr);
if (ret == ESP_OK) {
this->search_prestarted_ = true;
} else {
this->log_gattc_warning_("esp_ble_gattc_search_service", ret);
}
}
}
@@ -733,6 +789,9 @@ bool BluedroidGattClient::gattc_event_handler(esp_gattc_cb_event_t event, esp_ga
// The connected report waited for the MTU; forwarded, not stored.
this->listener_->on_connection_state(
true, param->cfg_mtu.status == ESP_GATT_OK ? param->cfg_mtu.mtu : ble_device_base::DEFAULT_ATT_MTU, 0);
// The consumer requests discovery from inside that report; when the
// pre-started search already finished, complete it in the same drain.
this->deliver_pending_search_();
}
break;
}
@@ -756,8 +815,15 @@ bool BluedroidGattClient::gattc_event_handler(esp_gattc_cb_event_t event, esp_ga
if (this->conn_id_ != param->search_cmpl.conn_id)
return false;
ESP_LOGI(TAG, "[%d] Service discovery complete", this->connection_index_);
this->search_done_ = true;
this->search_status_ = this->handle_search_cmpl_();
if (this->state() == ClientState::DISCONNECTING) {
// Teardown already owns the link: keep its state and safety timer;
// the terminal connected=false report settles the consumer.
break;
}
this->set_state(ClientState::ESTABLISHED);
this->handle_search_cmpl_();
this->deliver_pending_search_();
if (this->state() != ClientState::DISCONNECTING) {
// Settled; a failed count started a teardown that needs the loop.
this->disable_loop();
esphome/components/bluetooth_connection/bluetooth_connection_bluedroid.h
+14 -3
View File
@@ -84,7 +84,8 @@ class BluedroidGattClient final : public esp32_ble_tracker::ESPBTClient, public
void tracker_connect_();
void handle_open_evt_(esp_ble_gattc_cb_param_t *param);
void handle_disconnect_evt_(esp_ble_gattc_cb_param_t *param);
void handle_search_cmpl_();
int handle_search_cmpl_();
bool deliver_pending_search_();
void unconditional_disconnect_();
void set_idle_();
void set_disconnecting_();
@@ -130,10 +131,20 @@ class BluedroidGattClient final : public esp32_ble_tracker::ESPBTClient, public
uint8_t connection_index_;
// Terminates an in-flight stream (never send a partial list as authoritative)
// and marks a cleaned cache unsafe to walk (Bluedroid asserts).
bool services_released_{false};
bool services_released_ : 1 {false};
// The connected report waits for the MTU exchange; OPEN_EVT alone would
// hand HA the default 23.
bool seen_mtu_{false};
bool seen_mtu_ : 1 {false};
// Pre-started discovery latch: the search is issued at OPEN_EVT so it
// overlaps the MTU exchange (the replaced class's timing) and the
// consumer's discover_services() completes from it instead of paying a
// serialized ATT round trip. requested/done meet in
// deliver_pending_search_().
bool search_prestarted_ : 1 {false};
bool search_done_ : 1 {false};
bool search_requested_ : 1 {false};
// esp_gatt_status_t of the completed search, held until requested.
uint8_t search_status_{0};
};
} // namespace esphome::bluetooth_connection