Replace semaphores with task notifications. (#9)

* Replace all semaphores with task notifications.

* use critical sections to prevent concurrent data access.

* Ensure scan stop has been called before connecting.

* Optimize and cleanup

* Add template casting to NimBLERemoteDescriptor::readValue()

* Removed storage of the descriptor value read as it did not serve any purpose.
This commit is contained in:
h2zero
2020-06-21 22:07:01 -06:00
committed by GitHub
parent 5bc9d59646
commit f5541d18de
11 changed files with 368 additions and 401 deletions
+93 -94
View File
@@ -60,6 +60,7 @@ static const char* LOG_TAG = "NimBLERemoteCharacteristic";
m_pRemoteService = pRemoteService;
m_notifyCallback = nullptr;
m_timestamp = 0;
m_valMux = portMUX_INITIALIZER_UNLOCKED;
} // NimBLERemoteCharacteristic
@@ -67,7 +68,7 @@ static const char* LOG_TAG = "NimBLERemoteCharacteristic";
*@brief Destructor.
*/
NimBLERemoteCharacteristic::~NimBLERemoteCharacteristic() {
deleteDescriptors(); // Release resources for any descriptor information we may have allocated.
deleteDescriptors();
} // ~NimBLERemoteCharacteristic
/*
@@ -147,12 +148,12 @@ int NimBLERemoteCharacteristic::descriptorDiscCB(uint16_t conn_handle,
NIMBLE_LOGD(LOG_TAG,"Descriptor Discovered >> status: %d handle: %d",
error->status, (error->status == 0) ? dsc->handle : -1);
disc_filter_t *filter = (disc_filter_t*)arg;
NimBLEUUID *uuid_filter = (NimBLEUUID*)filter->uuid;
NimBLERemoteCharacteristic *characteristic = (NimBLERemoteCharacteristic*)filter->attribute;
desc_filter_t *filter = (desc_filter_t*)arg;
const NimBLEUUID *uuid_filter = filter->uuid;
ble_task_data_t *pTaskData = (ble_task_data_t*)filter->task_data;
NimBLERemoteCharacteristic *characteristic = (NimBLERemoteCharacteristic*)pTaskData->pATT;
int rc=0;
// Make sure the discovery is for this device
if(characteristic->getRemoteService()->getClient()->getConnId() != conn_handle){
return 0;
}
@@ -172,7 +173,7 @@ int NimBLERemoteCharacteristic::descriptorDiscCB(uint16_t conn_handle,
rc = BLE_HS_EDONE;
}
}
// Found a descriptor - add it to the vector
NimBLERemoteDescriptor* pNewRemoteDescriptor = new NimBLERemoteDescriptor(characteristic, dsc);
characteristic->m_descriptorVector.push_back(pNewRemoteDescriptor);
break;
@@ -182,18 +183,20 @@ int NimBLERemoteCharacteristic::descriptorDiscCB(uint16_t conn_handle,
break;
}
/** If rc == BLE_HS_EDONE, release the semaphore with a success error code and stop the discovery process.
/** If rc == BLE_HS_EDONE, resume the task with a success error code and stop the discovery process.
* Else if rc == 0, just return 0 to continue the discovery until we get BLE_HS_EDONE.
* If we get any other error code tell the application to abort by returning non-zero in the semaphore rc.
* If we get any other error code tell the application to abort by returning non-zero in the rc.
*/
if (rc == BLE_HS_EDONE) {
characteristic->m_semaphoreGetDescEvt.give(0);
pTaskData->rc = 0;
xTaskNotifyGive(pTaskData->task);
} else if(rc != 0) {
/* Error; abort discovery. */
// pass error code to semaphore waiting
characteristic->m_semaphoreGetDescEvt.give(rc);
// Error; abort discovery.
pTaskData->rc = rc;
xTaskNotifyGive(pTaskData->task);
}
NIMBLE_LOGD(LOG_TAG,"<< Descriptor Discovered. status: %d", rc);
NIMBLE_LOGD(LOG_TAG,"<< Descriptor Discovered. status: %d", pTaskData->rc);
return rc;
}
@@ -206,11 +209,8 @@ bool NimBLERemoteCharacteristic::retrieveDescriptors(const NimBLEUUID *uuid_filt
NIMBLE_LOGD(LOG_TAG, ">> retrieveDescriptors() for characteristic: %s", getUUID().toString().c_str());
int rc = 0;
disc_filter_t filter;
filter.uuid = uuid_filter;
filter.attribute = this;
m_semaphoreGetDescEvt.take("retrieveDescriptors");
ble_task_data_t taskData = {this, xTaskGetCurrentTaskHandle(), 0, nullptr};
desc_filter_t filter = {uuid_filter, &taskData};
rc = ble_gattc_disc_all_dscs(getRemoteService()->getClient()->getConnId(),
m_handle,
@@ -219,11 +219,12 @@ bool NimBLERemoteCharacteristic::retrieveDescriptors(const NimBLEUUID *uuid_filt
&filter);
if (rc != 0) {
NIMBLE_LOGE(LOG_TAG, "ble_gattc_disc_all_chrs: rc=%d %s", rc, NimBLEUtils::returnCodeToString(rc));
m_semaphoreGetDescEvt.give();
return false;
}
if(m_semaphoreGetDescEvt.wait("retrieveDescriptors") != 0) {
ulTaskNotifyTake(pdTRUE, portMAX_DELAY);
if(taskData.rc != 0) {
return false;
}
@@ -337,6 +338,22 @@ NimBLEUUID NimBLERemoteCharacteristic::getUUID() {
} // getUUID
/**
* @brief Get the value of the remote characteristic.
* @return The value of the remote characteristic.
*/
std::string NimBLERemoteCharacteristic::getValue(time_t *timestamp) {
portENTER_CRITICAL(&m_valMux);
std::string value = m_value;
if(timestamp != nullptr) {
*timestamp = m_timestamp;
}
portEXIT_CRITICAL(&m_valMux);
return value;
}
/**
* @brief Read an unsigned 16 bit value
* @return The unsigned 16 bit value.
@@ -384,33 +401,31 @@ std::string NimBLERemoteCharacteristic::readValue(time_t *timestamp) {
NIMBLE_LOGD(LOG_TAG, ">> readValue(): uuid: %s, handle: %d 0x%.2x",
getUUID().toString().c_str(), getHandle(), getHandle());
int rc = 0;
int retryCount = 1;
NimBLEClient* pClient = getRemoteService()->getClient();
std::string value;
// Check to see that we are connected.
if (!pClient->isConnected()) {
NIMBLE_LOGE(LOG_TAG, "Disconnected");
return "";
return value;
}
do {
m_semaphoreReadCharEvt.take("readValue");
// Clear the value before reading.
m_value = "";
int rc = 0;
int retryCount = 1;
ble_task_data_t taskData = {this, xTaskGetCurrentTaskHandle(),0, &value};
do {
rc = ble_gattc_read_long(pClient->getConnId(), m_handle, 0,
NimBLERemoteCharacteristic::onReadCB,
this);
&taskData);
if (rc != 0) {
NIMBLE_LOGE(LOG_TAG, "Error: Failed to read characteristic; rc=%d, %s",
rc, NimBLEUtils::returnCodeToString(rc));
m_semaphoreReadCharEvt.give(0);
return "";
return value;
}
rc = m_semaphoreReadCharEvt.wait("readValue");
ulTaskNotifyTake(pdTRUE, portMAX_DELAY);
rc = taskData.rc;
switch(rc){
case 0:
case BLE_HS_EDONE:
@@ -428,39 +443,24 @@ std::string NimBLERemoteCharacteristic::readValue(time_t *timestamp) {
break;
/* Else falls through. */
default:
return "";
NIMBLE_LOGE(LOG_TAG, "<< readValue rc=%d", rc);
return value;
}
} while(rc != 0 && retryCount--);
NIMBLE_LOGD(LOG_TAG, "<< readValue(): length: %d", m_value.length());
m_semaphoreReadCharEvt.take("returnValue");
std::string value = m_value;
portENTER_CRITICAL(&m_valMux);
m_value = value;
m_timestamp = time(nullptr);
if(timestamp != nullptr) {
*timestamp = m_timestamp;
}
m_semaphoreReadCharEvt.give();
portEXIT_CRITICAL(&m_valMux);
NIMBLE_LOGD(LOG_TAG, "<< readValue length: %d rc=%d", value.length(), rc);
return value;
} // readValue
/**
* @brief Get the value of the remote characteristic.
* @return The value of the remote characteristic.
*/
std::string NimBLERemoteCharacteristic::getValue(time_t *timestamp) {
m_semaphoreReadCharEvt.take("getValue");
std::string value = m_value;
if(timestamp != nullptr) {
*timestamp = m_timestamp;
}
m_semaphoreReadCharEvt.give();
return value;
}
/**
* @brief Callback for characteristic read operation.
* @return 0 or error code.
@@ -469,27 +469,35 @@ int NimBLERemoteCharacteristic::onReadCB(uint16_t conn_handle,
const struct ble_gatt_error *error,
struct ble_gatt_attr *attr, void *arg)
{
NimBLERemoteCharacteristic* characteristic = (NimBLERemoteCharacteristic*)arg;
ble_task_data_t *pTaskData = (ble_task_data_t*)arg;
NimBLERemoteCharacteristic *characteristic = (NimBLERemoteCharacteristic*)pTaskData->pATT;
uint16_t conn_id = characteristic->getRemoteService()->getClient()->getConnId();
// Make sure the read is for this client
if(conn_id != conn_handle) {
return 0;
}
NIMBLE_LOGI(LOG_TAG, "Read complete; status=%d conn_handle=%d", error->status, conn_handle);
if(error->status == 0) {
if(attr) {
NIMBLE_LOGD(LOG_TAG, "Got %d bytes", attr->om->om_len);
std::string *strBuf = (std::string*)pTaskData->buf;
int rc = error->status;
characteristic->m_value += std::string((char*) attr->om->om_data, attr->om->om_len);
characteristic->m_timestamp = time(nullptr);
return 0;
if(rc == 0) {
if(attr) {
if(((*strBuf).length() + attr->om->om_len) > BLE_ATT_ATTR_MAX_LEN) {
rc = BLE_ATT_ERR_INVALID_ATTR_VALUE_LEN;
} else {
NIMBLE_LOGD(LOG_TAG, "Got %d bytes", attr->om->om_len);
(*strBuf) += std::string((char*) attr->om->om_data, attr->om->om_len);
return 0;
}
}
}
// Read complete release semaphore and let the app can continue.
characteristic->m_semaphoreReadCharEvt.give(error->status);
return 0;
pTaskData->rc = rc;
xTaskNotifyGive(pTaskData->task);
return rc;
}
@@ -509,7 +517,7 @@ bool NimBLERemoteCharacteristic::setNotify(uint16_t val, bool response, notify_c
return false;
}
m_notifyCallback = notifyCallback; // Save the notification callback.
m_notifyCallback = notifyCallback;
NIMBLE_LOGD(LOG_TAG, "<< setNotify()");
@@ -572,7 +580,7 @@ bool NimBLERemoteCharacteristic::registerForNotify(notify_callback notifyCallbac
*/
void NimBLERemoteCharacteristic::deleteDescriptors() {
NIMBLE_LOGD(LOG_TAG, ">> deleteDescriptors");
// Iterate through all the descriptors releasing their storage and erasing them from the vector.
for(auto &it: m_descriptorVector) {
delete it;
}
@@ -588,7 +596,7 @@ void NimBLERemoteCharacteristic::deleteDescriptors() {
*/
size_t NimBLERemoteCharacteristic::deleteDescriptor(const NimBLEUUID &uuid) {
NIMBLE_LOGD(LOG_TAG, ">> deleteDescriptor");
// Delete the requested descriptor.
for(auto it = m_descriptorVector.begin(); it != m_descriptorVector.end(); ++it) {
if((*it)->getUUID() == uuid) {
delete *it;
@@ -665,47 +673,45 @@ bool NimBLERemoteCharacteristic::writeValue(const uint8_t* data, size_t length,
NIMBLE_LOGD(LOG_TAG, ">> writeValue(), length: %d", length);
NimBLEClient* pClient = getRemoteService()->getClient();
int rc = 0;
int retryCount = 1;
uint16_t mtu;
// Check to see that we are connected.
if (!pClient->isConnected()) {
NIMBLE_LOGE(LOG_TAG, "Disconnected");
return false;
}
mtu = ble_att_mtu(pClient->getConnId()) - 3;
int rc = 0;
int retryCount = 1;
uint16_t mtu = ble_att_mtu(pClient->getConnId()) - 3;
// Check if the data length is longer than we can write in 1 connection event.
// Check if the data length is longer than we can write in one connection event.
// If so we must do a long write which requires a response.
if(length <= mtu && !response) {
rc = ble_gattc_write_no_rsp_flat(pClient->getConnId(), m_handle, data, length);
return (rc==0);
}
do {
m_semaphoreWriteCharEvt.take("writeValue");
ble_task_data_t taskData = {this, xTaskGetCurrentTaskHandle(), 0, nullptr};
do {
if(length > mtu) {
NIMBLE_LOGI(LOG_TAG,"long write %d bytes", length);
os_mbuf *om = ble_hs_mbuf_from_flat(data, length);
rc = ble_gattc_write_long(pClient->getConnId(), m_handle, 0, om,
NimBLERemoteCharacteristic::onWriteCB,
this);
&taskData);
} else {
rc = ble_gattc_write_flat(pClient->getConnId(), m_handle,
data, length,
NimBLERemoteCharacteristic::onWriteCB,
this);
&taskData);
}
if (rc != 0) {
NIMBLE_LOGE(LOG_TAG, "Error: Failed to write characteristic; rc=%d", rc);
m_semaphoreWriteCharEvt.give();
return false;
}
rc = m_semaphoreWriteCharEvt.wait("writeValue");
ulTaskNotifyTake(pdTRUE, portMAX_DELAY);
rc = taskData.rc;
switch(rc){
case 0:
@@ -725,11 +731,12 @@ bool NimBLERemoteCharacteristic::writeValue(const uint8_t* data, size_t length,
break;
/* Else falls through. */
default:
NIMBLE_LOGE(LOG_TAG, "<< writeValue, rc: %d", rc);
return false;
}
} while(rc != 0 && retryCount--);
NIMBLE_LOGD(LOG_TAG, "<< writeValue, rc: %d",rc);
NIMBLE_LOGD(LOG_TAG, "<< writeValue, rc: %d", rc);
return (rc == 0);
} // writeValue
@@ -742,29 +749,21 @@ int NimBLERemoteCharacteristic::onWriteCB(uint16_t conn_handle,
const struct ble_gatt_error *error,
struct ble_gatt_attr *attr, void *arg)
{
NimBLERemoteCharacteristic* characteristic = (NimBLERemoteCharacteristic*)arg;
ble_task_data_t *pTaskData = (ble_task_data_t*)arg;
NimBLERemoteCharacteristic *characteristic = (NimBLERemoteCharacteristic*)pTaskData->pATT;
// Make sure the discovery is for this device
if(characteristic->getRemoteService()->getClient()->getConnId() != conn_handle){
return 0;
}
NIMBLE_LOGI(LOG_TAG, "Write complete; status=%d conn_handle=%d", error->status, conn_handle);
characteristic->m_semaphoreWriteCharEvt.give(error->status);
pTaskData->rc = error->status;
xTaskNotifyGive(pTaskData->task);
return 0;
}
void NimBLERemoteCharacteristic::releaseSemaphores() {
for (auto &it: m_descriptorVector) {
it->releaseSemaphores();
}
m_semaphoreWriteCharEvt.give(1);
m_semaphoreGetDescEvt.give(1);
m_semaphoreReadCharEvt.give(1);
}
#endif // #if defined( CONFIG_BT_NIMBLE_ROLE_CENTRAL)
#endif /* CONFIG_BT_ENABLED */