From da9152761f5e1cf51c7421614ce8222149c9dc5d Mon Sep 17 00:00:00 2001 From: William Emfinger Date: Fri, 6 Sep 2024 11:10:11 -0500 Subject: [PATCH] fix(ble_gatt_server): Deinit services before device deinit (#325) * fix(ble_gatt_server): Deinit services before device deinit * update example to exit loop after all devices have disconnected (but at least one device has connected). added additional logging * ensure client connection is unset after we are done with it so that later cleanup doesnt think its still connected when its not * update connection to timeout for easier test * 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 | 290 ++++++++++-------- .../include/ble_gatt_server.hpp | 23 +- components/esp-nimble-cpp | 2 +- 3 files changed, 180 insertions(+), 135 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 69d2c530e..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,148 +12,178 @@ 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 - // 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("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; + } + // 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; - // now lets update the battery level every second for a little while - uint8_t battery_level = 99; - bool was_connected = 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; - 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 (!ble_gatt_server.is_connected()) { // 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; + // 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] + } - // sleep - std::this_thread::sleep_until(start + 1s); + logger.info("Done"); + // now just sleep forever + while (true) { + std::this_thread::sleep_for(1s); } - //! [ble gatt server example] } diff --git a/components/ble_gatt_server/include/ble_gatt_server.hpp b/components/ble_gatt_server/include/ble_gatt_server.hpp index 0cb5c351b..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,13 +286,18 @@ 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(); // 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; @@ -551,7 +561,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 +617,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; } 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