Just return a pointer for getServices(false) and getCharacteristics(false)

[Jeroen88]

I was surprised that the services count of my device was not 'zero' directly after connecting to that device. I used this sketch:

#include <NimBLEDevice.h>

void setup() {
  // put your setup code here, to run once:
  Serial.begin(115200);
  Serial.println("\n\nStarted");

  BLEDevice::init("ESP32");

  Serial.println("Create client");
  NimBLEClient *pClient = BLEDevice::createClient();
  Serial.printf("Service count before connection is %u\n", pClient->getServices()->size());
  Serial.println("Connecting...");
  bool success = pClient->connect(BLEAddress("a4:c1:38:5d:ef:16"));
  if(!success) {
    BLEDevice::deleteClient(pClient);

    Serial.println("Failed to connect");
  } else {
    Serial.println("Connected!");
  }
  Serial.printf("Service count after connection is %u\n", pClient->getServices()->size());
  pClient->disconnect();
  Serial.printf("Service count after disconnection is %u\n", pClient->getServices()->size());
  pClient->deleteServices();
  Serial.printf("Service count after deletion is %u\n", pClient->getServices()->size());
}

void loop() {
  // put your main code here, to run repeatedly:

}

This resulted:

Started
Create client
Service count before connection is 0
Connecting...
Connected!
Service count after connection is 8
Service count after disconnection is 8
Service count after deletion is 0

It turned out that the inspection of the size of the vector of services pClient->getServices()->size() automatically retrieved all services, even though the default parameter to getServices() is false. IMHO that is semantically wrong.

If parameter 'refresh' equals 'false' services or characteristics should not be retrieved, just a pointer to the vector should be returned.

[h2zero]

I do not disagree with your observations here, however it is working as intended.
There are 2 reasons for this:

1. If the user is calling getServices() (plural) it shows they want to inspect either all the services of an unknown device or they want to inspect the vector of services individually discovered (the reason for the refresh parameter).
2. This functionality is backward compatible with the same function in the original library, but with the added bool parameter.

I do not see any reason to change this, why else would you call this function immediately after connecting if you didn't want to inspect all the services available?

[Jeroen88]

If refresh is true, then all services are deleted. Next, because the vector is empty all services are retrieved. This is not intuitive, anyway not for me. A better semantics for refresh would be to retrieve only the current set of services again from the peripheral. But that is not what this PR is about...

If refresh is false, and no services are yet present, they are all retrieved. IMHO refresh == false means: do not refresh!

or they want to inspect the vector of services individually discovered (the reason for the refresh parameter)

I agree with you, an this behavior should be consistent also if no service has been individually discovered yet (which is not the case now)

why else would you call this function

My use case is the following: I read a characteristic and I get notifications for another characteristic on the same service. When I read a characteristic, afterwards I delete it, but for notifications I have to keep the service alive. So when I do the read, I check the size of the service vector before and after registration. If it is the same, the service was already there for notifications and can't be deleted.

I am still convinced that the interpretation in this PR is correct. If someone needs all services, just pass true as a parameter. But if you do not want to change it, I would suggest a new function, e.g. inspectServices() that returns just the pointer.

[h2zero]

I've been thinking about this one, for it's limited use in most applications I agree this should be changed to accommodate different use cases. I need a better way to document all the api changes 😃

[Jeroen88]

I agree this should be changed to accommodate different use cases.

Thnx!