diff --git a/esphome/components/bluetooth_proxy/bluetooth_proxy.cpp b/esphome/components/bluetooth_proxy/bluetooth_proxy.cpp index 3cb4d9c0ad..b5cb8be359 100644 --- a/esphome/components/bluetooth_proxy/bluetooth_proxy.cpp +++ b/esphome/components/bluetooth_proxy/bluetooth_proxy.cpp @@ -128,6 +128,11 @@ BluetoothConnection *BluetoothProxy::get_connection_(uint64_t address, bool rese for (auto *connection : this->connections_) { if (connection->get_address() == 0) { connection->set_address(address); + // All connections must start at INIT + // We only set the state if we allocate the connection + // to avoid a race where multiple connection attempts + // are made. + connection->set_state(espbt::ClientState::INIT); return connection; } } @@ -144,8 +149,19 @@ void BluetoothProxy::bluetooth_device_request(const api::BluetoothDeviceRequest api::global_api_server->send_bluetooth_device_connection(msg.address, false); return; } - ESP_LOGV(TAG, "[%d] [%s] Searching to connect", connection->get_connection_index(), - connection->address_str().c_str()); + if (connection->state() == espbt::ClientState::CONNECTED || + connection->state() == espbt::ClientState::ESTABLISHED) { + ESP_LOGW(TAG, "[%d] [%s] Connection already established", connection->get_connection_index(), + connection->address_str().c_str()); + api::global_api_server->send_bluetooth_device_connection(msg.address, true); + api::global_api_server->send_bluetooth_connections_free(this->get_bluetooth_connections_free(), + this->get_bluetooth_connections_limit()); + return; + } else if (connection->state() != espbt::ClientState::INIT) { + ESP_LOGW(TAG, "[%d] [%s] Connection already in progress", connection->get_connection_index(), + connection->address_str().c_str()); + return; + } connection->set_state(espbt::ClientState::SEARCHING); api::global_api_server->send_bluetooth_connections_free(this->get_bluetooth_connections_free(), this->get_bluetooth_connections_limit()); diff --git a/esphome/components/esp32_ble_client/ble_client_base.cpp b/esphome/components/esp32_ble_client/ble_client_base.cpp index 9254e13277..c1e67108ca 100644 --- a/esphome/components/esp32_ble_client/ble_client_base.cpp +++ b/esphome/components/esp32_ble_client/ble_client_base.cpp @@ -23,7 +23,9 @@ void BLEClientBase::setup() { } void BLEClientBase::loop() { - if (this->state_ == espbt::ClientState::DISCOVERED) { + // READY_TO_CONNECT means we have discovered the device + // and the scanner has been stopped by the tracker. + if (this->state_ == espbt::ClientState::READY_TO_CONNECT) { this->connect(); } } @@ -51,7 +53,8 @@ bool BLEClientBase::parse_device(const espbt::ESPBTDevice &device) { } void BLEClientBase::connect() { - ESP_LOGI(TAG, "[%d] [%s] Attempting BLE connection", this->connection_index_, this->address_str_.c_str()); + ESP_LOGI(TAG, "[%d] [%s] 0x%02x Attempting BLE connection", this->connection_index_, this->address_str_.c_str(), + this->remote_addr_type_); auto ret = esp_ble_gattc_open(this->gattc_if_, this->remote_bda_, this->remote_addr_type_, true); if (ret) { ESP_LOGW(TAG, "[%d] [%s] esp_ble_gattc_open error, status=%d", this->connection_index_, this->address_str_.c_str(), @@ -63,6 +66,8 @@ void BLEClientBase::connect() { } void BLEClientBase::disconnect() { + if (this->state_ == espbt::ClientState::IDLE || this->state_ == espbt::ClientState::DISCONNECTING) + return; ESP_LOGI(TAG, "[%d] [%s] Disconnecting.", this->connection_index_, this->address_str_.c_str()); auto err = esp_ble_gattc_close(this->gattc_if_, this->conn_id_); if (err != ESP_OK) { @@ -70,9 +75,12 @@ void BLEClientBase::disconnect() { err); } - if (this->state_ == espbt::ClientState::SEARCHING) { + if (this->state_ == espbt::ClientState::SEARCHING || this->state_ == espbt::ClientState::READY_TO_CONNECT || + this->state_ == espbt::ClientState::DISCOVERED) { this->set_address(0); this->set_state(espbt::ClientState::IDLE); + } else { + this->set_state(espbt::ClientState::DISCONNECTING); } } diff --git a/esphome/components/esp32_ble_tracker/esp32_ble_tracker.cpp b/esphome/components/esp32_ble_tracker/esp32_ble_tracker.cpp index 642fa27bdc..cdefd31288 100644 --- a/esphome/components/esp32_ble_tracker/esp32_ble_tracker.cpp +++ b/esphome/components/esp32_ble_tracker/esp32_ble_tracker.cpp @@ -66,7 +66,11 @@ void ESP32BLETracker::setup() { #endif if (this->scan_continuous_) { - this->start_scan_(true); + if (xSemaphoreTake(this->scan_end_lock_, 0L)) { + this->start_scan_(true); + } else { + ESP_LOGW(TAG, "Cannot start scan!"); + } } } @@ -83,86 +87,106 @@ void ESP32BLETracker::loop() { ble_event = this->ble_events_.pop(); } - if (this->scanner_idle_) { - return; - } - - bool connecting = false; + int connecting = 0; + int discovered = 0; + int searching = 0; for (auto *client : this->clients_) { - if (client->state() == ClientState::CONNECTING || client->state() == ClientState::DISCOVERED) - connecting = true; - } - - if (!connecting && xSemaphoreTake(this->scan_end_lock_, 0L)) { - xSemaphoreGive(this->scan_end_lock_); - if (this->scan_continuous_) { - this->start_scan_(false); - } else if (xSemaphoreTake(this->scan_end_lock_, 0L) && !this->scanner_idle_) { - xSemaphoreGive(this->scan_end_lock_); - this->end_of_scan_(); - return; + switch (client->state()) { + case ClientState::DISCOVERED: + discovered++; + break; + case ClientState::SEARCHING: + searching++; + break; + case ClientState::CONNECTING: + case ClientState::READY_TO_CONNECT: + connecting++; + break; + default: + break; } } + bool promote_to_connecting = discovered && !searching && !connecting; - if (xSemaphoreTake(this->scan_result_lock_, 5L / portTICK_PERIOD_MS)) { - uint32_t index = this->scan_result_index_; - xSemaphoreGive(this->scan_result_lock_); + if (!this->scanner_idle_) { + if (this->scan_result_index_ && // if it looks like we have a scan result we will take the lock + xSemaphoreTake(this->scan_result_lock_, 5L / portTICK_PERIOD_MS)) { + uint32_t index = this->scan_result_index_; + if (index) { + if (index >= 16) { + ESP_LOGW(TAG, "Too many BLE events to process. Some devices may not show up."); + } + for (size_t i = 0; i < index; i++) { + ESPBTDevice device; + device.parse_scan_rst(this->scan_result_buffer_[i]); - if (index >= 16) { - ESP_LOGW(TAG, "Too many BLE events to process. Some devices may not show up."); - } - for (size_t i = 0; i < index; i++) { - ESPBTDevice device; - device.parse_scan_rst(this->scan_result_buffer_[i]); + bool found = false; + for (auto *listener : this->listeners_) { + if (listener->parse_device(device)) + found = true; + } - bool found = false; - for (auto *listener : this->listeners_) { - if (listener->parse_device(device)) - found = true; - } - - for (auto *client : this->clients_) { - if (client->parse_device(device)) { - found = true; - if (client->state() == ClientState::DISCOVERED) { - esp_ble_gap_stop_scanning(); -#ifdef USE_ARDUINO - constexpr TickType_t block_time = 10L / portTICK_PERIOD_MS; -#else - constexpr TickType_t block_time = 0L; // PR #3594 -#endif - if (xSemaphoreTake(this->scan_end_lock_, block_time)) { - xSemaphoreGive(this->scan_end_lock_); + for (auto *client : this->clients_) { + if (client->parse_device(device)) { + found = true; + if (!connecting && client->state() == ClientState::DISCOVERED) { + promote_to_connecting = true; + } } } + + if (!found && !this->scan_continuous_) { + this->print_bt_device_info(device); + } } + this->scan_result_index_ = 0; } - - if (!found) { - this->print_bt_device_info(device); - } - } - - if (xSemaphoreTake(this->scan_result_lock_, 10L / portTICK_PERIOD_MS)) { - this->scan_result_index_ = 0; xSemaphoreGive(this->scan_result_lock_); } + + if (this->scan_set_param_failed_) { + ESP_LOGE(TAG, "Scan set param failed: %d", this->scan_set_param_failed_); + esp_ble_gap_stop_scanning(); + this->scan_set_param_failed_ = ESP_BT_STATUS_SUCCESS; + } + + if (this->scan_start_failed_) { + ESP_LOGE(TAG, "Scan start failed: %d", this->scan_start_failed_); + esp_ble_gap_stop_scanning(); + this->scan_start_failed_ = ESP_BT_STATUS_SUCCESS; + } + + if (!connecting && xSemaphoreTake(this->scan_end_lock_, 0L)) { + if (this->scan_continuous_) { + if (!promote_to_connecting) { + this->start_scan_(false); + } + } else if (!this->scanner_idle_) { + this->end_of_scan_(); + return; + } + } } - if (this->scan_set_param_failed_) { - ESP_LOGE(TAG, "Scan set param failed: %d", this->scan_set_param_failed_); - this->scan_set_param_failed_ = ESP_BT_STATUS_SUCCESS; - } - - if (this->scan_start_failed_) { - ESP_LOGE(TAG, "Scan start failed: %d", this->scan_start_failed_); - this->scan_start_failed_ = ESP_BT_STATUS_SUCCESS; + // If there is a discovered client and no connecting + // clients and no clients using the scanner to search for + // devices, then stop scanning and promote the discovered + // client to ready to connect. + if (promote_to_connecting) { + for (auto *client : this->clients_) { + if (client->state() == ClientState::DISCOVERED) { + ESP_LOGD(TAG, "Pausing scan to make connection..."); + esp_ble_gap_stop_scanning(); + // We only want to promote one client at a time. + client->set_state(ClientState::READY_TO_CONNECT); + break; + } + } } } void ESP32BLETracker::start_scan() { if (xSemaphoreTake(this->scan_end_lock_, 0L)) { - xSemaphoreGive(this->scan_end_lock_); this->start_scan_(true); } else { ESP_LOGW(TAG, "Scan requested when a scan is already in progress. Ignoring."); @@ -256,8 +280,9 @@ bool ESP32BLETracker::ble_setup() { } void ESP32BLETracker::start_scan_(bool first) { - if (!xSemaphoreTake(this->scan_end_lock_, 0L)) { - ESP_LOGW(TAG, "Cannot start scan!"); + // The lock must be held when calling this function. + if (xSemaphoreTake(this->scan_end_lock_, 0L)) { + ESP_LOGE(TAG, "start_scan called without holding scan_end_lock_"); return; } @@ -284,8 +309,9 @@ void ESP32BLETracker::start_scan_(bool first) { } void ESP32BLETracker::end_of_scan_() { - if (!xSemaphoreTake(this->scan_end_lock_, 0L)) { - ESP_LOGW(TAG, "Cannot clean up end of scan!"); + // The lock must be held when calling this function. + if (xSemaphoreTake(this->scan_end_lock_, 0L)) { + ESP_LOGE(TAG, "end_of_scan_ called without holding the scan_end_lock_"); return; } @@ -337,6 +363,9 @@ void ESP32BLETracker::gap_scan_set_param_complete_(const esp_ble_gap_cb_param_t: void ESP32BLETracker::gap_scan_start_complete_(const esp_ble_gap_cb_param_t::ble_scan_start_cmpl_evt_param ¶m) { this->scan_start_failed_ = param.status; + if (param.status != ESP_BT_STATUS_SUCCESS) { + xSemaphoreGive(this->scan_end_lock_); + } } void ESP32BLETracker::gap_scan_stop_complete_(const esp_ble_gap_cb_param_t::ble_scan_stop_cmpl_evt_param ¶m) { diff --git a/esphome/components/esp32_ble_tracker/esp32_ble_tracker.h b/esphome/components/esp32_ble_tracker/esp32_ble_tracker.h index 153a224378..0e980e9923 100644 --- a/esphome/components/esp32_ble_tracker/esp32_ble_tracker.h +++ b/esphome/components/esp32_ble_tracker/esp32_ble_tracker.h @@ -145,12 +145,18 @@ class ESPBTDeviceListener { }; enum class ClientState { + // Connection is allocated + INIT, + // Client is disconnecting + DISCONNECTING, // Connection is idle, no device detected. IDLE, // Searching for device. SEARCHING, // Device advertisement found. DISCOVERED, + // Device is discovered and the scanner is stopped + READY_TO_CONNECT, // Connection in progress. CONNECTING, // Initial connection established.