Skip to content
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.

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

updating manufacturer data in BLEAdvertisementData instance - impossible? #564

Closed
mhaberler opened this issue Jul 2, 2023 · 9 comments · Fixed by #683
Closed

updating manufacturer data in BLEAdvertisementData instance - impossible? #564

mhaberler opened this issue Jul 2, 2023 · 9 comments · Fixed by #683
Labels
enhancement New feature or request

Comments

@mhaberler
Copy link

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

@h2zero h2zero added the enhancement New feature or request label Jul 4, 2023
@h2zero
Copy link
Owner

h2zero commented Jul 4, 2023

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

@mhaberler
Copy link
Author

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

@h2zero
Copy link
Owner

h2zero commented Jul 4, 2023

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

@mhaberler
Copy link
Author

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
Copy link
Author

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

@h2zero
Copy link
Owner

h2zero commented Jul 5, 2023

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.

@mhaberler
Copy link
Author

mhaberler commented Jul 6, 2023

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

@h2zero
Copy link
Owner

h2zero commented Jul 9, 2023

That should be the process, looks good to me.

@mhaberler
Copy link
Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants