refactor(RemoteServ): Reduce nesting

* Adds parameter out to retrieveCharacteristics, default valuel works as
  the original method.
    * out: if not nullptr, will allow caller to obtain the characteristic
* Add check to retrieveCharacteristics, return if found the last handle
* General cleanup

Author: thekurtovic

src/NimBLERemoteService.cpp

@@ -76,46 +76,31 @@ NimBLERemoteCharacteristic* NimBLERemoteService::getCharacteristic(const char* u
  */
 NimBLERemoteCharacteristic* NimBLERemoteService::getCharacteristic(const NimBLEUUID& uuid) const {
     NIMBLE_LOGD(LOG_TAG, ">> getCharacteristic: uuid: %s", uuid.toString().c_str());
-    NimBLERemoteCharacteristic* pChar     = nullptr;
-    size_t                      prev_size = m_vChars.size();
+    NimBLERemoteCharacteristic* pChar = nullptr;
+    NimBLEUUID                  uuidTmp;
 
-    for (const auto& it : m_vChars) {
-        if (it->getUUID() == uuid) {
-            pChar = it;
+    for (const auto& chr : m_vChars) {
+        if (chr->getUUID() == uuid) {
+            pChar = chr;
             goto Done;
         }
     }
 
-    if (retrieveCharacteristics(&uuid)) {
-        if (m_vChars.size() > prev_size) {
-            pChar = m_vChars.back();
-            goto Done;
-        }
+    if (!retrieveCharacteristics(&uuid, pChar) || pChar) {
+        goto Done;
+    }
 
-        // If the request was successful but 16/32 bit uuid not found
-        // try again with the 128 bit uuid.
-        if (uuid.bitSize() == BLE_UUID_TYPE_16 || uuid.bitSize() == BLE_UUID_TYPE_32) {
-            NimBLEUUID uuid128(uuid);
-            uuid128.to128();
-            if (retrieveCharacteristics(&uuid128)) {
-                if (m_vChars.size() > prev_size) {
-                    pChar = m_vChars.back();
-                }
-            }
-        } else {
-            // If the request was successful but the 128 bit uuid not found
-            // try again with the 16 bit uuid.
-            NimBLEUUID uuid16(uuid);
-            uuid16.to16();
-            // if the uuid was 128 bit but not of the BLE base type this check will fail
-            if (uuid16.bitSize() == BLE_UUID_TYPE_16) {
-                if (retrieveCharacteristics(&uuid16)) {
-                    if (m_vChars.size() > prev_size) {
-                        pChar = m_vChars.back();
-                    }
-                }
-            }
-        }
+    // Try again with 128 bit uuid if request succeeded with no uuid found.
+    if (uuid.bitSize() == BLE_UUID_TYPE_16 || uuid.bitSize() == BLE_UUID_TYPE_32) {
+        uuidTmp = NimBLEUUID(uuid).to128();
+        retrieveCharacteristics(&uuidTmp, pChar);
+        goto Done;
+    }
+    // Try again with 16 bit uuid if request succeeded with no uuid found.
+    // If the uuid was 128 bit but not of the BLE base type this check will fail.
+    uuidTmp = NimBLEUUID(uuid).to16();
+    if (uuidTmp.bitSize() == BLE_UUID_TYPE_16) {
+        retrieveCharacteristics(&uuidTmp, pChar);
     }
 
 Done:
@@ -143,77 +128,67 @@ const std::vector<NimBLERemoteCharacteristic*>& NimBLERemoteService::getCharacte
  * @brief Callback for Characteristic discovery.
  * @return success == 0 or error code.
  */
-int NimBLERemoteService::characteristicDiscCB(uint16_t              conn_handle,
+int NimBLERemoteService::characteristicDiscCB(uint16_t              connHandle,
                                               const ble_gatt_error* error,
                                               const ble_gatt_chr*   chr,
                                               void*                 arg) {
-    NIMBLE_LOGD(LOG_TAG,
-                "Characteristic Discovery >> status: %d handle: %d",
-                error->status,
-                (error->status == 0) ? chr->def_handle : -1);
+    const int  rc        = error->status;
     auto       pTaskData = (NimBLETaskData*)arg;
     const auto pSvc      = (NimBLERemoteService*)pTaskData->m_pInstance;
-
-    if (error->status == BLE_HS_ENOTCONN) {
-        NIMBLE_LOGE(LOG_TAG, "<< Characteristic Discovery; Not connected");
-        NimBLEUtils::taskRelease(*pTaskData, error->status);
-        return error->status;
-    }
+    NIMBLE_LOGD(LOG_TAG, "Characteristic Discovery >> status: %d handle: %d", rc, (rc == 0) ? chr->def_handle : -1);
 
     // Make sure the discovery is for this device
-    if (pSvc->getClient()->getConnHandle() != conn_handle) {
+    if (pSvc->getClient()->getConnHandle() != connHandle) {
         return 0;
     }
 
-    if (error->status == 0) {
+    if (rc == 0) {
         pSvc->m_vChars.push_back(new NimBLERemoteCharacteristic(pSvc, chr));
         return 0;
     }
 
-    NimBLEUtils::taskRelease(*pTaskData, error->status);
-    NIMBLE_LOGD(LOG_TAG, "<< Characteristic Discovery");
-    return error->status;
+    NimBLEUtils::taskRelease(*pTaskData, rc);
+    NIMBLE_LOGD(LOG_TAG, "<< Characteristic Discovery%s", (rc == BLE_HS_ENOTCONN) ? "; Not connected" : "");
+    return rc;
 }
 
 /**
  * @brief Retrieve all the characteristics for this service.
  * This function will not return until we have all the characteristics.
- * @return True if successful.
+ * @return True if successfully retrieved, success = BLE_HS_EDONE.
  */
-bool NimBLERemoteService::retrieveCharacteristics(const NimBLEUUID* uuidFilter) const {
+bool NimBLERemoteService::retrieveCharacteristics(const NimBLEUUID* uuid, NimBLERemoteCharacteristic* out) const {
     NIMBLE_LOGD(LOG_TAG, ">> retrieveCharacteristics()");
-    int            rc = 0;
     NimBLETaskData taskData(const_cast<NimBLERemoteService*>(this));
+    const uint16_t connHandle = m_pClient->getConnHandle();
+    const uint16_t endHandle  = getEndHandle();
+    const uint16_t handle     = getHandle();
 
-    if (uuidFilter == nullptr) {
-        rc = ble_gattc_disc_all_chrs(m_pClient->getConnHandle(),
-                                     getHandle(),
-                                     getEndHandle(),
-                                     NimBLERemoteService::characteristicDiscCB,
-                                     &taskData);
-    } else {
-        rc = ble_gattc_disc_chrs_by_uuid(m_pClient->getConnHandle(),
-                                         getHandle(),
-                                         getEndHandle(),
-                                         uuidFilter->getBase(),
-                                         NimBLERemoteService::characteristicDiscCB,
-                                         &taskData);
+    // If this is the last handle then there are no more characteristics
+    if (handle == endHandle) {
+        NIMBLE_LOGD(LOG_TAG, "<< retrieveCharacteristics(): found %d characteristics.", m_vChars.size());
+        return true;
     }
 
+    int rc = (uuid == nullptr)
+           ? ble_gattc_disc_all_chrs(connHandle, handle, endHandle, characteristicDiscCB, &taskData)
+           : ble_gattc_disc_chrs_by_uuid(connHandle, handle, endHandle, uuid->getBase(), characteristicDiscCB, &taskData);
+
     if (rc != 0) {
         NIMBLE_LOGE(LOG_TAG, "ble_gattc_disc_chrs rc=%d %s", rc, NimBLEUtils::returnCodeToString(rc));
         return false;
     }
 
     NimBLEUtils::taskWait(taskData, BLE_NPL_TIME_FOREVER);
     rc = taskData.m_flags;
-    if (rc == 0 || rc == BLE_HS_EDONE) {
-        NIMBLE_LOGD(LOG_TAG, "<< retrieveCharacteristics()");
-        return true;
+    if (rc != BLE_HS_EDONE) {
+        NIMBLE_LOGE(LOG_TAG, "<< retrieveCharacteristics(): failed: rc=%d %s", rc, NimBLEUtils::returnCodeToString(rc));
+        return false;
     }
 
-    NIMBLE_LOGE(LOG_TAG, "<< retrieveCharacteristics() rc=%d %s", rc, NimBLEUtils::returnCodeToString(rc));
-    return false;
+    *out = m_vChars.back();
+    NIMBLE_LOGD(LOG_TAG, "<< retrieveCharacteristics(): found %d characteristics.", m_vChars.size());
+    return true;
 } // retrieveCharacteristics
 
 /**
@@ -231,11 +206,8 @@ NimBLEClient* NimBLERemoteService::getClient() const {
  */
 NimBLEAttValue NimBLERemoteService::getValue(const NimBLEUUID& uuid) const {
     const auto pChar = getCharacteristic(uuid);
-    if (pChar) {
-        return pChar->readValue();
-    }
-
-    return NimBLEAttValue{};
+    return pChar ? pChar->readValue()
+                 : NimBLEAttValue{};
 } // readValue
 
 /**
@@ -246,11 +218,8 @@ NimBLEAttValue NimBLERemoteService::getValue(const NimBLEUUID& uuid) const {
  */
 bool NimBLERemoteService::setValue(const NimBLEUUID& uuid, const NimBLEAttValue& value) const {
     const auto pChar = getCharacteristic(uuid);
-    if (pChar) {
-        return pChar->writeValue(value);
-    }
-
-    return false;
+    return pChar ? pChar->writeValue(value)
+                 : false;
 } // setValue
 
 /**
@@ -260,8 +229,8 @@ bool NimBLERemoteService::setValue(const NimBLEUUID& uuid, const NimBLEAttValue&
  * them. This method does just that.
  */
 void NimBLERemoteService::deleteCharacteristics() const {
-    for (const auto& it : m_vChars) {
-        delete it;
+    for (const auto& chr : m_vChars) {
+        delete chr;
     }
     std::vector<NimBLERemoteCharacteristic*>{}.swap(m_vChars);
 } // deleteCharacteristics

src/NimBLERemoteService.h

@@ -53,8 +53,9 @@ class NimBLERemoteService : public NimBLEAttribute {
 
     NimBLERemoteService(NimBLEClient* pClient, const struct ble_gatt_svc* service);
     ~NimBLERemoteService();
-    bool       retrieveCharacteristics(const NimBLEUUID* uuidFilter = nullptr) const;
-    static int characteristicDiscCB(uint16_t                     conn_handle,
+    bool       retrieveCharacteristics(const NimBLEUUID* uuid = nullptr,
+                                       NimBLERemoteCharacteristic* out = nullptr) const;
+    static int characteristicDiscCB(uint16_t                     connHandle,
                                     const struct ble_gatt_error* error,
                                     const struct ble_gatt_chr*   chr,
                                     void*                        arg);