Skip to content

Commit a41b6a1

Browse files
thekurtovicthekurtovic
committed
refactor(Client): Reduce nesting
* Renames serviceDiscoveredCB to serviceDiscCB. * Adds parameter out to retrieveServices, default value works as the original method. * out: if not nullptr, will allow caller to obtain the service * Rename serviceDiscoveredCB to svcDiscCB * General cleanup
1 parent 5ac7272 commit a41b6a1

File tree

2 files changed

+92
-119
lines changed

2 files changed

+92
-119
lines changed

‎src/NimBLEClient.cpp

Lines changed: 86 additions & 113 deletions
Original file line numberDiff line numberDiff line change
@@ -101,8 +101,8 @@ NimBLEClient::~NimBLEClient() {
101101
*/
102102
void NimBLEClient::deleteServices() {
103103
// Delete all the services.
104-
for (auto& it : m_svcVec) {
105-
delete it;
104+
for (auto& svc : m_svcVec) {
105+
delete svc;
106106
}
107107

108108
std::vector<NimBLERemoteService*>().swap(m_svcVec);
@@ -243,8 +243,7 @@ bool NimBLEClient::connect(const NimBLEAddress& address, bool deleteAttributes,
243243
break;
244244

245245
default:
246-
NIMBLE_LOGE(LOG_TAG,
247-
"Failed to connect to %s, rc=%d; %s",
246+
NIMBLE_LOGE(LOG_TAG, "Failed to connect to %s, rc=%d; %s",
248247
std::string(m_peerAddress).c_str(),
249248
rc,
250249
NimBLEUtils::returnCodeToString(rc));
@@ -631,48 +630,36 @@ NimBLERemoteService* NimBLEClient::getService(const char* uuid) {
631630
*/
632631
NimBLERemoteService* NimBLEClient::getService(const NimBLEUUID& uuid) {
633632
NIMBLE_LOGD(LOG_TAG, ">> getService: uuid: %s", uuid.toString().c_str());
633+
NimBLERemoteService *pSvc = nullptr;
634+
NimBLEUUID uuidTmp;
634635

635-
for (auto& it : m_svcVec) {
636-
if (it->getUUID() == uuid) {
636+
for (auto& svc : m_svcVec) {
637+
if (svc->getUUID() == uuid) {
637638
NIMBLE_LOGD(LOG_TAG, "<< getService: found the service with uuid: %s", uuid.toString().c_str());
638-
return it;
639+
return svc;
639640
}
640641
}
641642

642-
size_t prevSize = m_svcVec.size();
643-
if (retrieveServices(&uuid)) {
644-
if (m_svcVec.size() > prevSize) {
645-
return m_svcVec.back();
646-
}
643+
if (!retrieveServices(&uuid, pSvc) || pSvc) {
644+
goto Done;
645+
}
647646

648-
// If the request was successful but 16/32 bit uuid not found
649-
// try again with the 128 bit uuid.
650-
if (uuid.bitSize() == BLE_UUID_TYPE_16 || uuid.bitSize() == BLE_UUID_TYPE_32) {
651-
NimBLEUUID uuid128(uuid);
652-
uuid128.to128();
653-
if (retrieveServices(&uuid128)) {
654-
if (m_svcVec.size() > prevSize) {
655-
return m_svcVec.back();
656-
}
657-
}
658-
} else {
659-
// If the request was successful but the 128 bit uuid not found
660-
// try again with the 16 bit uuid.
661-
NimBLEUUID uuid16(uuid);
662-
uuid16.to16();
663-
// if the uuid was 128 bit but not of the BLE base type this check will fail
664-
if (uuid16.bitSize() == BLE_UUID_TYPE_16) {
665-
if (retrieveServices(&uuid16)) {
666-
if (m_svcVec.size() > prevSize) {
667-
return m_svcVec.back();
668-
}
669-
}
670-
}
671-
}
647+
// Try again with 128 bit uuid if request succeeded with no uuid found.
648+
if (uuid.bitSize() == BLE_UUID_TYPE_16 || uuid.bitSize() == BLE_UUID_TYPE_32) {
649+
uuidTmp = NimBLEUUID(uuid).to128();
650+
retrieveServices(&uuidTmp, pSvc);
651+
goto Done;
652+
}
653+
// Try again with 16 bit uuid if request succeeded with no uuid found.
654+
// If the uuid was 128 bit but not of the BLE base type this check will fail.
655+
uuidTmp = NimBLEUUID(uuid).to16();
656+
if (uuidTmp.bitSize() == BLE_UUID_TYPE_16) {
657+
retrieveServices(&uuidTmp, pSvc);
672658
}
673659

660+
Done:
674661
NIMBLE_LOGD(LOG_TAG, "<< getService: not found");
675-
return nullptr;
662+
return pSvc;
676663
} // getService
677664

678665
/**
@@ -725,20 +712,16 @@ bool NimBLEClient::discoverAttributes() {
725712
* * Here we ask the server for its set of services and wait until we have received them all.
726713
* @return true on success otherwise false if an error occurred
727714
*/
728-
bool NimBLEClient::retrieveServices(const NimBLEUUID* uuidFilter) {
715+
bool NimBLEClient::retrieveServices(const NimBLEUUID* uuidFilter, NimBLERemoteService *out) {
729716
if (!isConnected()) {
730717
NIMBLE_LOGE(LOG_TAG, "Disconnected, could not retrieve services -aborting");
731718
return false;
732719
}
733720

734-
int rc = 0;
735721
NimBLETaskData taskData(this);
736-
737-
if (uuidFilter == nullptr) {
738-
rc = ble_gattc_disc_all_svcs(m_connHandle, NimBLEClient::serviceDiscoveredCB, &taskData);
739-
} else {
740-
rc = ble_gattc_disc_svc_by_uuid(m_connHandle, uuidFilter->getBase(), NimBLEClient::serviceDiscoveredCB, &taskData);
741-
}
722+
int rc = (uuidFilter == nullptr)
723+
? ble_gattc_disc_all_svcs(m_connHandle, svcDiscCB, &taskData)
724+
: ble_gattc_disc_svc_by_uuid(m_connHandle, uuidFilter->getBase(), svcDiscCB, &taskData);
742725

743726
if (rc != 0) {
744727
NIMBLE_LOGE(LOG_TAG, "ble_gattc_disc_all_svcs: rc=%d %s", rc, NimBLEUtils::returnCodeToString(rc));
@@ -748,7 +731,10 @@ bool NimBLEClient::retrieveServices(const NimBLEUUID* uuidFilter) {
748731

749732
NimBLEUtils::taskWait(taskData, BLE_NPL_TIME_FOREVER);
750733
rc = taskData.m_flags;
751-
if (rc == 0 || rc == BLE_HS_EDONE) {
734+
if (rc == BLE_HS_EDONE) {
735+
if (out) {
736+
out = m_svcVec.back();
737+
}
752738
return true;
753739
}
754740

@@ -762,39 +748,29 @@ bool NimBLEClient::retrieveServices(const NimBLEUUID* uuidFilter) {
762748
* @details When a service is found or there is none left or there was an error
763749
* the API will call this and report findings.
764750
*/
765-
int NimBLEClient::serviceDiscoveredCB(uint16_t connHandle,
766-
const struct ble_gatt_error* error,
767-
const struct ble_gatt_svc* service,
768-
void* arg) {
769-
NIMBLE_LOGD(LOG_TAG,
770-
"Service Discovered >> status: %d handle: %d",
771-
error->status,
772-
(error->status == 0) ? service->start_handle : -1);
773-
774-
NimBLETaskData* pTaskData = (NimBLETaskData*)arg;
775-
NimBLEClient* pClient = (NimBLEClient*)pTaskData->m_pInstance;
776-
777-
if (error->status == BLE_HS_ENOTCONN) {
778-
NIMBLE_LOGE(LOG_TAG, "<< Service Discovered; Disconnected");
779-
NimBLEUtils::taskRelease(*pTaskData, error->status);
780-
return error->status;
781-
}
751+
int NimBLEClient::svcDiscCB(uint16_t connHandle,
752+
const struct ble_gatt_error* error,
753+
const struct ble_gatt_svc* service,
754+
void* arg) {
755+
const int rc = error->status;
756+
auto pTaskData = (NimBLETaskData*)arg;
757+
auto pClient = (NimBLEClient*)pTaskData->m_pInstance;
758+
NIMBLE_LOGD(LOG_TAG, "Service Discovered >> status: %d handle: %d", rc, !rc ? service->start_handle : -1);
782759

783760
// Make sure the service discovery is for this device
784761
if (pClient->getConnHandle() != connHandle) {
785762
return 0;
786763
}
787764

788-
if (error->status == 0) {
789-
// Found a service - add it to the vector
765+
if (rc == 0) { // Found a service - add it to the vector
790766
pClient->m_svcVec.push_back(new NimBLERemoteService(pClient, service));
791767
return 0;
792768
}
793769

794770
NimBLEUtils::taskRelease(*pTaskData, error->status);
795-
NIMBLE_LOGD(LOG_TAG, "<< Service Discovered");
771+
NIMBLE_LOGD(LOG_TAG, "<< Service Discovered%s", (rc == BLE_HS_ENOTCONN) ? "; Disconnected" : "");
796772
return error->status;
797-
} // serviceDiscoveredCB
773+
} // serviceDiscCB
798774

799775
/**
800776
* @brief Get the value of a specific characteristic associated with a specific service.
@@ -803,10 +779,8 @@ int NimBLEClient::serviceDiscoveredCB(uint16_t connHandle,
803779
* @returns characteristic value or an empty value if not found.
804780
*/
805781
NimBLEAttValue NimBLEClient::getValue(const NimBLEUUID& serviceUUID, const NimBLEUUID& characteristicUUID) {
806-
NIMBLE_LOGD(LOG_TAG,
807-
">> getValue: serviceUUID: %s, characteristicUUID: %s",
808-
serviceUUID.toString().c_str(),
809-
characteristicUUID.toString().c_str());
782+
NIMBLE_LOGD(LOG_TAG, ">> getValue: serviceUUID: %s, characteristicUUID: %s",
783+
serviceUUID.toString().c_str(), characteristicUUID.toString().c_str());
810784

811785
NimBLEAttValue ret{};
812786
auto pService = getService(serviceUUID);
@@ -833,15 +807,13 @@ bool NimBLEClient::setValue(const NimBLEUUID& serviceUUID,
833807
const NimBLEUUID& characteristicUUID,
834808
const NimBLEAttValue& value,
835809
bool response) {
836-
NIMBLE_LOGD(LOG_TAG,
837-
">> setValue: serviceUUID: %s, characteristicUUID: %s",
838-
serviceUUID.toString().c_str(),
839-
characteristicUUID.toString().c_str());
810+
NIMBLE_LOGD(LOG_TAG, ">> setValue: serviceUUID: %s, characteristicUUID: %s",
811+
serviceUUID.toString().c_str(), characteristicUUID.toString().c_str());
840812

841813
bool ret = false;
842814
auto pService = getService(serviceUUID);
843815
if (pService != nullptr) {
844-
NimBLERemoteCharacteristic* pChar = pService->getCharacteristic(characteristicUUID);
816+
auto pChar = pService->getCharacteristic(characteristicUUID);
845817
if (pChar != nullptr) {
846818
ret = pChar->writeValue(value, response);
847819
}
@@ -858,11 +830,13 @@ bool NimBLEClient::setValue(const NimBLEUUID& serviceUUID,
858830
*/
859831
NimBLERemoteCharacteristic* NimBLEClient::getCharacteristic(uint16_t handle) {
860832
for (const auto& svc : m_svcVec) {
861-
if (svc->getStartHandle() <= handle && handle <= svc->getEndHandle()) {
862-
for (const auto& chr : svc->m_vChars) {
863-
if (chr->getHandle() == handle) {
864-
return chr;
865-
}
833+
if (svc->getStartHandle() > handle && handle > svc->getEndHandle()) {
834+
continue;
835+
}
836+
837+
for (const auto& chr : svc->m_vChars) {
838+
if (chr->getHandle() == handle) {
839+
return chr;
866840
}
867841
}
868842
}
@@ -882,28 +856,28 @@ uint16_t NimBLEClient::getMTU() const {
882856
* @brief Callback for the MTU exchange API function.
883857
* @details When the MTU exchange is complete the API will call this and report the new MTU.
884858
*/
885-
int NimBLEClient::exchangeMTUCb(uint16_t conn_handle, const ble_gatt_error* error, uint16_t mtu, void* arg) {
886-
NIMBLE_LOGD(LOG_TAG, "exchangeMTUCb: status=%d, mtu=%d", error->status, mtu);
859+
int NimBLEClient::exchangeMTUCB(uint16_t connHandle, const ble_gatt_error* error, uint16_t mtu, void* arg) {
860+
NIMBLE_LOGD(LOG_TAG, "exchangeMTUCB: status=%d, mtu=%d", error->status, mtu);
887861

888862
NimBLEClient* pClient = (NimBLEClient*)arg;
889-
if (pClient->getConnHandle() != conn_handle) {
863+
if (pClient->getConnHandle() != connHandle) {
890864
return 0;
891865
}
892866

893867
if (error->status != 0) {
894-
NIMBLE_LOGE(LOG_TAG, "exchangeMTUCb() rc=%d %s", error->status, NimBLEUtils::returnCodeToString(error->status));
868+
NIMBLE_LOGE(LOG_TAG, "exchangeMTUCB() rc=%d %s", error->status, NimBLEUtils::returnCodeToString(error->status));
895869
pClient->m_lastErr = error->status;
896870
}
897871

898872
return 0;
899-
} // exchangeMTUCb
873+
} // exchangeMTUCB
900874

901875
/**
902876
* @brief Begin the MTU exchange process with the server.
903877
* @returns true if the request was sent successfully.
904878
*/
905879
bool NimBLEClient::exchangeMTU() {
906-
int rc = ble_gattc_exchange_mtu(m_connHandle, NimBLEClient::exchangeMTUCb, this);
880+
int rc = ble_gattc_exchange_mtu(m_connHandle, NimBLEClient::exchangeMTUCB, this);
907881
if (rc != 0) {
908882
NIMBLE_LOGE(LOG_TAG, "MTU exchange error; rc=%d %s", rc, NimBLEUtils::returnCodeToString(rc));
909883
m_lastErr = rc;
@@ -989,25 +963,25 @@ int NimBLEClient::handleGapEvent(struct ble_gap_event* event, void* arg) {
989963
pClient->m_pClientCallbacks->onConnect(pClient);
990964
}
991965

992-
if (pClient->m_config.exchangeMTU) {
993-
if (!pClient->exchangeMTU()) {
994-
rc = pClient->m_lastErr; // sets the error in the task data
995-
break;
996-
}
997-
998-
return 0; // return as we may have a task waiting for the MTU before releasing it.
966+
if (!pClient->m_config.exchangeMTU) {
967+
break;
999968
}
1000-
} else {
1001-
pClient->m_connHandle = BLE_HS_CONN_HANDLE_NONE;
1002969

1003-
if (pClient->m_config.asyncConnect) {
1004-
pClient->m_pClientCallbacks->onConnectFail(pClient, rc);
1005-
if (pClient->m_config.deleteOnConnectFail) {
1006-
NimBLEDevice::deleteClient(pClient);
1007-
}
970+
if (!pClient->exchangeMTU()) {
971+
rc = pClient->m_lastErr; // sets the error in the task data
972+
break;
1008973
}
974+
975+
return 0; // return as we may have a task waiting for the MTU before releasing it.
1009976
}
1010977

978+
pClient->m_connHandle = BLE_HS_CONN_HANDLE_NONE;
979+
if (pClient->m_config.asyncConnect) {
980+
pClient->m_pClientCallbacks->onConnectFail(pClient, rc);
981+
if (pClient->m_config.deleteOnConnectFail) {
982+
NimBLEDevice::deleteClient(pClient);
983+
}
984+
}
1011985
break;
1012986
} // BLE_GAP_EVENT_CONNECT
1013987

@@ -1035,23 +1009,23 @@ int NimBLEClient::handleGapEvent(struct ble_gap_event* event, void* arg) {
10351009
continue;
10361010
}
10371011

1038-
NIMBLE_LOGD(LOG_TAG,
1039-
"checking service %s for handle: %d",
1012+
NIMBLE_LOGD(LOG_TAG, "checking service %s for handle: %d",
10401013
svc->getUUID().toString().c_str(),
10411014
event->notify_rx.attr_handle);
10421015

10431016
for (const auto& chr : svc->m_vChars) {
1044-
if (chr->getHandle() == event->notify_rx.attr_handle) {
1045-
NIMBLE_LOGD(LOG_TAG, "Got Notification for characteristic %s", chr->toString().c_str());
1017+
if (chr->getHandle() != event->notify_rx.attr_handle) {
1018+
continue;
1019+
}
10461020

1047-
uint32_t data_len = OS_MBUF_PKTLEN(event->notify_rx.om);
1048-
chr->m_value.setValue(event->notify_rx.om->om_data, data_len);
1021+
NIMBLE_LOGD(LOG_TAG, "Got Notification for characteristic %s", chr->toString().c_str());
1022+
uint32_t data_len = OS_MBUF_PKTLEN(event->notify_rx.om);
1023+
chr->m_value.setValue(event->notify_rx.om->om_data, data_len);
10491024

1050-
if (chr->m_notifyCallback != nullptr) {
1051-
chr->m_notifyCallback(chr, event->notify_rx.om->om_data, data_len, !event->notify_rx.indication);
1052-
}
1053-
break;
1025+
if (chr->m_notifyCallback != nullptr) {
1026+
chr->m_notifyCallback(chr, event->notify_rx.om->om_data, data_len, !event->notify_rx.indication);
10541027
}
1028+
break;
10551029
}
10561030
}
10571031

@@ -1064,8 +1038,7 @@ int NimBLEClient::handleGapEvent(struct ble_gap_event* event, void* arg) {
10641038
return 0;
10651039
}
10661040
NIMBLE_LOGD(LOG_TAG, "Peer requesting to update connection parameters");
1067-
NIMBLE_LOGD(LOG_TAG,
1068-
"MinInterval: %d, MaxInterval: %d, Latency: %d, Timeout: %d",
1041+
NIMBLE_LOGD(LOG_TAG, "MinInterval: %d, MaxInterval: %d, Latency: %d, Timeout: %d",
10691042
event->conn_update_req.peer_params->itvl_min,
10701043
event->conn_update_req.peer_params->itvl_max,
10711044
event->conn_update_req.peer_params->latency,

‎src/NimBLEClient.h

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -116,13 +116,13 @@ class NimBLEClient {
116116
NimBLEClient(const NimBLEClient&) = delete;
117117
NimBLEClient& operator=(const NimBLEClient&) = delete;
118118

119-
bool retrieveServices(const NimBLEUUID* uuidFilter = nullptr);
119+
bool retrieveServices(const NimBLEUUID* uuidFilter = nullptr, NimBLERemoteService *out = nullptr);
120120
static int handleGapEvent(struct ble_gap_event* event, void* arg);
121-
static int exchangeMTUCb(uint16_t conn_handle, const ble_gatt_error* error, uint16_t mtu, void* arg);
122-
static int serviceDiscoveredCB(uint16_t connHandle,
123-
const struct ble_gatt_error* error,
124-
const struct ble_gatt_svc* service,
125-
void* arg);
121+
static int exchangeMTUCB(uint16_t connHandle, const ble_gatt_error* error, uint16_t mtu, void* arg);
122+
static int svcDiscCB(uint16_t connHandle,
123+
const struct ble_gatt_error* error,
124+
const struct ble_gatt_svc* service,
125+
void* arg);
126126

127127
NimBLEAddress m_peerAddress;
128128
mutable int m_lastErr;

0 commit comments

Comments
 (0)