mirror of
https://github.com/esphome/esphome.git
synced 2026-10-01 00:40:21 +00:00
Tighten review-round comments to repo style
This commit is contained in:
@@ -50,9 +50,8 @@ void BluedroidGattClient::setup() {
|
||||
|
||||
void BluedroidGattClient::loop() {
|
||||
if (!esp32_ble::global_ble->is_active()) {
|
||||
// Stack down: no CLOSE_EVT will ever come. Settle a link that was past
|
||||
// IDLE so the consumer frees its slot instead of holding a phantom
|
||||
// connection, then re-register the app on the next enable.
|
||||
// Stack down: no CLOSE_EVT will come. Settle a live link so the consumer
|
||||
// frees its slot, then re-register the app on the next enable.
|
||||
auto down_st = this->state();
|
||||
if (down_st != ClientState::IDLE && down_st != ClientState::INIT) {
|
||||
this->release_services();
|
||||
@@ -72,20 +71,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::DISCONNECTING || this->want_disconnect_) {
|
||||
// The one teardown safety net (the wrapper has no timer): covers a lost
|
||||
// CLOSE_EVT and a scheduled teardown whose OPEN_EVT never arrives.
|
||||
// The one teardown safety net: a lost CLOSE_EVT, or a scheduled
|
||||
// teardown whose OPEN_EVT never arrives.
|
||||
if (millis() - this->disconnecting_started_ > ble_device_base::GATT_DISCONNECT_TIMEOUT_MS) {
|
||||
ESP_LOGE(TAG, "[%d] Timeout waiting for teardown, forcing IDLE", this->connection_index_);
|
||||
// Release before idling: unconditional disconnect does not release,
|
||||
// and a lost completion would otherwise leak the table and the cache.
|
||||
// Release before idling: a lost completion must not leak the cache.
|
||||
this->release_services();
|
||||
this->set_idle_(); // also clears want_disconnect_
|
||||
this->listener_->on_connection_state(false, 0, ESP_GATT_CONN_TIMEOUT);
|
||||
}
|
||||
} else {
|
||||
// While a link exists the loop stays on watching for a stack-down (the
|
||||
// settle above needs a tick to run) and flushing a pre-started search
|
||||
// claimed outside an event drain; it settles only back at IDLE.
|
||||
// The loop stays on while a link exists (stack-down watch, pre-started
|
||||
// search flush); it settles only back at IDLE.
|
||||
this->deliver_pending_search_();
|
||||
if (this->state() == ClientState::IDLE) {
|
||||
this->disable_loop();
|
||||
@@ -131,9 +128,8 @@ void BluedroidGattClient::tracker_connect_() {
|
||||
this->services_released_ = false;
|
||||
this->seen_mtu_ = false;
|
||||
this->mtu_failed_ = false;
|
||||
// Per-attempt reset: the stack-down path in loop() reaches IDLE through
|
||||
// set_state() without set_idle_(), and a stale completed search would
|
||||
// satisfy this connection's discovery with the previous one's result.
|
||||
// Per-attempt reset: the stack-down path reaches IDLE without set_idle_(),
|
||||
// and a stale result must not satisfy this attempt's discovery.
|
||||
this->search_state_ = SearchState::NONE;
|
||||
this->search_status_ = 0;
|
||||
this->enable_loop();
|
||||
@@ -174,8 +170,7 @@ int BluedroidGattClient::gatt_disconnect() {
|
||||
if (st == ClientState::CONNECTING || this->conn_id_ == UNSET_CONN_ID) {
|
||||
ESP_LOGD(TAG, "[%d] Disconnect scheduled", this->connection_index_);
|
||||
this->want_disconnect_ = true;
|
||||
// The backend owns the whole safety window (the wrapper has no timer):
|
||||
// a lost OPEN_EVT must not leak the scheduled teardown.
|
||||
// Arm the safety window: a lost OPEN_EVT must not leak the teardown.
|
||||
this->disconnecting_started_ = millis();
|
||||
this->enable_loop();
|
||||
return 0;
|
||||
@@ -448,8 +443,8 @@ bool BluedroidGattClient::build_service_table_() {
|
||||
return true;
|
||||
});
|
||||
if (!filled || char_index != char_total || desc_index != desc_total) {
|
||||
// A walk error or a database that changed between the two passes; the
|
||||
// consumer sees an empty table rather than a corrupt one.
|
||||
// Walk error or the database changed between passes; better an empty
|
||||
// table than a corrupt one.
|
||||
ESP_LOGW(TAG, "[%d] Service table walk mismatch, discarding", this->connection_index_);
|
||||
this->free_service_table_();
|
||||
return false;
|
||||
@@ -507,8 +502,8 @@ void BluedroidGattClient::log_gattc_warning_(const char *operation, int code) {
|
||||
|
||||
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.
|
||||
// Re-discovery moves the counts table_view_() derives offsets from; free
|
||||
// the stale table.
|
||||
this->free_service_table_();
|
||||
#endif
|
||||
// Step down from the fast discovery params.
|
||||
@@ -529,10 +524,8 @@ int BluedroidGattClient::handle_search_cmpl_() {
|
||||
return 0;
|
||||
}
|
||||
|
||||
// Reports a completed search once it has a claimant; a pre-started search
|
||||
// stays silent until claimed. Delivery consumes the state, so a later
|
||||
// discover_services() on the same connection issues a real search instead
|
||||
// of re-reporting this result.
|
||||
// Reports a completed search once claimed; delivery consumes the state so
|
||||
// a re-discovery issues a real search.
|
||||
void BluedroidGattClient::deliver_pending_search_() {
|
||||
if (this->search_state_ != SearchState::REPORT_PENDING)
|
||||
return;
|
||||
@@ -733,8 +726,8 @@ void BluedroidGattClient::handle_open_evt_(esp_ble_gattc_cb_param_t *param) {
|
||||
this->search_state_ = SearchState::PRESTARTED;
|
||||
}
|
||||
if (this->mtu_failed_ && !this->seen_mtu_) {
|
||||
// The MTU request was refused at CONNECT_EVT: report here with the
|
||||
// default so the consumer proceeds instead of waiting forever.
|
||||
// Refused MTU request: report with the default so the consumer
|
||||
// proceeds.
|
||||
this->seen_mtu_ = true;
|
||||
this->listener_->on_connection_state(true, ble_device_base::DEFAULT_ATT_MTU, 0);
|
||||
this->deliver_pending_search_();
|
||||
@@ -785,8 +778,7 @@ bool BluedroidGattClient::gattc_event_handler(esp_gattc_cb_event_t event, esp_ga
|
||||
auto ret = esp_ble_gattc_send_mtu_req(this->gattc_if_, param->connect.conn_id);
|
||||
if (ret) {
|
||||
this->log_gattc_warning_("esp_ble_gattc_send_mtu_req", ret);
|
||||
// No CFG_MTU_EVT will follow: OPEN_EVT reports with the default MTU
|
||||
// so the connection still reaches a reported state.
|
||||
// No CFG_MTU_EVT will follow; OPEN_EVT reports with the default.
|
||||
this->mtu_failed_ = true;
|
||||
}
|
||||
break;
|
||||
@@ -836,9 +828,8 @@ bool BluedroidGattClient::gattc_event_handler(esp_gattc_cb_event_t event, esp_ga
|
||||
return false;
|
||||
ESP_LOGI(TAG, "[%d] Service discovery complete", this->connection_index_);
|
||||
if (this->state() == ClientState::DISCONNECTING) {
|
||||
// Teardown already owns the link: keep its state and safety timer,
|
||||
// and skip the param/count work - the result is never delivered (the
|
||||
// terminal connected=false report settles the consumer).
|
||||
// Teardown owns the link; the result is never delivered, skip the
|
||||
// work.
|
||||
break;
|
||||
}
|
||||
this->search_status_ = this->handle_search_cmpl_();
|
||||
@@ -894,9 +885,8 @@ void BluedroidGattClient::gap_event_handler(esp_gap_ble_cb_event_t event, esp_bl
|
||||
case ESP_GAP_BLE_SEC_REQ_EVT: {
|
||||
if (!this->check_addr_(param->ble_security.auth_cmpl.bd_addr))
|
||||
break;
|
||||
// Always accept a server-initiated security request. A refused
|
||||
// response means no AUTH_CMPL will follow, so answer the pairing
|
||||
// request with the failure instead of hanging it.
|
||||
// Always accept; a refused response means no AUTH_CMPL, so answer the
|
||||
// pairing request with the failure.
|
||||
int sec_err = this->check_and_log_error_("esp_ble_gap_security_rsp",
|
||||
esp_ble_gap_security_rsp(param->ble_security.ble_req.bd_addr, true));
|
||||
if (sec_err != 0) {
|
||||
|
||||
@@ -146,10 +146,8 @@ class BluedroidGattClient final : public esp32_ble_tracker::ESPBTClient, public
|
||||
bool seen_mtu_ : 1 {false};
|
||||
// The MTU request was refused at CONNECT_EVT; OPEN_EVT reports instead.
|
||||
bool mtu_failed_ : 1 {false};
|
||||
// Pre-started discovery: 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. Reset per attempt and on idle.
|
||||
// Search issued at OPEN_EVT overlaps the MTU exchange; discover_services()
|
||||
// completes from it. Reset per attempt and on idle.
|
||||
SearchState search_state_ : 4 {SearchState::NONE};
|
||||
// esp_gatt_status_t of the completed search, held until claimed.
|
||||
uint8_t search_status_{0};
|
||||
|
||||
@@ -46,10 +46,8 @@ void BluetoothConnection::disconnect() {
|
||||
}
|
||||
int err = this->backend_->gatt_disconnect();
|
||||
if (err != 0) {
|
||||
// Both backends return nonzero only when there is nothing to tear down
|
||||
// (already idle): free the slot so the client is not stuck. Accepted
|
||||
// teardowns always reach a terminal report - the backend owns the
|
||||
// safety timer on every path.
|
||||
// Nonzero means nothing to tear down (both backends): free the slot.
|
||||
// Accepted teardowns always reach a terminal report.
|
||||
ESP_LOGW(TAG, "[%d] [%s] disconnect while backend idle, err=%d", this->connection_index_, this->address_str_, err);
|
||||
this->reset_connection_(err);
|
||||
return;
|
||||
|
||||
@@ -41,9 +41,8 @@ class BluetoothConnection final : public ble_device_base::GattClientListener {
|
||||
conn_err_t notify_characteristic(uint16_t handle, bool enable);
|
||||
conn_err_t update_connection_params(uint16_t min_interval, uint16_t max_interval, uint16_t latency, uint16_t timeout);
|
||||
|
||||
/// Streamer abort: latch the GATT cause for the disconnect report (first
|
||||
/// cause wins, matching disconnect()), park the stream cursor, and tear
|
||||
/// the connection down.
|
||||
/// Streamer abort: latch the GATT cause (first wins), park the cursor,
|
||||
/// tear down.
|
||||
void abort_service_stream(conn_err_t err) {
|
||||
if (this->pending_error_ == 0) {
|
||||
this->pending_error_ = err;
|
||||
@@ -81,9 +80,8 @@ class BluetoothConnection final : public ble_device_base::GattClientListener {
|
||||
// an authoritative empty database).
|
||||
bool has_gatt_services() const { return this->services_discovered_; }
|
||||
|
||||
/// Stream any pending service-discovery batch. Called from the proxy's
|
||||
/// loop — the wrapper has no Component loop of its own. The disconnect
|
||||
/// safety timer lives in the backend, which owns every teardown path.
|
||||
/// Stream any pending service-discovery batch (proxy loop; the backend
|
||||
/// owns the disconnect safety timer).
|
||||
void process_pending_services() {
|
||||
if (this->send_service_ >= 0) {
|
||||
this->stream_pending_(this->backend_);
|
||||
@@ -137,8 +135,7 @@ class BluetoothConnection final : public ble_device_base::GattClientListener {
|
||||
// Group 4: Arrays
|
||||
char address_str_[MAC_ADDRESS_PRETTY_BUFFER_SIZE]{};
|
||||
|
||||
// Group 5: bit-packed tail. address_ makes the object 8-aligned, so this
|
||||
// group must stay within 2 bytes to keep the wrapper at 48 (dev parity).
|
||||
// Group 5: bit-packed tail; within 2 bytes the 8-aligned object stays 48.
|
||||
ClientState state_ : 3 {ClientState::IDLE};
|
||||
bool paired_ : 1 {false};
|
||||
ConnectionType connection_type_ : 2 {ConnectionType::V1};
|
||||
|
||||
Reference in New Issue
Block a user