h2zero / esp-nimble-cpp

C++ library for the esp32 NimBLE stack based on and mostly compatible with @nkolban cpp_utils BLE library.
https://h2zero.github.io/esp-nimble-cpp/
Apache License 2.0
181 stars 62 forks source link

fix: #200 Enable use of `data()`/`size()` before trying `c_str()`/`length()` #201

Closed finger563 closed 1 month ago

finger563 commented 1 month ago

Closes #200

Replace test for c_str() and length() (which only matches a few stl containers), for the more general data() and size() check.

I tested with this example application and it now works properly:

https://github.com/finger563/nimble-cpp-hid-issue-example/tree/main

Note: I realize I accidentally committed the update to NimBLEHIDDevice to expose the reportMap characteristic. Personally I think it's a good change, but LMK if you want to revert that.

finger563 commented 1 month ago

Follow-up: if you're willing to require c++17 (c++20 is default in esp-idf v5), then we can use the concepts and requires functionality to clean up the metaprogramming.

h2zero commented 1 month ago

Thanks, the problem with this is that it will not work with Arduino String, which is why I used the c_str and length check since those functions are available in std::string and String.

finger563 commented 1 month ago

@h2zero I've pushed an update which keeps support for data/size but will fall back to c_str/length and finally &v[0]/sizeof(v)

finger563 commented 1 month ago

FYI, esp-idf 4.4 reached end of life back in July, so it is no longer supported. Might be worth upgrading your CI to allow the codebase to migrate to more modern standards :)

h2zero commented 1 month ago

Looks good to me, thanks!. I'll do some testing then merge shortly.

finger563 commented 1 month ago

@h2zero Thanks, I've addressed your comments, hopefully they all pass now :)

h2zero commented 1 month ago

Thanks!