NimBLE-Arduino icon indicating copy to clipboard operation
NimBLE-Arduino copied to clipboard

getting duplicate service

Open sdetweil opened this issue 2 years ago • 14 comments

followup on previous add/removeServiceUUID issue

service created only once, is the only advertised service in nrfConnect.

connect to see services available, but this recently created service is shown twice, both empty( no characteristics)

connecting app says characteristics missing

239750299-a6432a30-cba3-49a1-b0b3-2d2d8b32b8fe

fails on the test app sample I provided last week.

I had seen this when I did addServiceUUID() multiple times, but I have verified there is only one add call done now

while in loop()

stop advertising (prior service UUID) remove prior advertising UUID ( now none) call library which creates service on server lib creates characteristics (6) lib starts service lib returns service address add serviceuuid (added debug in nimble method to report count of UUIDs, after add call, only 1) start advertising

mobile scan(nrfConnect) correct UUID advertised connect services as shown in screen shot

if I do NOT addServiceUUID after lib return, no service is advertised, but on connect the service is still shown twice, as above

sdetweil avatar May 22 '23 12:05 sdetweil

To resolve this I had to move this:

  // disconnect the(any) connected clients
  for (auto it: pServer->getPeerDevices()) {
    pServer->disconnect(it);
  }
  while (pServer->getConnectedCount() > 0) {
    yield();
  }

to line 466 and add a delay(100); after this.

Also removed this line in the OTA lib https://github.com/sdetweil/ArduinoBleOTA/blob/6cf3edbd73d72b94848ef9c62ae5592160ee2400/src/ArduinoBleOtaClassNimBle.cpp#L74

And finally added:

pServer->advertiseOnDisconnect(false);

here https://github.com/sdetweil/testapp/blob/ce7302b5a1942b65d1dd7824e1cf8e6e66e8b607/src/testapp.cpp#L521

There were simply too many things triggering service start at once.

h2zero avatar May 22 '23 16:05 h2zero

thanks

  1. i had moved up the client disconnect, as there is a disconnect handler that wants to turn advertising back on. 1.5 will add the delay
  2. I had removed the service->start(), but it didn't affect the results.
  3. pServer->advertiseOnDisconnect(false); I had assumed that was the default.. cause I manually restart on disconnect (see 1!)..

  4. scanner... i have changed it to cycle less often, 2 seconds every 10. to save power... we shall see

sdetweil avatar May 22 '23 16:05 sdetweil

ok, not there yet

now I have one service, but empty

also turned off the scanner when we are starting the OTA service onward.

in loop check for OTAService requested first, (start service and set Running then check if OTA Running (yes), skip scan

i don't restart advertising in onDisconnect handler if OTARunning

void startOTAService()
{
  OTARunning = 2;
#ifdef ESP32
  advertising = false;
  #ifdef USE_NIMBLE
    Sprintln("stop advertising");
    pAdvertising->stop();
    Sprintf("removing advertised UUID=");
    Sprintln(activeService[status]->getUUID().toString().c_str());
    pAdvertising->removeServiceUUID(activeService[status]->getUUID());
  #else
    pAdvertising->stop();
  #endif
  // disconnect the(any) connected clients
  disconnectClient();  // the code you shared + the delay(100)
  // start the Over The Air update service
  OTAService = ArduinoBleOTA.begin(pServer, InternalStorage, HW_NAME_INFO.c_str(), HW_VER, SW_NAME.c_str(), SW_VER);
  OTARunning = 1;
  ArduinoBleOTA.setUploadCallbacks(*(new myUploadCallbacks()));
  Sprintln("restart advertising after OTA Service setup");
  Sprintln("setting OTA service as only");
  #if USE_NIMBLE
     pAdvertising->addServiceUUID(OTAService->getUUID());
     Sprint("added UUID to advertising, should be only=");
     Sprintln(OTAService->getUUID().toString().c_str());
     pAdvertising->start();
     Sprintln("restart advertising");
  #else
    pAdvertising->setServiceUUID(OTAService->getUUID());
    pAdvertising->start();
  #endif
    advertising = true;
#else

#endif

}

//if characteristic write handler write characteristic=9A64 OTA Start requested

// later ...1 second called from loop

//start OtaService stop advertising removing advertised UUID=00000001-27b9-42f0-82aa-2e951747bbf9 disconnected (force disconnect) restart advertising after OTA Service setup setting OTA service as only added UUID to advertising, should be only=15c155ca-36c5-11ed-adc0-9741d6a72f04 restart advertising connected // nrfConnect on disconnect start advertising= 15c155ca-36c5-11ed-adc0-9741d6a72f04 //restart advet on ota service disconnected // nrfConnect end of onDisconnect() handler

sdetweil avatar May 22 '23 17:05 sdetweil

I updated the testapp sample, with all these changes, and included more to stop scanning while engaging in startup and running of the ota service.

https://github.com/sdetweil/testapp

still getting duplicate empty services

sdetweil avatar May 24 '23 15:05 sdetweil

I left comments on the latest commit

h2zero avatar May 24 '23 21:05 h2zero

thank you. left comments back, really on disconnect

sdetweil avatar May 24 '23 22:05 sdetweil

is there a usecase that service->start() is appropriate?

I don't own the ota service library

sdetweil avatar May 25 '23 01:05 sdetweil

service->start() is only necessary for the initial startup, if the services are changed while running the library will take care of starting them.

h2zero avatar May 25 '23 15:05 h2zero

service->start().. thanks.. is this a wasted call, if already running? or a damaging call?

again , I don't own the library, don't "want" to own my own fork...

sdetweil avatar May 25 '23 19:05 sdetweil

It's problematic to call it the way it's being done here. if you don't want to use a fork then I would suggest adding the service at the startup and just remove it until needed.

h2zero avatar May 25 '23 22:05 h2zero

create. remove it's started. but not accessible

when needed addService() then addServiceUUID

sdetweil avatar May 29 '23 11:05 sdetweil

so, I tried this, but see the OTA service when connected... altho removed

in setup

  // create the two default services
  for(int i=0; i<NUM_SERVICES; i++){
    activeService[i]=setupService(i);
  }
#ifdef ESP32
  Sprint("setting advertised service=");
  Sprintln(activeService[status]->getUUID().toString().c_str());
  // pAdvertising set in setupService()
  pAdvertising->setScanResponse(true);
  #ifdef USE_NIMBLE
    #ifdef USE_OTA
    // create the service
    OTAService = ArduinoBleOTA.begin(pServer, InternalStorage, HW_NAME_INFO.c_str(), HW_VER, SW_NAME.c_str(), SW_VER);
    // but remove it til needed... Nimble library requirement
    // can only do this for Nimble as ESPBLE has removeService(), but no addService()
    pServer->removeService(OTAService, false);
    // don't know if this is needed, doesn't make a difference
    pAdvertising->removeServiceUUID(OTAService->getUUID());
    #endif
  #endif 
#endif

in loop

void startOTAService()
{
  OTARunning = OTA_Pending;
#ifdef ESP32
  advertising = false;
  pAdvertising->stop();
  Sprintf("removing advertised UUID=");
  Sprintln(activeService[status]->getUUID().toString().c_str());
  pAdvertising->removeServiceUUID(activeService[status]->getUUID());
  delay(50);
#ifdef USE_NIMBLE
    Sprintln("nimble adding OTA service");
    pServer->addService(OTAService);
#else
    // start the Over The Air update service
    Sprintln("NOT nimble creating OTA service");
    OTAService = ArduinoBleOTA.begin(pServer, InternalStorage, HW_NAME_INFO.c_str(), HW_VER, SW_NAME.c_str(), SW_VER);
#endif
  // disconnect the(any) connected clients
  disconnectClient();
  OTARunning = OTA_Running;
  ArduinoBleOTA.setUploadCallbacks(*(new myUploadCallbacks()));
  Sprintln("restart advertising after OTA Service setup");
  Sprintln("setting OTA service as only");
  Sprint("added UUID to advertising, should be only=");
  Sprintln(OTAService->getUUID().toString().c_str());
#if USE_NIMBLE
    // there is none set
    pAdvertising->addServiceUUID(OTAService->getUUID());
  #else
    // clear al uuids and set one // local fix
    pAdvertising->setServiceUUID(OTAService->getUUID());
  #endif
    Sprintln("restart advertising");
    pAdvertising->start();
    advertising = true;
#else
   // non esp32 
#endif;
}

sdetweil avatar May 30 '23 16:05 sdetweil

I updated the test app and the library to remove the pService->start()

but see the service in nrfconnect after connecting to the device.. shouldn't be there if I removed it..

sdetweil avatar May 31 '23 21:05 sdetweil

This works as intended by adding pServer->start(); after ArduinoBleOTA.begin

h2zero avatar Aug 13 '23 18:08 h2zero