-
Notifications
You must be signed in to change notification settings - Fork 86
Return error if calling function will block host task #419
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -876,6 +876,7 @@ void NimBLEDevice::onSync(void) { | |||||||||||||||
| */ | ||||||||||||||||
| void NimBLEDevice::host_task(void* param) { | ||||||||||||||||
| NIMBLE_LOGI(LOG_TAG, "NimBLE Started!"); | ||||||||||||||||
| NimBLEUtils::m_hostTaskHandle = ble_npl_get_current_task_id(); | ||||||||||||||||
| nimble_port_run(); // This function will return only when nimble_port_stop() is executed | ||||||||||||||||
| nimble_port_freertos_deinit(); | ||||||||||||||||
|
Comment on lines
+879
to
881
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
rg -n -C2 'm_hostTaskHandle|inHostTask\s*\(' srcRepository: h2zero/esp-nimble-cpp Length of output: 5298 Arr, swab the deck—reset This bit o' code be stashin' the host-task ID at startup but never clairin' it when ⚓ Proposed fix void NimBLEDevice::host_task(void* param) {
NIMBLE_LOGI(LOG_TAG, "NimBLE Started!");
NimBLEUtils::m_hostTaskHandle = ble_npl_get_current_task_id();
nimble_port_run(); // This function will return only when nimble_port_stop() is executed
+ NimBLEUtils::m_hostTaskHandle = nullptr;
nimble_port_freertos_deinit();
} // host_task📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||
| } // host_task | ||||||||||||||||
|
|
||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -155,6 +155,11 @@ bool NimBLERemoteCharacteristic::retrieveDescriptors(NimBLEDescriptorFilter* pFi | |
| */ | ||
| NimBLERemoteDescriptor* NimBLERemoteCharacteristic::getDescriptor(const NimBLEUUID& uuid) const { | ||
| NIMBLE_LOGD(LOG_TAG, ">> getDescriptor: uuid: %s", uuid.toString().c_str()); | ||
| if (NimBLEUtils::inHostTask()) { | ||
| NIMBLE_LOGE(LOG_TAG, "getDescriptor cannot be called from host task"); | ||
| return nullptr; | ||
|
Comment on lines
+158
to
+160
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Arrr, let cached descriptors pass; only block discovery. This return fires before the cache scan on Lines 167-172, so even an already discovered descriptor now looks missing from host-task callbacks. Downstream, ⚓ Proposed fix NimBLERemoteDescriptor* NimBLERemoteCharacteristic::getDescriptor(const NimBLEUUID& uuid) const {
NIMBLE_LOGD(LOG_TAG, ">> getDescriptor: uuid: %s", uuid.toString().c_str());
- if (NimBLEUtils::inHostTask()) {
- NIMBLE_LOGE(LOG_TAG, "getDescriptor cannot be called from host task");
- return nullptr;
- }
NimBLEUUID uuidTmp{uuid};
NimBLETaskData taskData(const_cast<NimBLERemoteCharacteristic*>(this));
NimBLEDescriptorFilter filter{nullptr, &uuidTmp, &taskData};
for (const auto& dsc : m_vDescriptors) {
if (dsc->getUUID() == uuid) {
filter.dsc = dsc;
goto Done;
}
}
+
+ if (NimBLEUtils::inHostTask()) {
+ NIMBLE_LOGE(LOG_TAG, "getDescriptor cannot discover from host task");
+ goto Done;
+ }
if (!retrieveDescriptors(&filter) || filter.dsc) {
goto Done;
}🤖 Prompt for AI Agents |
||
| } | ||
|
|
||
| NimBLEUUID uuidTmp{uuid}; | ||
| NimBLETaskData taskData(const_cast<NimBLERemoteCharacteristic*>(this)); | ||
| NimBLEDescriptorFilter filter{nullptr, &uuidTmp, &taskData}; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -75,6 +75,11 @@ NimBLERemoteCharacteristic* NimBLERemoteService::getCharacteristic(const char* u | |
| */ | ||
| NimBLERemoteCharacteristic* NimBLERemoteService::getCharacteristic(const NimBLEUUID& uuid) const { | ||
| NIMBLE_LOGD(LOG_TAG, ">> getCharacteristic: uuid: %s", uuid.toString().c_str()); | ||
| if (NimBLEUtils::inHostTask()) { | ||
| NIMBLE_LOGE(LOG_TAG, "getCharacteristic cannot be called from host task"); | ||
| return nullptr; | ||
|
Comment on lines
+78
to
+80
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Arrr, don’t sink cached characteristic lookups. This guard runs before the cache scan on Lines 85-91, so host-task callbacks can no longer fetch an already discovered characteristic even though no blocking call would occur. Gate only ⚓ Proposed fix NimBLERemoteCharacteristic* NimBLERemoteService::getCharacteristic(const NimBLEUUID& uuid) const {
NIMBLE_LOGD(LOG_TAG, ">> getCharacteristic: uuid: %s", uuid.toString().c_str());
- if (NimBLEUtils::inHostTask()) {
- NIMBLE_LOGE(LOG_TAG, "getCharacteristic cannot be called from host task");
- return nullptr;
- }
NimBLERemoteCharacteristic* pChar = nullptr;
for (const auto& it : m_vChars) {
if (it->getUUID() == uuid) {
pChar = it;
NIMBLE_LOGD(LOG_TAG, "<< getCharacteristic: found in cache");
return pChar;
}
}
+
+ if (NimBLEUtils::inHostTask()) {
+ NIMBLE_LOGE(LOG_TAG, "getCharacteristic cannot discover from host task");
+ return nullptr;
+ }
if (retrieveCharacteristics(&uuid, &pChar) && pChar == nullptr) {🤖 Prompt for AI Agents |
||
| } | ||
|
|
||
| NimBLERemoteCharacteristic* pChar = nullptr; | ||
|
|
||
| for (const auto& it : m_vChars) { | ||
|
|
@@ -116,6 +121,11 @@ NimBLERemoteCharacteristic* NimBLERemoteService::getCharacteristic(const NimBLEU | |
| * @return A read-only reference to the vector of characteristics retrieved for this service. | ||
| */ | ||
| const std::vector<NimBLERemoteCharacteristic*>& NimBLERemoteService::getCharacteristics(bool refresh) const { | ||
| if (refresh && NimBLEUtils::inHostTask()) { | ||
| NIMBLE_LOGE(LOG_TAG, "cannot refresh characteristics from host task"); | ||
| return m_vChars; | ||
| } | ||
|
|
||
| if (refresh) { | ||
| deleteCharacteristics(); | ||
| retrieveCharacteristics(); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Arrr, cached services should still be reachable.
This now rejects
getService()before the cache scan on Lines 688-693. That breaks host-task callbacks that only need a previously discovered service even though no blocking call would occur. Move the guard to just before the firstretrieveServices()call so only cache misses are rejected.⚓ Proposed fix
NimBLERemoteService* NimBLEClient::getService(const NimBLEUUID& uuid) { NIMBLE_LOGD(LOG_TAG, ">> getService: uuid: %s", uuid.toString().c_str()); - if (NimBLEUtils::inHostTask()) { - NIMBLE_LOGE(LOG_TAG, "getService cannot be called from host task"); - return nullptr; - } for (auto& it : m_svcVec) { if (it->getUUID() == uuid) { NIMBLE_LOGD(LOG_TAG, "<< getService: found the service with uuid: %s", uuid.toString().c_str()); return it; } } + + if (NimBLEUtils::inHostTask()) { + NIMBLE_LOGE(LOG_TAG, "getService cannot discover from host task"); + return nullptr; + } size_t prevSize = m_svcVec.size(); if (retrieveServices(&uuid)) {🤖 Prompt for AI Agents