NimBLE-Arduino icon indicating copy to clipboard operation
NimBLE-Arduino copied to clipboard

updating manufacturer data in BLEAdvertisementData instance - impossible?

Open mhaberler opened this issue 2 years ago • 9 comments

I'm building a sensor which reports in the manufacturer data hence the manufacturer data changes frequently

this does work, however only on a fresh instance of BLEAdvertisementData():

  advData = new BLEAdvertisementData();
  advData->setManufacturerData(manufacturerData);

  pAdvertising->setAdvertisementData(*advData);
  pAdvertising->setAdvertisementType(BLE_GAP_CONN_MODE_NON);

  pAdvertising->start();
  delete advData;

reusing the same BLEAdvertisementData() fails - I think the payload is only appended to and there is no way to reset the payload my solution deletes and re-creates objects needlessly

Am I overlooking something?

if not: can I suggest a BLEAdvertisementData.reset() method which resets the payload?

thanks for a great and well supported library!

Michael

repo: https://github.com/mhaberler/flowsensor/blob/main/src/beacon.cpp#L45-L55

mhaberler avatar Jul 02 '23 17:07 mhaberler

Hi @mhaberler, yes that is a deficiency in the class and a reset method should definitely be implemented. Thanks for creating this issue.

h2zero avatar Jul 04 '23 00:07 h2zero

would you accept a PR? if so, on which branch - master?

mhaberler avatar Jul 04 '23 05:07 mhaberler

Yes, feel free to submit a PR to the release/1.4 branch. I will add it to the master branch after.

h2zero avatar Jul 04 '23 14:07 h2zero

there is something else fishy in the code I quoted:

if I call this code too frequently (for instance from a UI button pressed in rapid succession), I get a crash

it seems I try to fiddle the advertising before the previous fiddle has finished/synced to the host

is there any predicate I can test to prevent this?

  #0  0x403769ed:0x3fcf27c0 in panic_abort at /Users/mah/.platformio/packages/framework-espidf/components/esp_system/panic.c:408
  #1  0x4037fa6d:0x3fcf27e0 in esp_system_abort at /Users/mah/.platformio/packages/framework-espidf/components/esp_system/esp_system.c:137
  #2  0x403878d1:0x3fcf2800 in __assert_func at /Users/mah/.platformio/packages/framework-espidf/components/newlib/assert.c:85
  #3  0x40386a5f:0x3fcf2920 in tlsf_free at /Users/mah/.platformio/packages/framework-espidf/components/heap/heap_tlsf.c:964 (discriminator 1)
  #4  0x40387484:0x3fcf2940 in multi_heap_free_impl at /Users/mah/.platformio/packages/framework-espidf/components/heap/multi_heap.c:225
  #5  0x403778d2:0x3fcf2960 in heap_caps_free at /Users/mah/.platformio/packages/framework-espidf/components/heap/heap_caps.c:361
  #6  0x40387901:0x3fcf2980 in free at /Users/mah/.platformio/packages/framework-espidf/components/newlib/heap.c:39
  #7  0x4207afed:0x3fcf29a0 in operator delete(void*) at /builds/idf/crosstool-NG/.build/HOST-x86_64-apple-darwin12/xtensa-esp32s3-elf/src/gcc/libstdc++-v3/libsupc++/del_op.cc:49
  #8  0x42000189:0x3fcf29c0 in __gnu_cxx::new_allocator<char>::deallocate(char*, unsigned int) at /Users/mah/.platformio/packages/toolchain-xtensa-esp32s3/xtensa-esp32s3-elf/include/c++/8.4.0/ext/new_allocator.h:125
      (inlined by) std::allocator_traits<std::allocator<char> >::deallocate(std::allocator<char>&, char*, unsigned int) at /Users/mah/.platformio/packages/toolchain-xtensa-esp32s3/xtensa-esp32s3-elf/include/c++/8.4.0/bits/alloc_traits.h:462
      (inlined by) std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::_M_destroy(unsigned int) at /Users/mah/.platformio/packages/toolchain-xtensa-esp32s3/xtensa-esp32s3-elf/include/c++/8.4.0/bits/basic_string.h:226
      (inlined by) std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::_M_dispose() at /Users/mah/.platformio/packages/toolchain-xtensa-esp32s3/xtensa-esp32s3-elf/include/c++/8.4.0/bits/basic_string.h:221
      (inlined by) std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::~basic_string() at /Users/mah/.platformio/packages/toolchain-xtensa-esp32s3/xtensa-esp32s3-elf/include/c++/8.4.0/bits/basic_string.h:657
      (inlined by) NimBLEAdvertisementData::~NimBLEAdvertisementData() at lib/esp-nimble-cpp/src/NimBLEAdvertising.h:50
      (inlined by) beacon_update_manufacturer_data(unsigned char*, unsigned int) at src/beacon.cpp:49
  #9  0x420008ac:0x3fcf2a00 in sensor_update(bool) at src/main.cpp:147
  #10 0x420009a4:0x3fcf2a20 in setup()::{lambda()#2}::_FUN() at src/main.cpp:113
      (inlined by) _FUN at src/main.cpp:114
  #11 0x420582cf:0x3fcf2a40 in timer_process_alarm at /Users/mah/.platformio/packages/framework-espidf/components/esp_timer/src/esp_timer.c:360
  #12 0x42058325:0x3fcf2a70 in timer_task at /Users/mah/.platformio/packages/framework-espidf/components/esp_timer/src/esp_timer.c:386 (discriminator 1)
  #13 0x40382fd5:0x3fcf2a90 in vPortTaskWrapper at /Users/mah/.platformio/packages/framework-espidf/components/freertos/port/xtensa/port.c:142

mhaberler avatar Jul 05 '23 10:07 mhaberler

if I may tack on a question (is there a better venue like a forum?):

is the approach "stop advertising", "fiddle manufacturer data", "restart advertising" the right one, or is there some more elegant method I overlooked?

thanks in advance Michael

mhaberler avatar Jul 05 '23 10:07 mhaberler

The approach is correct in that the advertising should be stopped, data updated, then restarted. The issue here is that the stop process takes some time to complete and you need to wait for that before changing data.

h2zero avatar Jul 05 '23 20:07 h2zero

for my understanding - would this fix the race?

  • add a advCompleteCB callback to pAdvertising->start();

  • if there is new advertising data, stop the scan with pAdvertising->reset() and return

  • in the advCompleteCB callback, set advData->setManufacturerData(manufacturerData)

  • and pAdvertising->start(..., advCompleteCB);

edit: it seems not, because pAdvertising->reset() does not trigger the advCompleteCB callback

edit2: it seems this has done the trick: https://github.com/mhaberler/flowsensor/blob/main/src/beacon.cpp#L41-L52

mhaberler avatar Jul 06 '23 05:07 mhaberler

That should be the process, looks good to me.

h2zero avatar Jul 09 '23 18:07 h2zero

thanks! leaving this open until I have a reset() method PR and the eventual working code can be referred to

mhaberler avatar Jul 09 '23 19:07 mhaberler