From 0db487f7708f71f6e4cdc8e36c24280c8e78c82f Mon Sep 17 00:00:00 2001 From: William Emfinger Date: Fri, 6 Sep 2024 09:01:35 -0500 Subject: [PATCH 1/5] fix(ble_gatt_server): Deinit services before device deinit --- components/ble_gatt_server/include/ble_gatt_server.hpp | 6 +++--- components/esp-nimble-cpp | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/components/ble_gatt_server/include/ble_gatt_server.hpp b/components/ble_gatt_server/include/ble_gatt_server.hpp index 0cb5c351b..4d965833d 100644 --- a/components/ble_gatt_server/include/ble_gatt_server.hpp +++ b/components/ble_gatt_server/include/ble_gatt_server.hpp @@ -281,13 +281,13 @@ class BleGattServer : public BaseComponent { /// @note This method will also deinitialize the device info and battery /// services. void deinit() { + // deinitialize the services + device_info_service_.deinit(); + battery_service_.deinit(); // if true, deletes all server/advertising/scan/client objects which // invalidates any references/pointers to them bool clear_all = true; NimBLEDevice::deinit(clear_all); - // now deinitialize the services - device_info_service_.deinit(); - battery_service_.deinit(); // clear the server and client server_ = nullptr; client_ = nullptr; diff --git a/components/esp-nimble-cpp b/components/esp-nimble-cpp index cd115f173..de428fad3 160000 --- a/components/esp-nimble-cpp +++ b/components/esp-nimble-cpp @@ -1 +1 @@ -Subproject commit cd115f1738fbf9134a4b00ed41d36fb20cafe22c +Subproject commit de428fad3d01b3ead45fbb6de1ebc2ee37ddcdb5 From e111d3625e170017dc2bf5c1bc29717f952436f4 Mon Sep 17 00:00:00 2001 From: William Emfinger Date: Fri, 6 Sep 2024 09:27:22 -0500 Subject: [PATCH 2/5] update example to exit loop after all devices have disconnected (but at least one device has connected). added additional logging --- .../example/main/ble_gatt_server_example.cpp | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/components/ble_gatt_server/example/main/ble_gatt_server_example.cpp b/components/ble_gatt_server/example/main/ble_gatt_server_example.cpp index 69d2c530e..102d24581 100644 --- a/components/ble_gatt_server/example/main/ble_gatt_server_example.cpp +++ b/components/ble_gatt_server/example/main/ble_gatt_server_example.cpp @@ -118,19 +118,23 @@ extern "C" void app_main(void) { input.Start(); // this will not return until the user enters the `exit` command. #endif + logger.info("Menu has finished, starting advertising"); // The menu has finished, so let's go into a loop to keep the device running // now lets start advertising ble_gatt_server.start_advertising(); + logger.info("Waiting for connection..."); // now lets update the battery level every second for a little while uint8_t battery_level = 99; bool was_connected = false; + bool can_exit = false; while (true) { auto start = std::chrono::steady_clock::now(); // if we are now connected, but were not, then get the services if (ble_gatt_server.is_connected() && !was_connected) { was_connected = true; + can_exit = true; auto connected_device_infos = ble_gatt_server.get_connected_device_infos(); logger.info("Connected devices: {}", connected_device_infos.size()); std::vector connected_device_names; @@ -140,9 +144,16 @@ extern "C" void app_main(void) { logger.info(" Names: {}", connected_device_names); } else if (!ble_gatt_server.is_connected()) { was_connected = false; + if (can_exit) { + logger.info("No longer connected, exiting"); + break; + } } if (!ble_gatt_server.is_connected()) { + logger.move_up(); + logger.clear_line(); + logger.info("Waiting for connection..."); // sleep std::this_thread::sleep_until(start + 1s); continue; @@ -155,5 +166,14 @@ extern "C" void app_main(void) { // sleep std::this_thread::sleep_until(start + 1s); } + + // we are done, so stop the server and deinit the BLE stack + ble_gatt_server.deinit(); //! [ble gatt server example] + + logger.info("Done"); + // now just sleep forever + while (true) { + std::this_thread::sleep_for(1s); + } } From dabe226e06224f9713f4cd4c5b68db4d11262c43 Mon Sep 17 00:00:00 2001 From: William Emfinger Date: Fri, 6 Sep 2024 09:27:56 -0500 Subject: [PATCH 3/5] ensure client connection is unset after we are done with it so that later cleanup doesnt think its still connected when its not --- components/ble_gatt_server/include/ble_gatt_server.hpp | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/components/ble_gatt_server/include/ble_gatt_server.hpp b/components/ble_gatt_server/include/ble_gatt_server.hpp index 4d965833d..98875bd7b 100644 --- a/components/ble_gatt_server/include/ble_gatt_server.hpp +++ b/components/ble_gatt_server/include/ble_gatt_server.hpp @@ -551,7 +551,10 @@ class BleGattServer : public BaseComponent { return {}; } // and read it - return name_char->readValue(); + auto name = name_char->readValue(); + // unset the client connection + client_->clearConnection(); + return name; } /// Get the connected device names @@ -604,6 +607,8 @@ class BleGattServer : public BaseComponent { client_->setConnection(conn_info); // and read the RSSI from the client auto rssi = client_->getRssi(); + // unset the client connection + client_->clearConnection(); logger_.info("RSSI for connected device {}: {}", peer_address.toString(), rssi); return rssi; } From fe5adef0883ae9ddcfb64b7eedb20c746d0a5501 Mon Sep 17 00:00:00 2001 From: William Emfinger Date: Fri, 6 Sep 2024 09:33:24 -0500 Subject: [PATCH 4/5] update connection to timeout for easier test --- .../example/main/ble_gatt_server_example.cpp | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/components/ble_gatt_server/example/main/ble_gatt_server_example.cpp b/components/ble_gatt_server/example/main/ble_gatt_server_example.cpp index 102d24581..e0354257d 100644 --- a/components/ble_gatt_server/example/main/ble_gatt_server_example.cpp +++ b/components/ble_gatt_server/example/main/ble_gatt_server_example.cpp @@ -128,6 +128,7 @@ extern "C" void app_main(void) { uint8_t battery_level = 99; bool was_connected = false; bool can_exit = false; + int num_seconds_to_wait = 30; while (true) { auto start = std::chrono::steady_clock::now(); @@ -153,7 +154,11 @@ extern "C" void app_main(void) { if (!ble_gatt_server.is_connected()) { logger.move_up(); logger.clear_line(); - logger.info("Waiting for connection..."); + logger.info("Waiting for connection... {}s", --num_seconds_to_wait); + if (num_seconds_to_wait == 0) { + logger.info("No connection, exiting"); + break; + } // sleep std::this_thread::sleep_until(start + 1s); continue; From 996fb12dae7d2c97fcf561f963d438ee978e7106 Mon Sep 17 00:00:00 2001 From: William Emfinger Date: Fri, 6 Sep 2024 09:43:28 -0500 Subject: [PATCH 5/5] update to ensure init/deinit are protected from being called multiple times in a row; update example to test calling deinit() multiple times in a row via manual call and destructor --- .../example/main/ble_gatt_server_example.cpp | 301 +++++++++--------- .../include/ble_gatt_server.hpp | 10 + 2 files changed, 163 insertions(+), 148 deletions(-) diff --git a/components/ble_gatt_server/example/main/ble_gatt_server_example.cpp b/components/ble_gatt_server/example/main/ble_gatt_server_example.cpp index e0354257d..e575efe45 100644 --- a/components/ble_gatt_server/example/main/ble_gatt_server_example.cpp +++ b/components/ble_gatt_server/example/main/ble_gatt_server_example.cpp @@ -12,170 +12,175 @@ extern "C" void app_main(void) { espp::Logger logger({.tag = "BLE GATT Server Example", .level = espp::Logger::Verbosity::INFO}); logger.info("Starting"); - //! [ble gatt server example] - - // NOTE: esp-nimble-cpp already depends on nvs_flash and initializes - // nvs_flash in the NimBLEDevice::init(), so we don't have to do that - // to store bonding info - - // create the GATT server - espp::BleGattServer ble_gatt_server; - std::string device_name = "Espp BLE GATT Server"; - ble_gatt_server.set_log_level(espp::Logger::Verbosity::INFO); - ble_gatt_server.set_callbacks({ - .connect_callback = [&](NimBLEConnInfo &conn_info) { logger.info("Device connected"); }, - .disconnect_callback = [&](auto &conn_info, - auto reason) { logger.info("Device disconnected: {}", reason); }, - .authentication_complete_callback = - [&](const NimBLEConnInfo &conn_info) { logger.info("Device authenticated"); }, - // NOTE: this is optional, if you don't provide this callback, it will - // perform the exactly function as below: - .get_passkey_callback = - [&]() { - logger.info("Getting passkey"); - return NimBLEDevice::getSecurityPasskey(); - }, - // NOTE: this is optional, if you don't provide this callback, it will - // perform the exactly function as below: - .confirm_passkey_callback = - [&](const NimBLEConnInfo &conn_info, uint32_t passkey) { - logger.info("Confirming passkey: {}", passkey); - NimBLEDevice::injectConfirmPIN(conn_info, - passkey == NimBLEDevice::getSecurityPasskey()); - }, - }); - ble_gatt_server.init(device_name); + { + //! [ble gatt server example] + + // NOTE: esp-nimble-cpp already depends on nvs_flash and initializes + // nvs_flash in the NimBLEDevice::init(), so we don't have to do that + // to store bonding info + + // create the GATT server + espp::BleGattServer ble_gatt_server; + std::string device_name = "Espp BLE GATT Server"; + ble_gatt_server.set_log_level(espp::Logger::Verbosity::INFO); + ble_gatt_server.set_callbacks({ + .connect_callback = [&](NimBLEConnInfo &conn_info) { logger.info("Device connected"); }, + .disconnect_callback = [&](auto &conn_info, + auto reason) { logger.info("Device disconnected: {}", reason); }, + .authentication_complete_callback = + [&](const NimBLEConnInfo &conn_info) { logger.info("Device authenticated"); }, + // NOTE: this is optional, if you don't provide this callback, it will + // perform the exactly function as below: + .get_passkey_callback = + [&]() { + logger.info("Getting passkey"); + return NimBLEDevice::getSecurityPasskey(); + }, + // NOTE: this is optional, if you don't provide this callback, it will + // perform the exactly function as below: + .confirm_passkey_callback = + [&](const NimBLEConnInfo &conn_info, uint32_t passkey) { + logger.info("Confirming passkey: {}", passkey); + NimBLEDevice::injectConfirmPIN(conn_info, + passkey == NimBLEDevice::getSecurityPasskey()); + }, + }); + ble_gatt_server.init(device_name); #if !CONFIG_BT_NIMBLE_EXT_ADV - // extended advertisement does not support automatically advertising on - // disconnect - ble_gatt_server.set_advertise_on_disconnect(true); + // extended advertisement does not support automatically advertising on + // disconnect + ble_gatt_server.set_advertise_on_disconnect(true); #endif - // let's configure the security - bool bonding = true; - bool mitm = false; - bool secure_connections = true; - ble_gatt_server.set_security(bonding, mitm, secure_connections); - // and some i/o and key config - NimBLEDevice::setSecurityPasskey(123456); - ble_gatt_server.set_io_capabilities(BLE_HS_IO_NO_INPUT_OUTPUT); - ble_gatt_server.set_init_key_distribution(BLE_SM_PAIR_KEY_DIST_ENC | BLE_SM_PAIR_KEY_DIST_ID); - ble_gatt_server.set_resp_key_distribution(BLE_SM_PAIR_KEY_DIST_ENC | BLE_SM_PAIR_KEY_DIST_ID); - - // you can create a service and add it to the server using - // ble_gatt_server.server().addService() - - // now start the services - ble_gatt_server.start_services(); // starts the device info service and battery service - // NOTE: we could also directly start them ourselves if we wanted to - // control the order of starting the services - // e.g.: - // ble_gatt_server.battery_service().start(); - // ble_gatt_server.device_info_service().start(); - - // now start the gatt server - ble_gatt_server.start(); - - // let's set some of the service data - auto &battery_service = ble_gatt_server.battery_service(); - battery_service.set_battery_level(99); - - auto &device_info_service = ble_gatt_server.device_info_service(); - uint8_t vendor_source = 0x01; - uint16_t vid = 0xCafe; - uint16_t pid = 0xFace; - uint16_t product_version = 0x0100; - device_info_service.set_pnp_id(vendor_source, vid, pid, product_version); - device_info_service.set_manufacturer_name("ESP-CPP"); - device_info_service.set_model_number("esp-ble-01"); - device_info_service.set_serial_number("1234567890"); - device_info_service.set_software_version("1.0.0"); - device_info_service.set_firmware_version("1.0.0"); - device_info_service.set_hardware_version("1.0.0"); - - // set the advertising data - espp::BleGattServer::AdvertisedData adv_data; - // uint8_t flags = BLE_HS_ADV_F_DISC_LTD; - uint8_t flags = BLE_HS_ADV_F_DISC_GEN; - adv_data.setFlags(flags); - adv_data.setName(device_name); - adv_data.setAppearance((uint16_t)espp::BleAppearance::GENERIC_COMPUTER); - adv_data.addTxPower(); - ble_gatt_server.set_advertisement_data(adv_data); + // let's configure the security + bool bonding = true; + bool mitm = false; + bool secure_connections = true; + ble_gatt_server.set_security(bonding, mitm, secure_connections); + // and some i/o and key config + NimBLEDevice::setSecurityPasskey(123456); + ble_gatt_server.set_io_capabilities(BLE_HS_IO_NO_INPUT_OUTPUT); + ble_gatt_server.set_init_key_distribution(BLE_SM_PAIR_KEY_DIST_ENC | BLE_SM_PAIR_KEY_DIST_ID); + ble_gatt_server.set_resp_key_distribution(BLE_SM_PAIR_KEY_DIST_ENC | BLE_SM_PAIR_KEY_DIST_ID); + + // you can create a service and add it to the server using + // ble_gatt_server.server().addService() + + // now start the services + ble_gatt_server.start_services(); // starts the device info service and battery service + // NOTE: we could also directly start them ourselves if we wanted to + // control the order of starting the services + // e.g.: + // ble_gatt_server.battery_service().start(); + // ble_gatt_server.device_info_service().start(); + + // now start the gatt server + ble_gatt_server.start(); + + // let's set some of the service data + auto &battery_service = ble_gatt_server.battery_service(); + battery_service.set_battery_level(99); + + auto &device_info_service = ble_gatt_server.device_info_service(); + uint8_t vendor_source = 0x01; + uint16_t vid = 0xCafe; + uint16_t pid = 0xFace; + uint16_t product_version = 0x0100; + device_info_service.set_pnp_id(vendor_source, vid, pid, product_version); + device_info_service.set_manufacturer_name("ESP-CPP"); + device_info_service.set_model_number("esp-ble-01"); + device_info_service.set_serial_number("1234567890"); + device_info_service.set_software_version("1.0.0"); + device_info_service.set_firmware_version("1.0.0"); + device_info_service.set_hardware_version("1.0.0"); + + // set the advertising data + espp::BleGattServer::AdvertisedData adv_data; + // uint8_t flags = BLE_HS_ADV_F_DISC_LTD; + uint8_t flags = BLE_HS_ADV_F_DISC_GEN; + adv_data.setFlags(flags); + adv_data.setName(device_name); + adv_data.setAppearance((uint16_t)espp::BleAppearance::GENERIC_COMPUTER); + adv_data.addTxPower(); + ble_gatt_server.set_advertisement_data(adv_data); #if CONFIG_COMPILER_CXX_EXCEPTIONS - // let's test and use the BLE menu (CLI) - // turn off some of the logs so that it doesn't clutter up the CLI - ble_gatt_server.set_log_level(espp::Logger::Verbosity::WARN); - // and make the CLI - auto ble_menu = espp::BleGattServerMenu(ble_gatt_server); - cli::SetColor(); - cli::Cli cli(ble_menu.get()); - cli.ExitAction([](auto &out) { out << "Goodbye and thanks for all the fish.\n"; }); - - espp::Cli input(cli); - input.SetInputHistorySize(10); - input.Start(); // this will not return until the user enters the `exit` command. + // let's test and use the BLE menu (CLI) + // turn off some of the logs so that it doesn't clutter up the CLI + ble_gatt_server.set_log_level(espp::Logger::Verbosity::WARN); + // and make the CLI + auto ble_menu = espp::BleGattServerMenu(ble_gatt_server); + cli::SetColor(); + cli::Cli cli(ble_menu.get()); + cli.ExitAction([](auto &out) { out << "Goodbye and thanks for all the fish.\n"; }); + + espp::Cli input(cli); + input.SetInputHistorySize(10); + input.Start(); // this will not return until the user enters the `exit` command. #endif - logger.info("Menu has finished, starting advertising"); - // The menu has finished, so let's go into a loop to keep the device running - // now lets start advertising - ble_gatt_server.start_advertising(); - - logger.info("Waiting for connection..."); - // now lets update the battery level every second for a little while - uint8_t battery_level = 99; - bool was_connected = false; - bool can_exit = false; - int num_seconds_to_wait = 30; - while (true) { - auto start = std::chrono::steady_clock::now(); - - // if we are now connected, but were not, then get the services - if (ble_gatt_server.is_connected() && !was_connected) { - was_connected = true; - can_exit = true; - auto connected_device_infos = ble_gatt_server.get_connected_device_infos(); - logger.info("Connected devices: {}", connected_device_infos.size()); - std::vector connected_device_names; - std::transform(connected_device_infos.begin(), connected_device_infos.end(), - std::back_inserter(connected_device_names), - [&](auto &info) { return ble_gatt_server.get_connected_device_name(info); }); - logger.info(" Names: {}", connected_device_names); - } else if (!ble_gatt_server.is_connected()) { - was_connected = false; - if (can_exit) { - logger.info("No longer connected, exiting"); - break; + logger.info("Menu has finished, starting advertising"); + // The menu has finished, so let's go into a loop to keep the device running + // now lets start advertising + ble_gatt_server.start_advertising(); + + logger.info("Waiting for connection..."); + // now lets update the battery level every second for a little while + uint8_t battery_level = 99; + bool was_connected = false; + bool can_exit = false; + int num_seconds_to_wait = 30; + while (true) { + auto start = std::chrono::steady_clock::now(); + + // if we are now connected, but were not, then get the services + if (ble_gatt_server.is_connected() && !was_connected) { + was_connected = true; + can_exit = true; + auto connected_device_infos = ble_gatt_server.get_connected_device_infos(); + logger.info("Connected devices: {}", connected_device_infos.size()); + std::vector connected_device_names; + std::transform(connected_device_infos.begin(), connected_device_infos.end(), + std::back_inserter(connected_device_names), + [&](auto &info) { return ble_gatt_server.get_connected_device_name(info); }); + logger.info(" Names: {}", connected_device_names); + } else if (!ble_gatt_server.is_connected()) { + was_connected = false; + if (can_exit) { + logger.info("No longer connected, exiting"); + break; + } } - } - if (!ble_gatt_server.is_connected()) { - logger.move_up(); - logger.clear_line(); - logger.info("Waiting for connection... {}s", --num_seconds_to_wait); - if (num_seconds_to_wait == 0) { - logger.info("No connection, exiting"); - break; + if (!ble_gatt_server.is_connected()) { + logger.move_up(); + logger.clear_line(); + logger.info("Waiting for connection... {}s", --num_seconds_to_wait); + if (num_seconds_to_wait == 0) { + logger.info("No connection, exiting"); + break; + } + // sleep + std::this_thread::sleep_until(start + 1s); + continue; } + + // update the battery level + battery_service.set_battery_level(battery_level); + battery_level = (battery_level % 100) + 1; + // sleep std::this_thread::sleep_until(start + 1s); - continue; } - // update the battery level - battery_service.set_battery_level(battery_level); - battery_level = (battery_level % 100) + 1; - - // sleep - std::this_thread::sleep_until(start + 1s); + // we are done, so stop the server and deinit the BLE stack. NOTE: this will + // automatically be called by ~BleGattServer(), but we call it here to show + // manual control and to test calling it multiple times since the destructor + // will be called immediately after this block ends + ble_gatt_server.deinit(); + //! [ble gatt server example] } - // we are done, so stop the server and deinit the BLE stack - ble_gatt_server.deinit(); - //! [ble gatt server example] - logger.info("Done"); // now just sleep forever while (true) { diff --git a/components/ble_gatt_server/include/ble_gatt_server.hpp b/components/ble_gatt_server/include/ble_gatt_server.hpp index 98875bd7b..059b6db0c 100644 --- a/components/ble_gatt_server/include/ble_gatt_server.hpp +++ b/components/ble_gatt_server/include/ble_gatt_server.hpp @@ -239,6 +239,11 @@ class BleGattServer : public BaseComponent { /// @note This method must be called before starting the server. /// @return Whether the GATT server was initialized successfully. bool init(const std::string &device_name) { + if (server_) { + logger_.info("Server already created, not initializing again"); + return true; + } + logger_.info("Initializing GATT server with device name: '{}'", device_name); // create the device NimBLEDevice::init(device_name); @@ -281,6 +286,11 @@ class BleGattServer : public BaseComponent { /// @note This method will also deinitialize the device info and battery /// services. void deinit() { + if (!server_ || !client_) { + logger_.info("Server / Client are nullptr; already deinitialized, not deinitializing again"); + return; + } + // deinitialize the services device_info_service_.deinit(); battery_service_.deinit();