Skip to content

extmod/nimble: Support pairing/bonding on the ESP32. #13258

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

Merged
merged 3 commits into from
Feb 29, 2024

Conversation

DvdGiessen
Copy link
Contributor

@DvdGiessen DvdGiessen commented Dec 22, 2023

Two fixes to NimBLE to fix compatibility with the ESP-IDF.

The first commit moves the runtime initialisation of ble_hs_cfg to happen after nimble_port_init(). That is consistent with the order used in NimBLE examples. On the ESP32 port this is needed because the ESP-IDF sets up the default RAM secret store callbacks in its implementation of nimble_port_init() (specifically, it calls esp_nimble_init() which in turn calls ble_store_ram_init()). We want to override those callbacks with our own so used to implement the IRQ_[GS]ET_SECRET events in Python.

The second commit sets the BLE key distribution parameters at runtime. This isn't needed in most ports since we already set the default values in extmod/nimble/syscfg/syscfg.h; however in the ESP32 port that headerfile is not used, and the default values in the ESP-IDF don't enable key distribution nor can we change those defaults via sdkconfig. Setting the values explicitly at runtime seems like a reasonable solution.

These two commits together make persistent pairing/bonding work on the ESP32 in my own local tests.

Third commit updates the documentation to mention that the ESP32 now also supports pairing/bonding.

Fixes #12958.

@DvdGiessen DvdGiessen changed the title This extmod/nimble: Support pairing/bonding on the ESP32. Dec 22, 2023
Copy link

Code size report:

   bare-arm:    +0 +0.000% 
minimal x86:    +0 +0.000% 
   unix x64:    +0 +0.000% standard
      stm32:    +0 +0.000% PYBV10
     mimxrt:    +0 +0.000% TEENSY40
        rp2:    +0 +0.000% RPI_PICO
       samd:    +0 +0.000% ADAFRUIT_ITSYBITSY_M4_EXPRESS

Copy link

codecov bot commented Dec 22, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.36%. Comparing base (56f9dcb) to head (678707c).

Additional details and impacted files
@@           Coverage Diff           @@
##           master   #13258   +/-   ##
=======================================
  Coverage   98.36%   98.36%           
=======================================
  Files         161      161           
  Lines       21091    21091           
=======================================
  Hits        20746    20746           
  Misses        345      345           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@DvdGiessen
Copy link
Contributor Author

Rebased to include the explanations in the commit messages.

@dpgeorge
Copy link
Member

dpgeorge commented Jan 5, 2024

Thanks, this looks good.

But I'm having trouble testing it. Using IDF 5.0.4 and a normal ESP32 without SPIRAM, running the multi_bluetooth tests against a PYBD-SF6 (which also runs NimBLE with this PR), the ESP32 is crashing for a lot of the tests. Although testing without this PR it also crashes...

What IDF version and hardware set up do you use for the testing? Can you try running the multitests with two ESP32's, like this:

./run-multitests.py -i pyb:u0 -i pyb:u1 multi_bluetooth/ble_g*.py

(You may need to adjust u0 and u1 to match your serial ports.)

@robert-hh
Copy link
Contributor

robert-hh commented Jan 5, 2024

When testing BLE for the Arduino Nanoc connect 2040 and the SAMD extension @iabdalkader and me have seen a largely inconsistent test coverage. Some tests always failed, some tests always succeeded, and some tests succeeded sometimes.
Edit: We removed the test ble_deepsleep.py, because it caused the test to crash, and stress_log_filesystem, because it failed all of the time.
/Edit
The table below show pass rates of 100 test suite iterations:

Arduino Nano Connect 2040

40 ble_characteristic
100 ble_gap_advertise
100 ble_gap_connect
71 ble_gap_device_name
71 ble_gap_pair_bond
53 ble_gap_pair
72 ble_gattc_discover_services
52 ble_gatt_data_transfer
31 ble_l2cap
1 ble_mtu
33 ble_subscribe
91 perf_gatt_char_write
42 perf_gatt_notify
31 perf_l2cap

SAMD ItsyBitsyM4 with Airlift hat

77 ble_characteristic
100 ble_gap_advertise
100 ble_gap_connect
63 ble_gap_device_name
40 ble_gap_pair_bond
54 ble_gap_pair
55 ble_gattc_discover_services
63 ble_gatt_data_transfer
83 ble_l2cap
0 ble_mtu
57 ble_subscribe
94 perf_gatt_char_write
58 perf_gatt_notify
58 perf_l2cap

The pass rates vary somewhat if the test are changed, especially the timeout settings.

BLE testing works fine with the PYBD-SF6, and it works as well fine for ESP32 devices as BLE modem running the esp-hosted firmware. Then we get about 100% pass rates for the test.

@DvdGiessen
Copy link
Contributor Author

DvdGiessen commented Jan 5, 2024

What IDF version and hardware set up do you use for the testing?

