Skip to content

drivers/cywbt: Switch to new CYW43 BTHCI UART backend. #16848

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 4 commits into from
Apr 22, 2025

Conversation

iabdalkader
Copy link
Contributor

@iabdalkader iabdalkader commented Mar 3, 2025

Summary

Remove old BTCHI driver and switch to the new CYW43 BTHCI UART backend and update the CYW configuration accordingly. More details here: georgerobotics/cyw43-driver#137

Testing

mimxrt: Tested WiFi/BT with MIMXRT1062 + CYW43 module.
stm32: Tested WiFi/BT with Giga H7.

Requires:

This is also related #16833 I renamed the files to match the cyw43.

Copy link

github-actions bot commented Mar 3, 2025

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_W
       samd:    +0 +0.000% ADAFRUIT_ITSYBITSY_M4_EXPRESS
  qemu rv32:    +0 +0.000% VIRT_RV32

@iabdalkader iabdalkader force-pushed the mimxrt_cyw43_bthci branch 2 times, most recently from de42dd4 to 23c7f26 Compare March 30, 2025 07:37
@iabdalkader iabdalkader changed the title ports/mimxrt: Update CYW43 configuration. drivers/cywbt: Switch to new CYW43 BTHCI UART backend. Mar 30, 2025
@iabdalkader
Copy link
Contributor Author

iabdalkader commented Mar 30, 2025

@dpgeorge I updated this to remove the old cywbt.c and switch to the new backend. I believe it was only used by MIMXRT and STM32, and I think we should remove it since it's causing some confusion. At one point, I had two firmware images each linking to a different backend. I tested both port, WiFi (HTTP/SSL) and Bluetooth (aioble blinky), and both seem to work fine.

I'm not sure why SF2 is overflowing, maybe it never used the CYWBT firmware? It works without it btw. In this case we'll probably need a custom config for boards.

Copy link

codecov bot commented Mar 30, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.54%. Comparing base (5eee5a6) to head (23c7f26).

Additional details and impacted files
@@           Coverage Diff           @@
##           master   #16848   +/-   ##
=======================================
  Coverage   98.54%   98.54%           
=======================================
  Files         169      169           
  Lines       21890    21890           
=======================================
  Hits        21571    21571           
  Misses        319      319           

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@dpgeorge
Copy link
Member

dpgeorge commented Apr 8, 2025

Thanks, this is a good change. But it's is a bit too risky before a release, and will need to wait until after 1.25.0.

@iabdalkader
Copy link
Contributor Author

Thanks, this is a good change. But it's is a bit too risky before a release, and will need to wait until after 1.25.0.

I could rebase on #17050 and wait.

@dpgeorge
Copy link
Member

I'm not sure about mimxrt, but stm32 needs the following additional defines to make the new BT driver behave like the current one:

#define CYW43_HAL_UART_READCHAR_BLOCKING_WAIT __WFI()

#define CYW43_PIN_RFSW_SELECT           pyb_pin_WL_GPIO_1

#ifdef MICROPY_HW_BLE_UART_BAUDRATE_SECONDARY
#define CYW43_BT_UART_BAUDRATE_ACTIVE_USE MICROPY_HW_BLE_UART_BAUDRATE_SECONDARY
#endif
#ifdef MICROPY_HW_BLE_UART_BAUDRATE_DOWNLOAD_FIRMWARE
#define CYW43_BT_UART_BAUDRATE_DOWNLOAD_FIRMWARE MICROPY_HW_BLE_UART_BAUDRATE_DOWNLOAD_FIRMWARE
#endif

And also needs:

--- a/ports/stm32/boards/PYBD_SF2/f722_qspi.ld
+++ b/ports/stm32/boards/PYBD_SF2/f722_qspi.ld
@@ -49,7 +49,7 @@ SECTIONS
         *lib/mbedtls/*(.text* .rodata*)
         *lib/mynewt-nimble/*(.text* .rodata*)
         *lib/cyw43-driver/*(.rodata.w4343*_combined)
-        *drivers/cyw43/*(.rodata.cyw43_btfw_*)
+        *lib/cyw43-driver/*(.rodata.cyw43_btfw_*)
         . = ALIGN(4);
     } >FLASH_EXT
 }

@iabdalkader iabdalkader force-pushed the mimxrt_cyw43_bthci branch 3 times, most recently from 73d411c to f19046c Compare April 21, 2025 13:00
@iabdalkader
Copy link
Contributor Author

I'm not sure about mimxrt, but stm32 needs the following additional defines to make the new BT driver behave like the current one:

I've added those to both stm32 and mimxrt. In Alif port, they're defined in board config files.

The new CYW43 BTHCI UART backend requires CTS pin to be defined and
readable.  This patch enables the CTS pin SION bit to allow it to be read
regardless of mux mode.

Signed-off-by: iabdalkader <i.abdalkader@gmail.com>
Update the cyw43 configuration to use the new BTHCI UART backend provided
by cyw43-driver.

Signed-off-by: iabdalkader <i.abdalkader@gmail.com>
Update the cyw43 configuration to use the new BTHCI UART backend provided
by cyw43-driver.

Signed-off-by: iabdalkader <i.abdalkader@gmail.com>
It has been completely replaced by equivalent code in cyw43-driver.

Signed-off-by: iabdalkader <i.abdalkader@gmail.com>
@dpgeorge dpgeorge force-pushed the mimxrt_cyw43_bthci branch from 5256f68 to 0b72962 Compare April 22, 2025 02:52
@dpgeorge dpgeorge merged commit 0b72962 into micropython:master Apr 22, 2025
23 checks passed
@iabdalkader iabdalkader deleted the mimxrt_cyw43_bthci branch April 22, 2025 07:33
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.

2 participants