taks / esp32-nimble

A wrapper for the ESP32 NimBLE Bluetooth stack.
Apache License 2.0
102 stars 31 forks source link

Mark `&mut BLEAdvertising` as `Send`? #72

Closed ChocolateLoverRaj closed 1 month ago

ChocolateLoverRaj commented 7 months ago

Is it safe for &mut BLEAdvertising to be marked with Send similar to https://github.com/taks/esp32-nimble/commit/046882663252b08626744cb1ee3d9fffa5ef1fa2? I want to access the advertising from the on_write callback. Currently I'm using channels, but it would be more convenient if I could just use Arc.

This is related to #17

ChocolateLoverRaj commented 7 months ago

I still get this error:

error[E0277]: `*const ble_uuid16_t` cannot be shared between threads safely
      = help: within `&mut BLEAdvertising`, the trait `Sync` is not implemented for `*const ble_uuid16_t`
note: required because it appears within the type `ble_hs_adv_fields`
     --> /var/home/rajas/Documents/esp32c3/sensor-connect/target/riscv32imc-esp-espidf/debug/build/esp-idf-sys-501379320fed5ae9/out/bindings.rs:65854:12
      |
65854 | pub struct ble_hs_adv_fields {
      |            ^^^^^^^^^^^^^^^^^
note: required because it appears within the type `BLEAdvertising`
     --> /var/home/rajas/.cargo/git/checkouts/esp32-nimble-fc9d2c54a3bfa7fe/02e47f7/src/server/ble_advertising.rs:17:12
      |
17    | pub struct BLEAdvertising {
      |            ^^^^^^^^^^^^^^
      = note: required because it appears within the type `&mut BLEAdvertising`
      = note: required for `RwLock<&mut BLEAdvertising>` to implement `Sync`
      = note: required for `Arc<RwLock<&mut BLEAdvertising>>` to implement `std::marker::Send`

is this expected behavior? Is it actually unsafe to share &mut BLEAdvertising between threads? Is the only solution for me to use channels?

Full error:

error[E0277]: `*const ble_uuid16_t` cannot be shared between threads safely
     --> src/main.rs:131:13
      |
130   |           .on_write(
      |            -------- required by a bound introduced by this call
131   | /             move |args| match String::from_utf8(args.recv_data.to_vec()) {
132   | |                 Ok(short_name) => match validate_short_name(&short_name) {
133   | |                     Ok(_) => set_short_name(&short_name),
134   | |                     Err(message) => warn!("{}", message),
...     |
139   | |                 }
140   | |             },
      | |_____________^ `*const ble_uuid16_t` cannot be shared between threads safely
      |
      = help: within `&mut BLEAdvertising`, the trait `Sync` is not implemented for `*const ble_uuid16_t`
note: required because it appears within the type `ble_hs_adv_fields`
     --> /var/home/rajas/Documents/esp32c3/sensor-connect/target/riscv32imc-esp-espidf/debug/build/esp-idf-sys-501379320fed5ae9/out/bindings.rs:65854:12
      |
65854 | pub struct ble_hs_adv_fields {
      |            ^^^^^^^^^^^^^^^^^
note: required because it appears within the type `BLEAdvertising`
     --> /var/home/rajas/.cargo/git/checkouts/esp32-nimble-fc9d2c54a3bfa7fe/02e47f7/src/server/ble_advertising.rs:17:12
      |
17    | pub struct BLEAdvertising {
      |            ^^^^^^^^^^^^^^
      = note: required because it appears within the type `&mut BLEAdvertising`
      = note: required for `RwLock<&mut BLEAdvertising>` to implement `Sync`
      = note: required for `Arc<RwLock<&mut BLEAdvertising>>` to implement `std::marker::Send`
note: required because it's used within this closure
     --> src/main.rs:108:9
      |
108   |         move |new_name: &str| {
      |         ^^^^^^^^^^^^^^^^^^^^^
note: required because it's used within this closure
     --> src/main.rs:131:13
      |
131   |             move |args| match String::from_utf8(args.recv_data.to_vec()) {
      |             ^^^^^^^^^^^
note: required by a bound in `BLECharacteristic::on_write`
     --> /var/home/rajas/.cargo/git/checkouts/esp32-nimble-fc9d2c54a3bfa7fe/02e47f7/src/server/ble_characteristic.rs:140:46
      |
138   |   pub fn on_write(
      |          -------- required by a bound in this associated function
139   |     &mut self,
140   |     callback: impl FnMut(&mut OnWriteArgs) + Send + Sync + 'static,
      |                                              ^^^^ required by this bound in `BLECharacteristic::on_write`
taks commented 7 months ago

Is it possible to paste the main.rs source code?

ChocolateLoverRaj commented 7 months ago

Is it possible to paste the main.rs source code?

Here is the code which produces that error: https://github.com/ChocolateLoverRaj/rust-esp32c3-examples/blob/ae197aef46464ed5950683d3d34ae638a13e2c79/sensor-connect/src/main.rs#L132

it's not a minimum example. Let me know if I should make a minimum example.

ChocolateLoverRaj commented 7 months ago

I found out a much simpler way to reproduce the error: https://github.com/ChocolateLoverRaj/esp32-nimble/blob/d56af552fe6f8def6c951012b18c08ebff5dea03/examples/ble_secure_server.rs#L25

taks commented 7 months ago

Is it possible to paste the main.rs source code?

Here is the code which produces that error: https://github.com/ChocolateLoverRaj/rust-esp32c3-examples/blob/ae197aef46464ed5950683d3d34ae638a13e2c79/sensor-connect/src/main.rs#L132

it's not a minimum example. Let me know if I should make a minimum example.

I corrected the error in the project you attached. Please check it. https://github.com/taks/rust-esp32c3-examples/tree/fix-build-error/sensor-connect

I found out a much simpler way to reproduce the error: https://github.com/ChocolateLoverRaj/esp32-nimble/blob/d56af552fe6f8def6c951012b18c08ebff5dea03/examples/ble_secure_server.rs#L25

It should work if you change _server to server on line 16.

ChocolateLoverRaj commented 7 months ago

Thanks for fixing the error. I noticed that you used esp32_nimble::utilities::mutex::Mutex instead of std::sync::Mutex. Why?