Skip to content

Restore GAP device name after GATT reset #426

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
chmorgan wants to merge 1 commit into
base: master
Choose a base branch
from
+10 −0
Open

Restore GAP device name after GATT reset #426

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions src/NimBLEDevice.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,8 @@ bool NimBLEDevice::m_initialized{false};
uint32_t NimBLEDevice::m_passkey{123456};
bool NimBLEDevice::m_synced{false};
ble_gap_event_listener NimBLEDevice::m_listener{};
std::string NimBLEDevice::m_deviceName{};
bool NimBLEDevice::m_deviceNameSet{false};
std::vector<NimBLEAddress> NimBLEDevice::m_whiteList{};
uint8_t NimBLEDevice::m_ownAddrType{BLE_OWN_ADDR_PUBLIC};

Expand Down Expand Up @@ -1326,6 +1328,8 @@ bool NimBLEDevice::setDeviceName(const std::string& deviceName) {
return false;
}
# endif
m_deviceName = deviceName;
m_deviceNameSet = true;
return true;
} // setDeviceName

Expand Down
2 changes: 2 additions & 0 deletions src/NimBLEDevice.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -219,6 +219,8 @@ class NimBLEDevice {
static uint32_t m_passkey;
static ble_gap_event_listener m_listener;
static uint8_t m_ownAddrType;
static std::string m_deviceName;
static bool m_deviceNameSet;
static std::vector<NimBLEAddress> m_whiteList;
static NimBLEDeviceCallbacks* m_pDeviceCallbacks;
static NimBLEDeviceCallbacks defaultDeviceCallbacks;
Expand Down
4 changes: 4 additions & 0 deletions src/NimBLEServer.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -891,6 +891,10 @@ bool NimBLEServer::resetGATT() {

@h2zero h2zero Jun 24, 2026

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be better if we just added these lines here:

std::string name(ble_svc_gap_device_name());
uint16_t appearance = ble_svc_gap_device_appearance();

ble_gatts_reset();
ble_svc_gap_init();

@h2zero h2zero Jun 24, 2026

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Follwed up with:

ble_svc_gap_device_name_set(name.c_str());
ble_svc_gap_device_appearance_set(appearance);

Then we don't need to add any static members.

if (NimBLEDevice::m_deviceNameSet && !NimBLEDevice::setDeviceName(NimBLEDevice::m_deviceName)) {
return false;
}
Comment on lines +894 to +896

@coderabbitai coderabbitai Bot Jun 19, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚖️ Poor tradeoff

Shiver me timbers! A potential race condition be lurkin' in these waters!

Arr matey, while yer logic fer reapplyin' the cached device name be sound as a Spanish doubloon, there be a concurrency concern that could make yer ship run aground:

The static members NimBLEDevice::m_deviceName and NimBLEDevice::m_deviceNameSet be accessed here without any synchronization. If some scallywag calls NimBLEDevice::setDeviceName() from another thread whilst this resetGATT() be executin', ye could have yerself a classic race condition! One thread be writin' to m_deviceName while another be readin' from it - that be a recipe fer trouble on the high seas.

Now, in most BLE applications, these operations be serialized by the nature o' the code flow. But from a library design standpoint, there be no enforcement o' this serialization, savvy?

🏴‍☠️ Possible solutions fer this concurrency concern

Option 1: Document that BLE operations must be serialized by the application (low effort, shifts responsibility to user)

Option 2: Add a mutex to protect access to the static members (higher effort, makes library thread-safe by design)

// In NimBLEDevice.h, add a static mutex
static std::mutex m_deviceNameMutex;

// In setDeviceName:
{
    std::lock_guard<std::mutex> lock(m_deviceNameMutex);
    m_deviceName = deviceName;
    m_deviceNameSet = true;
}

// In resetGATT:
std::string cachedName;
bool nameWasSet;
{
    std::lock_guard<std::mutex> lock(NimBLEDevice::m_deviceNameMutex);
    cachedName = NimBLEDevice::m_deviceName;
    nameWasSet = NimBLEDevice::m_deviceNameSet;
}
if (nameWasSet && !NimBLEDevice::setDeviceName(cachedName)) {
    return false;
}
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/NimBLEServer.cpp` around lines 894 - 896, The static members
NimBLEDevice::m_deviceName and NimBLEDevice::m_deviceNameSet are accessed
without synchronization in the resetGATT() method, creating a potential race
condition if another thread calls setDeviceName() concurrently. To fix this, add
a static mutex member (like m_deviceNameMutex) to the NimBLEDevice class, then
protect all accesses to m_deviceName and m_deviceNameSet with std::lock_guard in
both the setDeviceName() method and the resetGATT() method. In resetGATT(),
acquire the lock, cache the values of m_deviceNameSet and m_deviceName into
local variables, release the lock, then perform the conditional call to
setDeviceName() with the cached name outside the critical section.


ble_svc_gatt_init();

for (auto svcIt = m_svcVec.begin(); svcIt != m_svcVec.end();) {
Expand Down