Repository navigation
Conversation
This will prevent a case where applications may notify a task that is blocking on a BLE operation before it completes. This can occur when writing to a characteristic in the application and blocking with task notifications until a notification occurs that releases it and the notification arrives before the write response.
ESP-IDF NimBLE can reset the GAP Device Name when the standard GAP service is reinitialized during GATT server startup. NimBLEDevice::init(deviceName) calls NimBLEDevice::setDeviceName(deviceName), but that happens before the server is started. Later, starting advertising starts the server and resets/re-registers the GAP service, causing the C GAP name backing storage to be rebuilt from CONFIG_BT_NIMBLE_SVC_GAP_DEVICE_NAME. In projects where that Kconfig default remains "nimble", the advertised Local Name can be correct while the Generic Access Device Name characteristic reports "nimble".
The failing sequence is:
-> pAdvertising->start()
-> NimBLEAdvertising::start()
-> NimBLEDevice::getServer()->start()
-> NimBLEServer::resetGATT()
-> ble_gatts_reset()
-> ble_svc_gap_init()
-> ESP-IDF NimBLE resets GAP name to CONFIG_BT_NIMBLE_SVC_GAP_DEVICE_NAME
-> "nimble"
-> ble_svc_gatt_init()
Fix this by saving the device name supplied through NimBLEDevice::setDeviceName() and reapplying it immediately after ble_svc_gap_init() in NimBLEServer::resetGATT(). This preserves the C++ wrapper contract that the configured device name remains effective even when the wrapper internally resets the GATT/GAP service tables.
NimBLEAttValue::append advanced m_capacity immediately after calling realloc. If realloc failed, the original buffer remained allocated but the object advertised a larger capacity than it actually owned. A later append could then skip reallocation and copy past the end of the allocation. Track whether the append actually needs growth and only update m_capacity after the realloc result passes the null check. Appends that fit the existing allocation leave capacity unchanged; failed growth preserves the old capacity invariant.
#431) * Fix NimBLEEddystoneTLM byte-order mismatch between getters and setters m_eddystoneData must always hold Eddystone-TLM wire-format (big-endian) bytes, since getData() returns it directly for the caller to memcpy straight into the BLE advertisement payload (see the BLE_EddystoneTLM_Beacon example), and setData()/getData() already treat it as raw wire bytes. The getters (getVolt/getTemp) correctly un-swap on read, but the setters (setVolt/setTemp/setCount/setTime) stored their host-order input directly with no swap, and the BeaconData default member initializers were host-order values instead of pre-swapped wire-format values. This breaks the setter/getter round trip for any value set locally (as opposed to arriving over the air via setData()), and means a freshly constructed object reports a garbage battery voltage. toString() also failed to un-swap the temperature field at all, and used unsigned division/modulo on a signed 8.8 fixed-point value, producing garbage output for negative temperatures. This mirrors the existing, correct pattern already used by the sibling NimBLEBeacon class, which swaps in its setters (setMajor/setMinor/ setManufacturerId). Fix: - setVolt/setTemp/setCount/setTime now apply ENDIAN_CHANGE_U16/U32 to their input before storing, matching what the getters already assume. - BeaconData's default volt/temp member initializers are now the wire-format (byte-swapped) encoding of the intended defaults (3300 mV, 23.0C), so a freshly constructed object reports correct values without calling a setter first. - toString() now swaps the temperature field the same way it already swaps voltage, and computes the displayed magnitude from a signed value with explicit sign handling instead of unsigned arithmetic on the raw field.
The constructor calls std::reverse_copy on its input, expecting bytes in MSB-first order (Bluedroid's esp_bd_addr_t layout). The previous docstring said "compatibility with bluedroid esp library using native ESP representation," which is correct but ambiguous if the reader doesn't already know the conventions of both stacks. Callers reaching for this constructor with LSB-first bytes (e.g. as they'd come out of NimBLE's own ble_addr_t.val) silently end up with a reversed address — the radio uses val[] directly for HCI commands, so GAP connects target a fictional peer and time out as BLE_HS_ETIMEOUT with no diagnostic. Documentation-only change; behaviour is unchanged. Points readers to NimBLEAddress(ble_addr_t) for LSB-first wire bytes and to NimBLEAddress(const uint64_t&, uint8_t) for hex literals. Refs #423
This makes the availableForWrite signature override compatible with Arduino Print.
…terator * fix: whiteListRemove use-after-free - stop iterating after erase + shrink whiteListRemove erases the matched element (invalidating the iterator) and shrink-to-fits m_whiteList (freeing the buffer the iterator points into), then continues the loop on the dangling iterator. A layout-dependent spurious re-match then calls erase() with a foreign iterator, and the vector's internal element move writes across unrelated heap memory. Observed on ESP32-C3 as intermittent heap corruption under whitelist churn (multi_heap_free bad-head asserts with address bytes in the corrupted header; a clobbered NimBLE host mutex tripping xQueueSemaphoreTake). Verified fixed under CONFIG_HEAP_POISONING_COMPREHENSIVE plus periodic heap_caps_check_integrity_all() sweeps. whiteListAdd guards duplicates via onWhiteList(), so at most one element can match - breaking out after the removal preserves semantics. * fix: use m_whiteList.data() - operator[] on an empty vector is UB After erasing the last whitelist entry, &m_whiteList[0] indexes an empty vector before ble_gap_wl_set(..., 0). data() is well-defined for empty vectors; same change applied to the whiteListAdd call site for consistency.
Fixes a crash when the stack is reinitialized without clearing all data on deinit then starting scanning.
Fixes a connect crash in the client
… at low handles (#443) * feat(NimBLEServer): add registerServicesFirst() to place app services at low handles By default the standard GAP (0x1800) and GATT (0x1801) services register first and take the low attribute handles (0x0001+), with the application's services placed after them. Some peripherals must instead expose their own services at the low handles — e.g. when emulating a device whose central addresses attributes by hardcoded handle rather than by discovery. Add an opt-in NimBLEServer::registerServicesFirst(bool) setter (default false, so existing behavior is unchanged) that defers GAP/GATT registration until after the application services in resetGATT(), giving the application services the low handles. The device-name/appearance restore that follows ble_gatts_reset() is factored into a small lambda so it stays correct in either order.
* Make deepCopy return whether memory allocation and copying succeeded. * Add copyFrom to expose that result to callers. * Allocate notification values using the full mbuf packet length, with BLE_ATT_ATTR_MAX_LEN as the maximum. * Reject oversized notifications and report temporary-buffer allocation or characteristic-value copy failures as insufficient resources. * Reassemble notification data before updating the characteristic value or invoking its callback.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configuration
You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.