From 8a03e4c2cb1f12db63dc8f4d0adccca7751a7d6d Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Fri, 25 Jul 2025 15:56:00 -1000 Subject: [PATCH 1/6] cleanup --- .../bluetooth_proxy/bluetooth_connection.cpp | 123 +++++++++--------- 1 file changed, 65 insertions(+), 58 deletions(-) diff --git a/esphome/components/bluetooth_proxy/bluetooth_connection.cpp b/esphome/components/bluetooth_proxy/bluetooth_connection.cpp index 4b84257e27..b7462803e8 100644 --- a/esphome/components/bluetooth_proxy/bluetooth_connection.cpp +++ b/esphome/components/bluetooth_proxy/bluetooth_connection.cpp @@ -113,72 +113,79 @@ void BluetoothConnection::send_service_for_discovery_() { } // Now process characteristics - uint16_t char_offset = 0; - esp_gattc_char_elem_t char_result; - while (true) { // characteristics - uint16_t char_count = 1; - esp_gatt_status_t char_status = - esp_ble_gattc_get_all_char(this->gattc_if_, this->conn_id_, service_result.start_handle, - service_result.end_handle, &char_result, &char_count, char_offset); - if (char_status == ESP_GATT_INVALID_OFFSET || char_status == ESP_GATT_NOT_FOUND) { - break; - } - if (char_status != ESP_GATT_OK) { - ESP_LOGE(TAG, "[%d] [%s] esp_ble_gattc_get_all_char error, status=%d", this->connection_index_, - this->address_str().c_str(), char_status); - break; - } - if (char_count == 0) { - break; - } - - service_resp.characteristics.emplace_back(); - auto &characteristic_resp = service_resp.characteristics.back(); - fill_128bit_uuid_array(characteristic_resp.uuid, char_result.uuid); - characteristic_resp.handle = char_result.char_handle; - characteristic_resp.properties = char_result.properties; - char_offset++; - - // Get the number of descriptors directly with one call - uint16_t total_desc_count = 0; - esp_gatt_status_t desc_count_status = - esp_ble_gattc_get_attr_count(this->gattc_if_, this->conn_id_, ESP_GATT_DB_DESCRIPTOR, char_result.char_handle, - service_result.end_handle, 0, &total_desc_count); - - if (desc_count_status == ESP_GATT_OK && total_desc_count > 0) { - // Only reserve if we successfully got a count - characteristic_resp.descriptors.reserve(total_desc_count); - } else if (desc_count_status != ESP_GATT_OK) { - ESP_LOGW(TAG, "[%d] [%s] Error getting descriptor count for char handle %d, status=%d", this->connection_index_, - this->address_str().c_str(), char_result.char_handle, desc_count_status); - } - - // Now process descriptors - uint16_t desc_offset = 0; - esp_gattc_descr_elem_t desc_result; - while (true) { // descriptors - uint16_t desc_count = 1; - esp_gatt_status_t desc_status = esp_ble_gattc_get_all_descr( - this->gattc_if_, this->conn_id_, char_result.char_handle, &desc_result, &desc_count, desc_offset); - if (desc_status == ESP_GATT_INVALID_OFFSET || desc_status == ESP_GATT_NOT_FOUND) { + if (char_count_status == ESP_GATT_OK && total_char_count > 0) { + uint16_t char_offset = 0; + esp_gattc_char_elem_t char_result; + while (true) { // characteristics + uint16_t char_count = 1; + esp_gatt_status_t char_status = + esp_ble_gattc_get_all_char(this->gattc_if_, this->conn_id_, service_result.start_handle, + service_result.end_handle, &char_result, &char_count, char_offset); + if (char_status == ESP_GATT_INVALID_OFFSET || char_status == ESP_GATT_NOT_FOUND) { break; } - if (desc_status != ESP_GATT_OK) { - ESP_LOGE(TAG, "[%d] [%s] esp_ble_gattc_get_all_descr error, status=%d", this->connection_index_, - this->address_str().c_str(), desc_status); + if (char_status != ESP_GATT_OK) { + ESP_LOGE(TAG, "[%d] [%s] esp_ble_gattc_get_all_char error, status=%d", this->connection_index_, + this->address_str().c_str(), char_status); break; } - if (desc_count == 0) { + if (char_count == 0) { break; } - characteristic_resp.descriptors.emplace_back(); - auto &descriptor_resp = characteristic_resp.descriptors.back(); - fill_128bit_uuid_array(descriptor_resp.uuid, desc_result.uuid); - descriptor_resp.handle = desc_result.handle; - desc_offset++; + service_resp.characteristics.emplace_back(); + auto &characteristic_resp = service_resp.characteristics.back(); + fill_128bit_uuid_array(characteristic_resp.uuid, char_result.uuid); + characteristic_resp.handle = char_result.char_handle; + characteristic_resp.properties = char_result.properties; + char_offset++; + + // Get the number of descriptors directly with one call + uint16_t total_desc_count = 0; + esp_gatt_status_t desc_count_status = + esp_ble_gattc_get_attr_count(this->gattc_if_, this->conn_id_, ESP_GATT_DB_DESCRIPTOR, char_result.char_handle, + service_result.end_handle, 0, &total_desc_count); + + if (desc_count_status == ESP_GATT_OK && total_desc_count > 0) { + // Only reserve if we successfully got a count + characteristic_resp.descriptors.reserve(total_desc_count); + } else if (desc_count_status != ESP_GATT_OK) { + ESP_LOGW(TAG, "[%d] [%s] Error getting descriptor count for char handle %d, status=%d", this->connection_index_, + this->address_str().c_str(), char_result.char_handle, desc_count_status); + } + + // Skip descriptor processing if there are no descriptors + if (desc_count_status != ESP_GATT_OK || total_desc_count == 0) { + continue; + } + + // Now process descriptors + uint16_t desc_offset = 0; + esp_gattc_descr_elem_t desc_result; + while (true) { // descriptors + uint16_t desc_count = 1; + esp_gatt_status_t desc_status = esp_ble_gattc_get_all_descr( + this->gattc_if_, this->conn_id_, char_result.char_handle, &desc_result, &desc_count, desc_offset); + if (desc_status == ESP_GATT_INVALID_OFFSET || desc_status == ESP_GATT_NOT_FOUND) { + break; + } + if (desc_status != ESP_GATT_OK) { + ESP_LOGE(TAG, "[%d] [%s] esp_ble_gattc_get_all_descr error, status=%d", this->connection_index_, + this->address_str().c_str(), desc_status); + break; + } + if (desc_count == 0) { + break; + } + + characteristic_resp.descriptors.emplace_back(); + auto &descriptor_resp = characteristic_resp.descriptors.back(); + fill_128bit_uuid_array(descriptor_resp.uuid, desc_result.uuid); + descriptor_resp.handle = desc_result.handle; + desc_offset++; + } } - } + } // end if (char_count_status == ESP_GATT_OK && total_char_count > 0) // Send the message (we already checked api_conn is not null at the beginning) api_conn->send_message(resp, api::BluetoothGATTGetServicesResponse::MESSAGE_TYPE); From 85a4f05d67de926c63f014152e98cba9300a903e Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Fri, 25 Jul 2025 15:57:23 -1000 Subject: [PATCH 2/6] cleanup --- .../bluetooth_proxy/bluetooth_connection.cpp | 75 ++++++++----------- 1 file changed, 32 insertions(+), 43 deletions(-) diff --git a/esphome/components/bluetooth_proxy/bluetooth_connection.cpp b/esphome/components/bluetooth_proxy/bluetooth_connection.cpp index b7462803e8..8e42ba5951 100644 --- a/esphome/components/bluetooth_proxy/bluetooth_connection.cpp +++ b/esphome/components/bluetooth_proxy/bluetooth_connection.cpp @@ -104,16 +104,12 @@ void BluetoothConnection::send_service_for_discovery_() { esp_ble_gattc_get_attr_count(this->gattc_if_, this->conn_id_, ESP_GATT_DB_CHARACTERISTIC, service_result.start_handle, service_result.end_handle, 0, &total_char_count); - if (char_count_status == ESP_GATT_OK && total_char_count > 0) { - // Only reserve if we successfully got a count - service_resp.characteristics.reserve(total_char_count); - } else if (char_count_status != ESP_GATT_OK) { + if (char_count_status != ESP_GATT_OK) { ESP_LOGW(TAG, "[%d] [%s] Error getting characteristic count, status=%d", this->connection_index_, this->address_str().c_str(), char_count_status); - } - - // Now process characteristics - if (char_count_status == ESP_GATT_OK && total_char_count > 0) { + } else if (total_char_count > 0) { + // Reserve space and process characteristics + service_resp.characteristics.reserve(total_char_count); uint16_t char_offset = 0; esp_gattc_char_elem_t char_result; while (true) { // characteristics @@ -146,46 +142,39 @@ void BluetoothConnection::send_service_for_discovery_() { esp_ble_gattc_get_attr_count(this->gattc_if_, this->conn_id_, ESP_GATT_DB_DESCRIPTOR, char_result.char_handle, service_result.end_handle, 0, &total_desc_count); - if (desc_count_status == ESP_GATT_OK && total_desc_count > 0) { - // Only reserve if we successfully got a count - characteristic_resp.descriptors.reserve(total_desc_count); - } else if (desc_count_status != ESP_GATT_OK) { + if (desc_count_status != ESP_GATT_OK) { ESP_LOGW(TAG, "[%d] [%s] Error getting descriptor count for char handle %d, status=%d", this->connection_index_, this->address_str().c_str(), char_result.char_handle, desc_count_status); - } + } else if (total_desc_count > 0) { + // Reserve space and process descriptors + characteristic_resp.descriptors.reserve(total_desc_count); + uint16_t desc_offset = 0; + esp_gattc_descr_elem_t desc_result; + while (true) { // descriptors + uint16_t desc_count = 1; + esp_gatt_status_t desc_status = esp_ble_gattc_get_all_descr( + this->gattc_if_, this->conn_id_, char_result.char_handle, &desc_result, &desc_count, desc_offset); + if (desc_status == ESP_GATT_INVALID_OFFSET || desc_status == ESP_GATT_NOT_FOUND) { + break; + } + if (desc_status != ESP_GATT_OK) { + ESP_LOGE(TAG, "[%d] [%s] esp_ble_gattc_get_all_descr error, status=%d", this->connection_index_, + this->address_str().c_str(), desc_status); + break; + } + if (desc_count == 0) { + break; + } - // Skip descriptor processing if there are no descriptors - if (desc_count_status != ESP_GATT_OK || total_desc_count == 0) { - continue; - } - - // Now process descriptors - uint16_t desc_offset = 0; - esp_gattc_descr_elem_t desc_result; - while (true) { // descriptors - uint16_t desc_count = 1; - esp_gatt_status_t desc_status = esp_ble_gattc_get_all_descr( - this->gattc_if_, this->conn_id_, char_result.char_handle, &desc_result, &desc_count, desc_offset); - if (desc_status == ESP_GATT_INVALID_OFFSET || desc_status == ESP_GATT_NOT_FOUND) { - break; + characteristic_resp.descriptors.emplace_back(); + auto &descriptor_resp = characteristic_resp.descriptors.back(); + fill_128bit_uuid_array(descriptor_resp.uuid, desc_result.uuid); + descriptor_resp.handle = desc_result.handle; + desc_offset++; } - if (desc_status != ESP_GATT_OK) { - ESP_LOGE(TAG, "[%d] [%s] esp_ble_gattc_get_all_descr error, status=%d", this->connection_index_, - this->address_str().c_str(), desc_status); - break; - } - if (desc_count == 0) { - break; - } - - characteristic_resp.descriptors.emplace_back(); - auto &descriptor_resp = characteristic_resp.descriptors.back(); - fill_128bit_uuid_array(descriptor_resp.uuid, desc_result.uuid); - descriptor_resp.handle = desc_result.handle; - desc_offset++; - } + } // end else if (total_desc_count > 0) } - } // end if (char_count_status == ESP_GATT_OK && total_char_count > 0) + } // end else if (total_char_count > 0) // Send the message (we already checked api_conn is not null at the beginning) api_conn->send_message(resp, api::BluetoothGATTGetServicesResponse::MESSAGE_TYPE); From 5d8f38cce4e4ceae508c539a3a48d6c057b1f5f8 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Fri, 25 Jul 2025 15:58:49 -1000 Subject: [PATCH 3/6] cleanup --- esphome/components/bluetooth_proxy/bluetooth_connection.cpp | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/esphome/components/bluetooth_proxy/bluetooth_connection.cpp b/esphome/components/bluetooth_proxy/bluetooth_connection.cpp index 8e42ba5951..b4dfb72ab4 100644 --- a/esphome/components/bluetooth_proxy/bluetooth_connection.cpp +++ b/esphome/components/bluetooth_proxy/bluetooth_connection.cpp @@ -119,8 +119,7 @@ void BluetoothConnection::send_service_for_discovery_() { service_result.end_handle, &char_result, &char_count, char_offset); if (char_status == ESP_GATT_INVALID_OFFSET || char_status == ESP_GATT_NOT_FOUND) { break; - } - if (char_status != ESP_GATT_OK) { + } else if (char_status != ESP_GATT_OK) { ESP_LOGE(TAG, "[%d] [%s] esp_ble_gattc_get_all_char error, status=%d", this->connection_index_, this->address_str().c_str(), char_status); break; @@ -156,8 +155,7 @@ void BluetoothConnection::send_service_for_discovery_() { this->gattc_if_, this->conn_id_, char_result.char_handle, &desc_result, &desc_count, desc_offset); if (desc_status == ESP_GATT_INVALID_OFFSET || desc_status == ESP_GATT_NOT_FOUND) { break; - } - if (desc_status != ESP_GATT_OK) { + } else if (desc_status != ESP_GATT_OK) { ESP_LOGE(TAG, "[%d] [%s] esp_ble_gattc_get_all_descr error, status=%d", this->connection_index_, this->address_str().c_str(), desc_status); break; From accbc8fb0b29fe3b556ef5b0b511407a056e62fa Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Fri, 25 Jul 2025 16:07:46 -1000 Subject: [PATCH 4/6] cleanup --- .../bluetooth_proxy/bluetooth_connection.cpp | 30 ++++++++----------- 1 file changed, 12 insertions(+), 18 deletions(-) diff --git a/esphome/components/bluetooth_proxy/bluetooth_connection.cpp b/esphome/components/bluetooth_proxy/bluetooth_connection.cpp index b4dfb72ab4..7737a249bb 100644 --- a/esphome/components/bluetooth_proxy/bluetooth_connection.cpp +++ b/esphome/components/bluetooth_proxy/bluetooth_connection.cpp @@ -80,15 +80,10 @@ void BluetoothConnection::send_service_for_discovery_() { &service_result, &service_count, this->send_service_); this->send_service_++; - if (service_status != ESP_GATT_OK) { - ESP_LOGE(TAG, "[%d] [%s] esp_ble_gattc_get_service error at offset=%d, status=%d", this->connection_index_, - this->address_str().c_str(), this->send_service_ - 1, service_status); - return; - } - - if (service_count == 0) { - ESP_LOGE(TAG, "[%d] [%s] esp_ble_gattc_get_service missing, service_count=%d", this->connection_index_, - this->address_str().c_str(), service_count); + if (service_status != ESP_GATT_OK || service_count == 0) { + ESP_LOGE(TAG, "[%d] [%s] esp_ble_gattc_get_service %s, status=%d, service_count=%d, offset=%d", + this->connection_index_, this->address_str().c_str(), service_status != ESP_GATT_OK ? "error" : "missing", + service_status, service_count, this->send_service_ - 1); return; } @@ -107,7 +102,12 @@ void BluetoothConnection::send_service_for_discovery_() { if (char_count_status != ESP_GATT_OK) { ESP_LOGW(TAG, "[%d] [%s] Error getting characteristic count, status=%d", this->connection_index_, this->address_str().c_str(), char_count_status); - } else if (total_char_count > 0) { + // Send the message (we already checked api_conn is not null at the beginning) + api_conn->send_message(resp, api::BluetoothGATTGetServicesResponse::MESSAGE_TYPE); + return; + } + + if (total_char_count > 0) { // Reserve space and process characteristics service_resp.characteristics.reserve(total_char_count); uint16_t char_offset = 0; @@ -117,16 +117,13 @@ void BluetoothConnection::send_service_for_discovery_() { esp_gatt_status_t char_status = esp_ble_gattc_get_all_char(this->gattc_if_, this->conn_id_, service_result.start_handle, service_result.end_handle, &char_result, &char_count, char_offset); - if (char_status == ESP_GATT_INVALID_OFFSET || char_status == ESP_GATT_NOT_FOUND) { + if (char_status == ESP_GATT_INVALID_OFFSET || char_status == ESP_GATT_NOT_FOUND || char_count == 0) { break; } else if (char_status != ESP_GATT_OK) { ESP_LOGE(TAG, "[%d] [%s] esp_ble_gattc_get_all_char error, status=%d", this->connection_index_, this->address_str().c_str(), char_status); break; } - if (char_count == 0) { - break; - } service_resp.characteristics.emplace_back(); auto &characteristic_resp = service_resp.characteristics.back(); @@ -153,16 +150,13 @@ void BluetoothConnection::send_service_for_discovery_() { uint16_t desc_count = 1; esp_gatt_status_t desc_status = esp_ble_gattc_get_all_descr( this->gattc_if_, this->conn_id_, char_result.char_handle, &desc_result, &desc_count, desc_offset); - if (desc_status == ESP_GATT_INVALID_OFFSET || desc_status == ESP_GATT_NOT_FOUND) { + if (desc_status == ESP_GATT_INVALID_OFFSET || desc_status == ESP_GATT_NOT_FOUND || desc_count == 0) { break; } else if (desc_status != ESP_GATT_OK) { ESP_LOGE(TAG, "[%d] [%s] esp_ble_gattc_get_all_descr error, status=%d", this->connection_index_, this->address_str().c_str(), desc_status); break; } - if (desc_count == 0) { - break; - } characteristic_resp.descriptors.emplace_back(); auto &descriptor_resp = characteristic_resp.descriptors.back(); From 3396dfe52ab8adf9e0b8735bacbb4c1f2d5e5206 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Fri, 25 Jul 2025 16:08:22 -1000 Subject: [PATCH 5/6] cleanup --- esphome/components/bluetooth_proxy/bluetooth_connection.cpp | 2 -- 1 file changed, 2 deletions(-) diff --git a/esphome/components/bluetooth_proxy/bluetooth_connection.cpp b/esphome/components/bluetooth_proxy/bluetooth_connection.cpp index 7737a249bb..d1a9a8d610 100644 --- a/esphome/components/bluetooth_proxy/bluetooth_connection.cpp +++ b/esphome/components/bluetooth_proxy/bluetooth_connection.cpp @@ -102,8 +102,6 @@ void BluetoothConnection::send_service_for_discovery_() { if (char_count_status != ESP_GATT_OK) { ESP_LOGW(TAG, "[%d] [%s] Error getting characteristic count, status=%d", this->connection_index_, this->address_str().c_str(), char_count_status); - // Send the message (we already checked api_conn is not null at the beginning) - api_conn->send_message(resp, api::BluetoothGATTGetServicesResponse::MESSAGE_TYPE); return; } From b22ff37e3d1229d3a3db515cb1ba6c5006038a7e Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Fri, 25 Jul 2025 16:09:44 -1000 Subject: [PATCH 6/6] cleanup --- .../components/bluetooth_proxy/bluetooth_connection.cpp | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/esphome/components/bluetooth_proxy/bluetooth_connection.cpp b/esphome/components/bluetooth_proxy/bluetooth_connection.cpp index d1a9a8d610..72efd17742 100644 --- a/esphome/components/bluetooth_proxy/bluetooth_connection.cpp +++ b/esphome/components/bluetooth_proxy/bluetooth_connection.cpp @@ -120,7 +120,7 @@ void BluetoothConnection::send_service_for_discovery_() { } else if (char_status != ESP_GATT_OK) { ESP_LOGE(TAG, "[%d] [%s] esp_ble_gattc_get_all_char error, status=%d", this->connection_index_, this->address_str().c_str(), char_status); - break; + return; } service_resp.characteristics.emplace_back(); @@ -139,7 +139,10 @@ void BluetoothConnection::send_service_for_discovery_() { if (desc_count_status != ESP_GATT_OK) { ESP_LOGW(TAG, "[%d] [%s] Error getting descriptor count for char handle %d, status=%d", this->connection_index_, this->address_str().c_str(), char_result.char_handle, desc_count_status); - } else if (total_desc_count > 0) { + return; + } + + if (total_desc_count > 0) { // Reserve space and process descriptors characteristic_resp.descriptors.reserve(total_desc_count); uint16_t desc_offset = 0; @@ -153,7 +156,7 @@ void BluetoothConnection::send_service_for_discovery_() { } else if (desc_status != ESP_GATT_OK) { ESP_LOGE(TAG, "[%d] [%s] esp_ble_gattc_get_all_descr error, status=%d", this->connection_index_, this->address_str().c_str(), desc_status); - break; + return; } characteristic_resp.descriptors.emplace_back();