Apply combined review: wire MTU parity, abort teardown parity, closure ownership

This commit is contained in:
J. Nick Koston
2026-08-09 03:08:47 -05:00
parent 833a835e0f
commit 612033062f
7 changed files with 48 additions and 22 deletions
@@ -140,10 +140,8 @@ concept BLEGattConnectionContract = requires(T conn, GattClientListener *listene
{ conn.update_connection_params(uint16_t{}, uint16_t{}, uint16_t{}, uint16_t{}) } -> std::same_as<int>;
{ conn.get_service_table() } -> std::same_as<GattServiceTable>;
{ conn.release_services() } -> std::same_as<void>;
// Deferred-disconnect visibility and the connection-type hint; backends
// without the underlying state carry inline no-ops.
{ conn.disconnect_pending() } -> std::same_as<bool>;
{ conn.cancel_pending_disconnect() } -> std::same_as<void>;
// Connection-type hint for backends that tune parameters by it; others
// carry an inline no-op.
{ conn.set_connection_type(ConnectionType{}) } -> std::same_as<void>;
};
@@ -19,6 +19,15 @@ DOMAIN = "bluetooth_connection"
def AUTO_LOAD() -> list[str]:
"""ble_device_base plus the platform BLE stack the build's backend
registers with (the Bluedroid header includes the tracker's), so
consumers need not know. The platform-less arm serves manifest tooling."""
if CORE.is_esp32:
return ["ble_device_base", "esp32_ble_tracker"]
if CORE.is_rp2:
return ["ble_device_base", "rp2040_ble"]
if CORE.target_platform is None:
return ["ble_device_base", "esp32_ble_tracker", "rp2040_ble"]
return ["ble_device_base"]
@@ -116,9 +125,13 @@ def gatt_client_schema(platform: str | None = None) -> cv.Schema:
def hub_connection_schema(platform: str | None = None) -> cv.Schema:
"""Per-slot schema for the proxy's connection wrappers: the wrapper id on
top of the backend fragment. Same platform rules as gatt_client_schema()."""
return gatt_client_schema(platform).extend(
{cv.GenerateID(): cv.declare_id(HubBluetoothConnection)}
top of the backend fragment, plus the component keys (setup_priority and
friends now apply to the backend, the slot's real Component). Same
platform rules as gatt_client_schema()."""
return (
gatt_client_schema(platform)
.extend({cv.GenerateID(): cv.declare_id(HubBluetoothConnection)})
.extend(cv.COMPONENT_SCHEMA)
)
@@ -131,9 +144,11 @@ async def new_gatt_backend(config: ConfigType) -> cg.MockObj:
ble_device_base.request_gatt_client()
backend = cg.new_Pvariable(config[CONF_BACKEND_ID])
# The backend has no user-facing component options; an empty config keeps
# the consumer's own keys (update_interval, ...) off it.
await cg.register_component(backend, {})
# The backend is the slot's real Component: component keys from the
# connection entry (setup_priority, ...) apply to it. Consumers whose own
# schema carries keys that register_component would misapply to the
# backend (e.g. a polling interval) must not put them in this config.
await cg.register_component(backend, config)
await _backend_entry().register(backend, config)
return backend
@@ -529,13 +529,16 @@ void BluedroidGattClient::stream_service_batch(BluetoothConnection &conn) {
ESP_LOGE(TAG, "[%d] [%s] Service walk failed (service %d), aborting stream", conn.connection_index_,
conn.address_str_, conn.send_service_);
conn.send_service_ = DONE_SENDING_SERVICES;
conn.disconnect();
return;
}
uint16_t total_char_count = 0;
if (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) != ESP_GATT_OK) {
this->log_gattc_warning_("esp_ble_gattc_get_attr_count", ESP_GATT_ERROR);
conn.send_service_ = DONE_SENDING_SERVICES;
conn.disconnect();
return;
}
@@ -568,6 +571,7 @@ void BluedroidGattClient::stream_service_batch(BluetoothConnection &conn) {
if (char_status != ESP_GATT_OK) {
this->log_gattc_warning_("esp_ble_gattc_get_all_char", char_status);
conn.send_service_ = DONE_SENDING_SERVICES;
conn.disconnect();
return;
}
break;
@@ -587,6 +591,7 @@ void BluedroidGattClient::stream_service_batch(BluetoothConnection &conn) {
// missing CCCD in a cached database breaks notifications for good.
this->log_gattc_warning_("esp_ble_gattc_get_attr_count", desc_count_status);
conn.send_service_ = DONE_SENDING_SERVICES;
conn.disconnect();
return;
}
if (total_desc_count > 0) {
@@ -604,6 +609,7 @@ void BluedroidGattClient::stream_service_batch(BluetoothConnection &conn) {
if (desc_status != ESP_GATT_OK) {
this->log_gattc_warning_("esp_ble_gattc_get_all_descr", desc_status);
conn.send_service_ = DONE_SENDING_SERVICES;
conn.disconnect();
return;
}
break;
@@ -668,7 +674,9 @@ void BluedroidGattClient::handle_open_evt_(esp_ble_gattc_cb_param_t *param) {
// suppressed by seen_mtu_ (HA tolerates a post-connect MTU of 23 here,
// matching the previous esp32 behavior).
this->seen_mtu_ = true;
this->report_connection_state_(true, 0, 0);
// Wire parity with the old class: no MTU exchange happened yet, and HA
// has always been handed the default 23 on the cached path.
this->report_connection_state_(true, 23, 0);
// Settled: only the disconnect safety net needs the loop, and
// set_disconnecting_() re-enables it.
this->disable_loop();
@@ -44,8 +44,6 @@ class StubGattBackend {
return ble_device_base::GATT_ERR_NOT_CONNECTED;
}
ble_device_base::GattServiceTable get_service_table() { return {}; }
bool disconnect_pending() const { return false; }
void cancel_pending_disconnect() {}
void set_connection_type(ble_device_base::ConnectionType ct) {}
void release_services() {}
};
@@ -94,8 +94,6 @@ class RP2GattClient final : public Component, public Parented<rp2040_ble::RP2040
ble_device_base::GattServiceTable get_service_table();
// No deferred-disconnect state (disconnect is one call) and no
// connection-type branching on this backend.
bool disconnect_pending() const { return false; }
void cancel_pending_disconnect() {}
void set_connection_type(ble_device_base::ConnectionType ct) {}
void release_services();
@@ -178,11 +178,22 @@ def test_rp2_rejects_esp32_only_keys_by_name(
def test_bluetooth_connection_auto_load_covers_its_includes() -> None:
# Every backend builds on ble_device_base alone; the Bluedroid backend
# talks to IDF directly, so esp32_ble_client is no longer in the closure.
for platform in ("esp32", "rp2", None):
_set_platform(platform)
assert bluetooth_connection.AUTO_LOAD() == ["ble_device_base"]
# The backend registers with its platform BLE stack (and the Bluedroid
# header includes the tracker's), so that closure lives here and
# consumers stay platform-blind; the platform-less arm is the union for
# manifest-resolving tooling.
_set_platform("esp32")
assert bluetooth_connection.AUTO_LOAD() == ["ble_device_base", "esp32_ble_tracker"]
_set_platform("rp2")
assert bluetooth_connection.AUTO_LOAD() == ["ble_device_base", "rp2040_ble"]
_set_platform("ln882x")
assert bluetooth_connection.AUTO_LOAD() == ["ble_device_base"]
_set_platform(None)
assert bluetooth_connection.AUTO_LOAD() == [
"ble_device_base",
"esp32_ble_tracker",
"rp2040_ble",
]
def test_every_registered_hub_platform_has_a_schema_arm() -> None:
@@ -50,8 +50,6 @@ class MinimalConnection {
}
GattServiceTable get_service_table() { return {}; }
void release_services() {}
bool disconnect_pending() const { return false; }
void cancel_pending_disconnect() {}
void set_connection_type(ConnectionType ct) {}
protected: