[Bug] Peripheral fed into discover callback losses underlying handles

[Jah-On]

Calling any member function on the Peripheral passed into the callback does not do anything or behaves incorrectly. Here is a minimal example of it not working:

#include "simpleble/SimpleBLE.h"
#include <chrono>
#include <thread>
#include <iostream>

#define NUS_SERVICE_UUID "6e400001-b5a3-f393-e0a9-e50e24dcca9e"
#define NUS_RX_UUID      "6e400002-b5a3-f393-e0a9-e50e24dcca9e"
#define NUS_TX_UUID      "6e400003-b5a3-f393-e0a9-e50e24dcca9e"

void notifyCallback(SimpleBLE::ByteArray data){
  std::cout << "Received: " << data.data() << std::endl;
}

void discoverCb(SimpleBLE::Peripheral peripheral){
  if (!peripheral.initialized()) return;
  if (peripheral.identifier() != "test_device") return;

  std::cout << "Found device" << std::endl;
  
  peripheral.connect();
  peripheral.notify(NUS_SERVICE_UUID, NUS_TX_UUID, notifyCallback);
  peripheral.write_command(NUS_SERVICE_UUID, NUS_RX_UUID, "Hello world!");
  std::this_thread::sleep_for(std::chrono::milliseconds(1000));
  peripheral.disconnect();
}

int main(){
  SimpleBLE::Adapter                   adapter    = SimpleBLE::Adapter::get_adapters().at(0);
  adapter.set_callback_on_scan_found(discoverCb);
  adapter.scan_start();

  std::this_thread::sleep_for(std::chrono::milliseconds(10000));
}

With a valid Peripheral instance, this code should send "Hello world!" to the NUS and print out the response from the device. However, when using the callback instance, it does not do anything.

The reason why this does not work is because SimpleBLE creates a clone of the Peripheral instance using the default constructor. Thus, the underlying back-end handles are no longer valid and the functions don't do anything.

Possible solutions

Personally, I think the callbacks should use pass by reference instead of a cloned constructor. If this can't be done, then a proper cloning constructor should be implemented to ensure the back-end handles are swapped or re-validated.

I can work on this myself unless a different maintainer wants to do it.

[Jah-On]

Source of issue #311.

[Jah-On]

I tested switching to a pass by reference for the callbacks and it had no impact on the results which makes sense since the internal objects is a shared_ptr. I'm not sure if I'll be able to contribute a fix.

[Jah-On]

It seems like using Safe::Adapter and Safe::Peripheral for the callbacks and variables remedies the issue. I guess the issue can be closed?

I'm curious why both the Safe and normal APIs exist for C++. I think it would make a lot more sense to have the Safe API be the normal one which would minimize these sorts of issues.

[kdewald]

Hey @Jah-On, thanks for the report!

I'll reopen the issue to make sure I do a once-over to see what can be improved.