refactor(RemoteServ): Reduce nesting

[thekurtovic]

• Adds parameter out to retrieveCharacteristics, default value 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
• Rename characteristicDiscCB to chrDiscCB
• General cleanup

[thekurtovic]

Same treatment as previous PR. I assumed similar rules apply here, but let me know if it's wrong. NimBLEClient also has a similar section of code. I was thinking of making a class which the other 3 classes will be derived from, then move the retrieve function into the new class and make it a template. Would look something like this

NimBLERemoteCharacteristic* NimBLERemoteService::getCharacteristic(const NimBLEUUID& uuid) const {
    NIMBLE_LOGD(LOG_TAG, ">> getCharacteristic: uuid: %s", uuid.toString().c_str());
    NimBLERemoteCharacteristic* pChar = nullptr;

    retrieve(uuid, pChar, m_vChars, [this](const NimBLEUUID* u, void* o) {
        return retrieveCharacteristics(u, (NimBLERemoteCharacteristic*)o);
    });

Might have to roll back to bool retrieveDescriptors(const NimBLEUUID* uuidFilter = nullptr, NimBLERemoteDescriptor* out = nullptr) const; in NimBLERemoteCharacteristic to make it work though.

[h2zero]

Looks good and I agree with the consolidation thought. Not sure if creating/inheriting from a class for this purpose is the way to go because the client class would also need to inherit from it. Maybe a utility function instead?

[thekurtovic]

Sure, that works too.

[thekurtovic]

Does retrieveCharacteristics need a check similar to https://github.com/h2zero/esp-nimble-cpp/commit/8158a16fbf3d716fd2dd16f6f8990c12265aefe7?

[h2zero]

Probably not, unless it's an empty service, which would be weird. Wouldn't be the worst idea to add it anyway though.

[thekurtovic]

I introduced some minor changes since the previous successful build. Let me know if I should revert. Since characteristicDiscCB doesn't mention the function name in the log messages, I shortened the name so that the multi-line function calls could be avoided in retrieveCharacteristics. Also tried thinking of a more compact way to write connHandle, endHandle, etc. Settled on hdleConn, hdlEnd, but considered using h as a prefix to signify handles.

Btw when a PR gets merged, does it use the message from the OP or the message from the commit?

[h2zero]

I'm not concerned with the naming, as long as it's somewhat consistent throughout the library and obvious/readable for casual observers as to what they represent, in this case the original naming suits but I'm happy to entertain changes.

Btw when a PR gets merged, does it use the message from the OP or the message from the commit?

The title/message on github for each commit in a PR is what will be seen in git log when pulled by anyone else.

[thekurtovic]

Some of the BLE jargon just adds so much bloat when written in full like characteristic, descriptor, etc.; which then causes the need to break function calls up with one parameter per line.

I definitely think some standardization should be established which will make things more concise while still having a clear meaning. Though that's probably a 3.0 discussion.

The title/message on github for each commit in a PR is what will be seen in git log when pulled by anyone else.

Good to know. Sometimes I mess up the message in the terminal and wasn't sure if I need to bother repushing.

[h2zero]

Some of the BLE jargon just adds so much bloat when written in full like characteristic, descriptor, etc.; which then causes the need to break function calls up with one parameter per line.

Yeah, drives me crazy sometimes.

I definitely think some standardization should be established which will make things more concise while still having a clear meaning. Though that's probably a 3.0 discussion.

Absolutely

Good to know. Sometimes I mess up the message in the terminal and wasn't sure if I need to bother repushing

No promises lol, it comes down to the person merging, it can all be edited.