Skip to content

Commit

Permalink
Workaround ulTaskNotifyTake not blocking. (#326)
Browse files Browse the repository at this point in the history 
The new FreeRTOS version used in newer IDF versions does not always clear the task notification value.
This resulted in calls to block using ulTaskNotifyTake returning immediately and causing various crashes.
  • Loading branch information
@h2zero
h2zero committed Jan 10, 2022
1 parent d160571 commit 335ad93
Show file tree
Hide file tree
Showing 5 changed files with 65 additions and 10 deletions.
22 changes: 19 additions & 3 deletions src/NimBLEClient.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,8 @@ bool NimBLEClient::connect(const NimBLEAddress &address, bool deleteAttibutes) {
m_peerAddress = address;
}

ble_task_data_t taskData = {this, xTaskGetCurrentTaskHandle(), 0, nullptr};
TaskHandle_t cur_task = xTaskGetCurrentTaskHandle();
ble_task_data_t taskData = {this, cur_task, 0, nullptr};
m_pTaskData = &taskData;
int rc = 0;

Expand Down Expand Up @@ -259,6 +260,10 @@ bool NimBLEClient::connect(const NimBLEAddress &address, bool deleteAttibutes) {
return false;
}

#ifdef ulTaskNotifyValueClear
// Clear the task notification value to ensure we block
ulTaskNotifyValueClear(cur_task, ULONG_MAX);
#endif
// Wait for the connect timeout time +1 second for the connection to complete
if(ulTaskNotifyTake(pdTRUE, pdMS_TO_TICKS(m_connectTimeout + 1000)) == pdFALSE) {
m_pTaskData = nullptr;
Expand Down Expand Up @@ -309,7 +314,8 @@ bool NimBLEClient::connect(const NimBLEAddress &address, bool deleteAttibutes) {
* @return True on success.
*/
bool NimBLEClient::secureConnection() {
ble_task_data_t taskData = {this, xTaskGetCurrentTaskHandle(), 0, nullptr};
TaskHandle_t cur_task = xTaskGetCurrentTaskHandle();
ble_task_data_t taskData = {this, cur_task, 0, nullptr};

int retryCount = 1;

Expand All @@ -323,6 +329,10 @@ bool NimBLEClient::secureConnection() {
return false;
}

#ifdef ulTaskNotifyValueClear
// Clear the task notification value to ensure we block
ulTaskNotifyValueClear(cur_task, ULONG_MAX);
#endif
ulTaskNotifyTake(pdTRUE, portMAX_DELAY);
} while (taskData.rc == (BLE_HS_ERR_HCI_BASE + BLE_ERR_PINKEY_MISSING) && retryCount--);

Expand Down Expand Up @@ -647,7 +657,8 @@ bool NimBLEClient::retrieveServices(const NimBLEUUID *uuid_filter) {
}

int rc = 0;
ble_task_data_t taskData = {this, xTaskGetCurrentTaskHandle(), 0, nullptr};
TaskHandle_t cur_task = xTaskGetCurrentTaskHandle();
ble_task_data_t taskData = {this, cur_task, 0, nullptr};

if(uuid_filter == nullptr) {
rc = ble_gattc_disc_all_svcs(m_conn_id, NimBLEClient::serviceDiscoveredCB, &taskData);
Expand All @@ -662,6 +673,11 @@ bool NimBLEClient::retrieveServices(const NimBLEUUID *uuid_filter) {
return false;
}

#ifdef ulTaskNotifyValueClear
// Clear the task notification value to ensure we block
ulTaskNotifyValueClear(cur_task, ULONG_MAX);
#endif

// wait until we have all the services
ulTaskNotifyTake(pdTRUE, portMAX_DELAY);
m_lastErr = taskData.rc;
Expand Down
25 changes: 22 additions & 3 deletions src/NimBLERemoteCharacteristic.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,8 @@ bool NimBLERemoteCharacteristic::retrieveDescriptors(const NimBLEUUID *uuid_filt
}

int rc = 0;
ble_task_data_t taskData = {this, xTaskGetCurrentTaskHandle(), 0, nullptr};
TaskHandle_t cur_task = xTaskGetCurrentTaskHandle();
ble_task_data_t taskData = {this, cur_task, 0, nullptr};

// If we don't know the end handle of this characteristic retrieve the next one in the service
// The end handle is the next characteristic definition handle -1.
Expand All @@ -256,6 +257,10 @@ bool NimBLERemoteCharacteristic::retrieveDescriptors(const NimBLEUUID *uuid_filt
return false;
}

#ifdef ulTaskNotifyValueClear
// Clear the task notification value to ensure we block
ulTaskNotifyValueClear(cur_task, ULONG_MAX);
#endif
ulTaskNotifyTake(pdTRUE, portMAX_DELAY);

if (taskData.rc != 0) {
Expand All @@ -277,6 +282,10 @@ bool NimBLERemoteCharacteristic::retrieveDescriptors(const NimBLEUUID *uuid_filt
return false;
}

#ifdef ulTaskNotifyValueClear
// Clear the task notification value to ensure we block
ulTaskNotifyValueClear(cur_task, ULONG_MAX);
#endif
ulTaskNotifyTake(pdTRUE, portMAX_DELAY);

if (taskData.rc != 0) {
Expand Down Expand Up @@ -477,7 +486,8 @@ std::string NimBLERemoteCharacteristic::readValue(time_t *timestamp) {

int rc = 0;
int retryCount = 1;
ble_task_data_t taskData = {this, xTaskGetCurrentTaskHandle(),0, &value};
TaskHandle_t cur_task = xTaskGetCurrentTaskHandle();
ble_task_data_t taskData = {this, cur_task, 0, &value};

do {
rc = ble_gattc_read_long(pClient->getConnId(), m_handle, 0,
Expand All @@ -489,6 +499,10 @@ std::string NimBLERemoteCharacteristic::readValue(time_t *timestamp) {
return value;
}

#ifdef ulTaskNotifyValueClear
// Clear the task notification value to ensure we block
ulTaskNotifyValueClear(cur_task, ULONG_MAX);
#endif
ulTaskNotifyTake(pdTRUE, portMAX_DELAY);
rc = taskData.rc;

Expand Down Expand Up @@ -746,7 +760,8 @@ bool NimBLERemoteCharacteristic::writeValue(const uint8_t* data, size_t length,
return (rc==0);
}

ble_task_data_t taskData = {this, xTaskGetCurrentTaskHandle(), 0, nullptr};
TaskHandle_t cur_task = xTaskGetCurrentTaskHandle();
ble_task_data_t taskData = {this, cur_task, 0, nullptr};

do {
if(length > mtu) {
Expand All @@ -766,6 +781,10 @@ bool NimBLERemoteCharacteristic::writeValue(const uint8_t* data, size_t length,
return false;
}

#ifdef ulTaskNotifyValueClear
// Clear the task notification value to ensure we block
ulTaskNotifyValueClear(cur_task, ULONG_MAX);
#endif
ulTaskNotifyTake(pdTRUE, portMAX_DELAY);
rc = taskData.rc;

Expand Down
14 changes: 12 additions & 2 deletions src/NimBLERemoteDescriptor.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,8 @@ std::string NimBLERemoteDescriptor::readValue() {

int rc = 0;
int retryCount = 1;
ble_task_data_t taskData = {this, xTaskGetCurrentTaskHandle(),0, &value};
TaskHandle_t cur_task = xTaskGetCurrentTaskHandle();
ble_task_data_t taskData = {this, cur_task, 0, &value};

do {
rc = ble_gattc_read_long(pClient->getConnId(), m_handle, 0,
Expand All @@ -151,6 +152,10 @@ std::string NimBLERemoteDescriptor::readValue() {
return value;
}

#ifdef ulTaskNotifyValueClear
// Clear the task notification value to ensure we block
ulTaskNotifyValueClear(cur_task, ULONG_MAX);
#endif
ulTaskNotifyTake(pdTRUE, portMAX_DELAY);
rc = taskData.rc;

Expand Down Expand Up @@ -290,7 +295,8 @@ bool NimBLERemoteDescriptor::writeValue(const uint8_t* data, size_t length, bool
return (rc == 0);
}

ble_task_data_t taskData = {this, xTaskGetCurrentTaskHandle(), 0, nullptr};
TaskHandle_t cur_task = xTaskGetCurrentTaskHandle();
ble_task_data_t taskData = {this, cur_task, 0, nullptr};

do {
if(length > mtu) {
Expand All @@ -311,6 +317,10 @@ bool NimBLERemoteDescriptor::writeValue(const uint8_t* data, size_t length, bool
return false;
}

#ifdef ulTaskNotifyValueClear
// Clear the task notification value to ensure we block
ulTaskNotifyValueClear(cur_task, ULONG_MAX);
#endif
ulTaskNotifyTake(pdTRUE, portMAX_DELAY);
rc = taskData.rc;

Expand Down
7 changes: 6 additions & 1 deletion src/NimBLERemoteService.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,8 @@ bool NimBLERemoteService::retrieveCharacteristics(const NimBLEUUID *uuid_filter)
NIMBLE_LOGD(LOG_TAG, ">> retrieveCharacteristics() for service: %s", getUUID().toString().c_str());

int rc = 0;
ble_task_data_t taskData = {this, xTaskGetCurrentTaskHandle(), 0, nullptr};
TaskHandle_t cur_task = xTaskGetCurrentTaskHandle();
ble_task_data_t taskData = {this, cur_task, 0, nullptr};

if(uuid_filter == nullptr) {
rc = ble_gattc_disc_all_chrs(m_pClient->getConnId(),
Expand All @@ -220,6 +221,10 @@ bool NimBLERemoteService::retrieveCharacteristics(const NimBLEUUID *uuid_filter)
return false;
}

#ifdef ulTaskNotifyValueClear
// Clear the task notification value to ensure we block
ulTaskNotifyValueClear(cur_task, ULONG_MAX);
#endif
ulTaskNotifyTake(pdTRUE, portMAX_DELAY);

if(taskData.rc == 0){
Expand Down
7 changes: 6 additions & 1 deletion src/NimBLEScan.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -358,10 +358,15 @@ NimBLEScanResults NimBLEScan::start(uint32_t duration, bool is_continue) {
NIMBLE_LOGW(LOG_TAG, "Blocking scan called with duration = forever");
}

ble_task_data_t taskData = {nullptr, xTaskGetCurrentTaskHandle(),0, nullptr};
TaskHandle_t cur_task = xTaskGetCurrentTaskHandle();
ble_task_data_t taskData = {nullptr, cur_task, 0, nullptr};
m_pTaskData = &taskData;

if(start(duration, nullptr, is_continue)) {
#ifdef ulTaskNotifyValueClear
// Clear the task notification value to ensure we block
ulTaskNotifyValueClear(cur_task, ULONG_MAX);
#endif
ulTaskNotifyTake(pdTRUE, portMAX_DELAY);
}

Expand Down

0 comments on commit 335ad93

Please sign in to comment.