Skip to content

Commit e843bc7

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 * General cleanup
1 parent 5ac7272 commit e843bc7

File tree

2 files changed

+91
-114
lines changed

2 files changed

+91
-114
lines changed

‎src/NimBLEClient.cpp‎

Lines changed: 85 additions & 108 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,7 +712,7 @@ 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;
@@ -735,9 +722,9 @@ bool NimBLEClient::retrieveServices(const NimBLEUUID* uuidFilter) {
735722
NimBLETaskData taskData(this);
736723

737724
if (uuidFilter == nullptr) {
738-
rc = ble_gattc_disc_all_svcs(m_connHandle, NimBLEClient::serviceDiscoveredCB, &taskData);
725+
rc = ble_gattc_disc_all_svcs(m_connHandle, NimBLEClient::serviceDiscCB, &taskData);
739726
} else {
740-
rc = ble_gattc_disc_svc_by_uuid(m_connHandle, uuidFilter->getBase(), NimBLEClient::serviceDiscoveredCB, &taskData);
727+
rc = ble_gattc_disc_svc_by_uuid(m_connHandle, uuidFilter->getBase(), NimBLEClient::serviceDiscCB, &taskData);
741728
}
742729

743730
if (rc != 0) {
@@ -748,7 +735,10 @@ bool NimBLEClient::retrieveServices(const NimBLEUUID* uuidFilter) {
748735

749736
NimBLEUtils::taskWait(taskData, BLE_NPL_TIME_FOREVER);
750737
rc = taskData.m_flags;
751-
if (rc == 0 || rc == BLE_HS_EDONE) {
738+
if (rc == BLE_HS_EDONE) {
739+
if (out) {
740+
out = m_svcVec.back();
741+
}
752742
return true;
753743
}
754744

@@ -762,39 +752,29 @@ bool NimBLEClient::retrieveServices(const NimBLEUUID* uuidFilter) {
762752
* @details When a service is found or there is none left or there was an error
763753
* the API will call this and report findings.
764754
*/
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-
}
755+
int NimBLEClient::serviceDiscCB(uint16_t connHandle,
756+
const struct ble_gatt_error* error,
757+
const struct ble_gatt_svc* service,
758+
void* arg) {
759+
const int rc = error->status;
760+
auto pTaskData = (NimBLETaskData*)arg;
761+
auto pClient = (NimBLEClient*)pTaskData->m_pInstance;
762+
NIMBLE_LOGD(LOG_TAG, "Service Discovered >> status: %d handle: %d", rc, !rc ? service->start_handle : -1);
782763

783764
// Make sure the service discovery is for this device
784765
if (pClient->getConnHandle() != connHandle) {
785766
return 0;
786767
}
787768

788-
if (error->status == 0) {
789-
// Found a service - add it to the vector
769+
if (rc == 0) { // Found a service - add it to the vector
790770
pClient->m_svcVec.push_back(new NimBLERemoteService(pClient, service));
791771
return 0;
792772
}
793773

794774
NimBLEUtils::taskRelease(*pTaskData, error->status);
795-
NIMBLE_LOGD(LOG_TAG, "<< Service Discovered");
775+
NIMBLE_LOGD(LOG_TAG, "<< Service Discovered%s", (rc == BLE_HS_ENOTCONN) ? "; Disconnected" : "");
796776
return error->status;
797-
} // serviceDiscoveredCB
777+
} // serviceDiscCB
798778

799779
/**
800780
* @brief Get the value of a specific characteristic associated with a specific service.
@@ -803,10 +783,8 @@ int NimBLEClient::serviceDiscoveredCB(uint16_t connHandle,
803783
* @returns characteristic value or an empty value if not found.
804784
*/
805785
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());
786+
NIMBLE_LOGD(LOG_TAG, ">> getValue: serviceUUID: %s, characteristicUUID: %s",
787+
serviceUUID.toString().c_str(), characteristicUUID.toString().c_str());
810788

811789
NimBLEAttValue ret{};
812790
auto pService = getService(serviceUUID);
@@ -833,15 +811,13 @@ bool NimBLEClient::setValue(const NimBLEUUID& serviceUUID,
833811
const NimBLEUUID& characteristicUUID,
834812
const NimBLEAttValue& value,
835813
bool response) {
836-
NIMBLE_LOGD(LOG_TAG,
837-
">> setValue: serviceUUID: %s, characteristicUUID: %s",
838-
serviceUUID.toString().c_str(),
839-
characteristicUUID.toString().c_str());
814+
NIMBLE_LOGD(LOG_TAG, ">> setValue: serviceUUID: %s, characteristicUUID: %s",
815+
serviceUUID.toString().c_str(), characteristicUUID.toString().c_str());
840816

841817
bool ret = false;
842818
auto pService = getService(serviceUUID);
843819
if (pService != nullptr) {
844-
NimBLERemoteCharacteristic* pChar = pService->getCharacteristic(characteristicUUID);
820+
auto pChar = pService->getCharacteristic(characteristicUUID);
845821
if (pChar != nullptr) {
846822
ret = pChar->writeValue(value, response);
847823
}
@@ -858,11 +834,13 @@ bool NimBLEClient::setValue(const NimBLEUUID& serviceUUID,
858834
*/
859835
NimBLERemoteCharacteristic* NimBLEClient::getCharacteristic(uint16_t handle) {
860836
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-
}
837+
if (svc->getStartHandle() > handle && handle > svc->getEndHandle()) {
838+
continue;
839+
}
840+
841+
for (const auto& chr : svc->m_vChars) {
842+
if (chr->getHandle() == handle) {
843+
return chr;
866844
}
867845
}
868846
}
@@ -882,28 +860,28 @@ uint16_t NimBLEClient::getMTU() const {
882860
* @brief Callback for the MTU exchange API function.
883861
* @details When the MTU exchange is complete the API will call this and report the new MTU.
884862
*/
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);
863+
int NimBLEClient::exchangeMTUCB(uint16_t connHandle, const ble_gatt_error* error, uint16_t mtu, void* arg) {
864+
NIMBLE_LOGD(LOG_TAG, "exchangeMTUCB: status=%d, mtu=%d", error->status, mtu);
887865

888866
NimBLEClient* pClient = (NimBLEClient*)arg;
889-
if (pClient->getConnHandle() != conn_handle) {
867+
if (pClient->getConnHandle() != connHandle) {
890868
return 0;
891869
}
892870

893871
if (error->status != 0) {
894-
NIMBLE_LOGE(LOG_TAG, "exchangeMTUCb() rc=%d %s", error->status, NimBLEUtils::returnCodeToString(error->status));
872+
NIMBLE_LOGE(LOG_TAG, "exchangeMTUCB() rc=%d %s", error->status, NimBLEUtils::returnCodeToString(error->status));
895873
pClient->m_lastErr = error->status;
896874
}
897875

898876
return 0;
899-
} // exchangeMTUCb
877+
} // exchangeMTUCB
900878

901879
/**
902880
* @brief Begin the MTU exchange process with the server.
903881
* @returns true if the request was sent successfully.
904882
*/
905883
bool NimBLEClient::exchangeMTU() {
906-
int rc = ble_gattc_exchange_mtu(m_connHandle, NimBLEClient::exchangeMTUCb, this);
884+
int rc = ble_gattc_exchange_mtu(m_connHandle, NimBLEClient::exchangeMTUCB, this);
907885
if (rc != 0) {
908886
NIMBLE_LOGE(LOG_TAG, "MTU exchange error; rc=%d %s", rc, NimBLEUtils::returnCodeToString(rc));
909887
m_lastErr = rc;
@@ -989,25 +967,25 @@ int NimBLEClient::handleGapEvent(struct ble_gap_event* event, void* arg) {
989967
pClient->m_pClientCallbacks->onConnect(pClient);
990968
}
991969

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.
970+
if (!pClient->m_config.exchangeMTU) {
971+
break;
999972
}
1000-
} else {
1001-
pClient->m_connHandle = BLE_HS_CONN_HANDLE_NONE;
1002973

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

982+
pClient->m_connHandle = BLE_HS_CONN_HANDLE_NONE;
983+
if (pClient->m_config.asyncConnect) {
984+
pClient->m_pClientCallbacks->onConnectFail(pClient, rc);
985+
if (pClient->m_config.deleteOnConnectFail) {
986+
NimBLEDevice::deleteClient(pClient);
987+
}
988+
}
1011989
break;
1012990
} // BLE_GAP_EVENT_CONNECT
1013991

@@ -1035,23 +1013,23 @@ int NimBLEClient::handleGapEvent(struct ble_gap_event* event, void* arg) {
10351013
continue;
10361014
}
10371015

1038-
NIMBLE_LOGD(LOG_TAG,
1039-
"checking service %s for handle: %d",
1016+
NIMBLE_LOGD(LOG_TAG, "checking service %s for handle: %d",
10401017
svc->getUUID().toString().c_str(),
10411018
event->notify_rx.attr_handle);
10421019

10431020
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());
1021+
if (chr->getHandle() != event->notify_rx.attr_handle) {
1022+
continue;
1023+
}
10461024

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);
1025+
NIMBLE_LOGD(LOG_TAG, "Got Notification for characteristic %s", chr->toString().c_str());
1026+
uint32_t data_len = OS_MBUF_PKTLEN(event->notify_rx.om);
1027+
chr->m_value.setValue(event->notify_rx.om->om_data, data_len);
10491028

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;
1029+
if (chr->m_notifyCallback != nullptr) {
1030+
chr->m_notifyCallback(chr, event->notify_rx.om->om_data, data_len, !event->notify_rx.indication);
10541031
}
1032+
break;
10551033
}
10561034
}
10571035

@@ -1064,8 +1042,7 @@ int NimBLEClient::handleGapEvent(struct ble_gap_event* event, void* arg) {
10641042
return 0;
10651043
}
10661044
NIMBLE_LOGD(LOG_TAG, "Peer requesting to update connection parameters");
1067-
NIMBLE_LOGD(LOG_TAG,
1068-
"MinInterval: %d, MaxInterval: %d, Latency: %d, Timeout: %d",
1045+
NIMBLE_LOGD(LOG_TAG, "MinInterval: %d, MaxInterval: %d, Latency: %d, Timeout: %d",
10691046
event->conn_update_req.peer_params->itvl_min,
10701047
event->conn_update_req.peer_params->itvl_max,
10711048
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 serviceDiscCB(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)