I currently build against IDFv5.0.5 (using DvdGiessen@a4bec6a). Most of my development and testing was done on an ESP32-S3 and connecting to it with an Android app, but I've just ran the testsuite between a regular ESP32 (with 4MB SPIRAM) and ESP32-S3 (with 8MB SPIRAM).

Tests from esp32 to esp32s3 succeed:

multi_bluetooth/ble_gap_advertise.py on esp32|esp32s3: pass
multi_bluetooth/ble_gap_connect.py on esp32|esp32s3: pass
multi_bluetooth/ble_gap_device_name.py on esp32|esp32s3: pass
multi_bluetooth/ble_gap_pair.py on esp32|esp32s3: pass
multi_bluetooth/ble_gap_pair_bond.py on esp32|esp32s3: pass
multi_bluetooth/ble_gatt_data_transfer.py on esp32|esp32s3: pass
multi_bluetooth/ble_gattc_discover_services.py on esp32|esp32s3: pass
7 tests performed
7 tests passed

However, I ran again with -p 2 and noticed that the other way round I'm consistently getting a panic on the ESP32 (Interrupt wdt timeout on CPU0, trace only contains btdm_controller_task):

1 tests failed: multi_bluetooth/ble_gap_connect.py

I poked around for a bit but haven't seen what's different there yet, maybe I'll find some time to dive into that later.

@DvdGiessen
Copy link
Contributor Author

DvdGiessen commented Feb 28, 2024

I've recently upgraded to IDF v5.2 (see #13775), so just decided to re-run these tests against that version. It seems all the issues I saw previously are gone:

$ ./run-multitests.py -p 2 -i pyb:/dev/ttyESP32S3 -i pyb:/dev/ttyESP32 multi_bluetooth/ble_g*.py
multi_bluetooth/ble_gap_advertise.py on esp32s3|esp32: pass
multi_bluetooth/ble_gap_connect.py on esp32s3|esp32: pass
multi_bluetooth/ble_gap_device_name.py on esp32s3|esp32: pass
multi_bluetooth/ble_gap_pair.py on esp32s3|esp32: pass
multi_bluetooth/ble_gap_pair_bond.py on esp32s3|esp32: pass
multi_bluetooth/ble_gatt_data_transfer.py on esp32s3|esp32: pass
multi_bluetooth/ble_gattc_discover_services.py on esp32s3|esp32: pass
7 tests performed
7 tests passed
multi_bluetooth/ble_gap_advertise.py on esp32|esp32s3: pass
multi_bluetooth/ble_gap_connect.py on esp32|esp32s3: pass
multi_bluetooth/ble_gap_device_name.py on esp32|esp32s3: pass
multi_bluetooth/ble_gap_pair.py on esp32|esp32s3: pass
multi_bluetooth/ble_gap_pair_bond.py on esp32|esp32s3: pass
multi_bluetooth/ble_gatt_data_transfer.py on esp32|esp32s3: pass
multi_bluetooth/ble_gattc_discover_services.py on esp32|esp32s3: pass
7 tests performed
7 tests passed

So perhaps a BLE issue was fixed upstream?

@dpgeorge
Copy link
Member

So perhaps a BLE issue was fixed upstream?

Maybe.

Also we had a few BLE fixes to esp32 in the meantime, see c27d304 and cd66aa0

@dpgeorge
Copy link
Member

Testing latest master as at a30c293, with ESP32_GENERIC and PYBD_SF6, the 14 standard BLE multitests pass reliably now, still using IDF 5.0.4.

This moves the runtime initialisation of `ble_hs_cfg` to happen after
`nimble_port_init()`. That is consistent with the order used in NimBLE
examples. On the ESP32 port this is needed because the ESP-IDF sets up
the default RAM secret store callbacks in its implementation of
`nimble_port_init()` (specifically, it calls `esp_nimble_init()` which
in turn calls `ble_store_ram_init()`). We want to override those with
our own callbacks to implement the `IRQ_[GS]ET_SECRET` events in Python.

Signed-off-by: Daniël van de Giessen <daniel@dvdgiessen.nl>
This sets the BLE key distribution parameters at runtime. This isn't
needed in most ports since we already set the default values in
`extmod/nimble/syscfg/syscfg.h`; however in the ESP32 port that
headerfile is not used, and the default values in the ESP-IDF don't
enable key distribution nor can we change those defaults via
`sdkconfig`. Thus we're setting these values explicitly at runtime.

Signed-off-by: Daniël van de Giessen <daniel@dvdgiessen.nl>
Pairing and bonding was fixed for the ESP32 in the two previous commits.

Signed-off-by: Daniël van de Giessen <daniel@dvdgiessen.nl>
@dpgeorge
Copy link
Member

Testing this PR with IDF 5.0.4, it works reliably (with a ESP32_GENERIC and PYBD_SF6).

@dpgeorge dpgeorge merged commit 678707c into micropython:master Feb 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ESP-32 BLE w/ NimBLE does not support bonding/pairing
3 participants