Skip to content

Commit

Permalink
Improve cleanup in BLEClient (#4742)
Browse files Browse the repository at this point in the history
- Remove client from the list of devices in case registration fails
- Filter other events not related to registration during registration phase
- Cleanup if connect fails
- Reset if after disconnect
- Disconnect callback *after* cleanup is done so object can be deleted

This fixes some of the issues I had like:
- `BLEClient::connect` hangs up and never recovered because registration failed
- `BLEClient` could not be deleted after disconnect or deletion creating ghost events #4047
- `BLEClient` could not be properly reused after a connection was attempted (successful or not) 

* Cleanup in case of registration and connect failure.
Cleanup before calling disconnect callback for safe delete.
Reject other events during registration.
Adresses #4047, #4055

* Clear if after unregister #4047
  • Loading branch information
kind3r authored Feb 16, 2021
1 parent 7cdfb8b commit 9be784f
Showing 1 changed file with 30 additions and 6 deletions.
36 changes: 30 additions & 6 deletions libraries/BLE/src/BLEClient.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,17 @@ bool BLEClient::connect(BLEAddress address, esp_ble_addr_type_t type) {
return false;
}

m_semaphoreRegEvt.wait("connect");
uint32_t rc = m_semaphoreRegEvt.wait("connect");

if (rc != ESP_GATT_OK) {
// fixes ESP_GATT_NO_RESOURCES error mostly
log_e("esp_ble_gattc_app_register_error: rc=%d", rc);
BLEDevice::removePeerDevice(m_appId, true);
// not sure if this is needed here
// esp_ble_gattc_app_unregister(m_gattc_if);
// m_gattc_if = ESP_GATT_IF_NONE;
return false;
}

m_peerAddress = address;

Expand All @@ -128,7 +138,13 @@ bool BLEClient::connect(BLEAddress address, esp_ble_addr_type_t type) {
return false;
}

uint32_t rc = m_semaphoreOpenEvt.wait("connect"); // Wait for the connection to complete.
rc = m_semaphoreOpenEvt.wait("connect"); // Wait for the connection to complete.
// check the status of the connection and cleanup in case of failure
if (rc != ESP_GATT_OK) {
BLEDevice::removePeerDevice(m_appId, true);
esp_ble_gattc_app_unregister(m_gattc_if);
m_gattc_if = ESP_GATT_IF_NONE;
}
log_v("<< connect(), rc=%d", rc==ESP_GATT_OK);
return rc == ESP_GATT_OK;
} // connect
Expand Down Expand Up @@ -160,6 +176,11 @@ void BLEClient::gattClientEventHandler(
log_d("gattClientEventHandler [esp_gatt_if: %d] ... %s",
gattc_if, BLEUtils::gattClientEventTypeToString(event).c_str());

// it is possible to receive events from other connections while waiting for registration
if (m_gattc_if == ESP_GATT_IF_NONE && event != ESP_GATTC_REG_EVT) {
return;
}

// Execute handler code based on the type of event received.
switch(event) {

Expand All @@ -184,15 +205,17 @@ void BLEClient::gattClientEventHandler(
if (evtParam->disconnect.conn_id != getConnId()) break;
// If we receive a disconnect event, set the class flag that indicates that we are
// no longer connected.
if (m_isConnected && m_pClientCallbacks != nullptr) {
m_pClientCallbacks->onDisconnect(this);
}
bool m_wasConnected = m_isConnected;
m_isConnected = false;
esp_ble_gattc_app_unregister(m_gattc_if);
m_gattc_if = ESP_GATT_IF_NONE;
m_semaphoreOpenEvt.give(ESP_GATT_IF_NONE);
m_semaphoreRssiCmplEvt.give();
m_semaphoreSearchCmplEvt.give(1);
BLEDevice::removePeerDevice(m_appId, true);
if (m_wasConnected && m_pClientCallbacks != nullptr) {
m_pClientCallbacks->onDisconnect(this);
}
break;
} // ESP_GATTC_DISCONNECT_EVT

Expand Down Expand Up @@ -228,7 +251,8 @@ void BLEClient::gattClientEventHandler(
//
case ESP_GATTC_REG_EVT: {
m_gattc_if = gattc_if;
m_semaphoreRegEvt.give();
// pass on the registration status result, in case of failure
m_semaphoreRegEvt.give(evtParam->reg.status);
break;
} // ESP_GATTC_REG_EVT

Expand Down

0 comments on commit 9be784f

Please sign in to comment.