stm32/wb55: Add usb/bluetooth transparent mode to stm32wb55 via native module. #9046

andrewleech · 2022-08-13T02:21:30Z

Alternate implementation of #7703 which moves most of the task-specific code to a native module example.

This version has full support for use with STM32CubeMonitor-Rf app. It should still also work as an external usb-cdc-hci radio for use with BLE in the unix port of micropython.

With this micropython compiled and flashed to the WB55 board (currently tested on nucleo) the native module can be compiled and deployed with:

cd examples/natmod/stm32wb55_transparent_vcp
make
mpremote cp transparent_vcp.mpy :

I'm using it from main.py as such:

from pyb import Pin, LED

sw = Pin("SW3", Pin.IN, Pin.PULL_UP)

if sw.value():
    LED(2).on()
    import transparent_vcp
    transparent_vcp.start()

codecov-commenter · 2022-08-13T02:41:34Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.50%. Comparing base (b336b6b) to head (f8fb56a).
Report is 1662 commits behind head on master.

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #9046   +/-   ##
=======================================
  Coverage   98.50%   98.50%           
=======================================
  Files         155      155           
  Lines       20549    20549           
=======================================
  Hits        20241    20241           
  Misses        308      308

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

dpgeorge · 2022-08-25T07:36:11Z

docs/library/stm.rst

@@ -102,3 +102,11 @@ the second CPU, the RF core.
    Execute a HCI command on the SYS channel.  The execution is synchronous.

    Returns a bytes object with the result of the SYS command.
+
+.. function:: rfcore_ble_hci(ogf, ocf, data) or rfcore_ble_hci(buffer)


Since the first form is not used (yet!) I'd say just implement the second form. It can be enhanced later if anyone ever needs it (BLE HCI commands are arguably different to SYS HCI commands, so I don't think it's a problem having a different function signature here, compared to rfcore_sys_hci).

Also, it could be: rfcore_ble_hci(cmd, outbuf) -> int where outbuf is a bytearray used to store the result, and it returns the number of bytes copied into that buffer. Then this function would be fully in-place (no heap allocation needed). Would you consider implementing that instead? I don't think it would change much (_stm32wb55_transparent.c already has a big 1024 byte buffer which could be reused for the outbuf).

Thanks @dpgeorge I've removed that second / unused form of the function, this does simplify things.

I've also added the second response_buf arg as optional - it certainly is a worthwhile option to allow no-alloc useage of the function. If the buffer arg isn't supplied I've kept it returing the fully-formed bytes object as this simpler usage could be handy for simple use cases.

I've used this buffer arg for the native module too as suggested, it's working well and cleaner overall.

dpgeorge · 2022-08-25T07:36:30Z

examples/natmod/stm32wb55_transparent_vcp/Makefile

+
+# Source files (.c or .py)
+SRC = \
+ _stm32wb55_transparent.c \


remove the leading underscore from the name?

Thought it might be useful to differentiate that this is intended to be "internal" as opposed to the .py file being the public interface, like the C accelerator modules on cpython. The implemented / exposed C module no longer matches the filename anyway so yeah the underscore isn't really useful here.

dpgeorge · 2022-08-25T07:36:38Z

examples/natmod/stm32wb55_transparent_vcp/Makefile

+ stm32wb55_transparent_vcp.py \
+
+
+# $(MPY_DIR)/ports/stm32/rfcore.c


remove unused lines

I've tried to properly clean up all the code / comments now in the latest push thanks.

dpgeorge · 2022-08-25T07:37:24Z

examples/natmod/stm32wb55_transparent_vcp/_stm32wb55_transparent.c

+#include "stm32wbxx_ll_utils.h"
+#include "stm32wbxx_ll_system.h"
+#include "ports/stm32/mpbthciport.h"
+#include "ports/stm32/rfcore.h"


I don't think these 2 includes are needed, neither the py/stream.h one. Then a lot of the CFLAGS lines in the Makefile can also be removed.

Ah yes, in earlier versions of this I was building stm32/rfcore.c into the module as well to use some of its functions... until I realised I obviously couldn't access the shared memory buffers this way.

Yep latest push has all of these cleaned up / minimsed and as suggested, the Makefile was able to become much simpler as well.

dpgeorge · 2022-08-25T07:37:43Z

examples/natmod/stm32wb55_transparent_vcp/_stm32wb55_transparent.c

+#include "ports/stm32/mpbthciport.h"
+#include "ports/stm32/rfcore.h"
+
+// Based on STM32CubeWB\Projects\P-NUCLEO-WB55.USBDongle\Applications\BLE\BLE_TransparentModeVCP


Are there any licensing/copyright issues to worry about here? Was any code copied?

Thanks for the reminder:

* Copyright (c) 2021 STMicroelectronics. * All rights reserved. * * This software is licensed under terms that can be found in the LICENSE file * in the root directory of this software component. * If no LICENSE file comes with this software, it is provided AS-IS.

A couple of folders up I find:
https://github.com/STMicroelectronics/STM32CubeWB/blob/83aacadecbd5136ad3194f39e002ff50a5439ad9/Projects/P-NUCLEO-WB55.USBDongle/Applications/LICENSE.md

SLA0044 Rev5/February 2018

It appears to be a generally permissive licence

Point 5 in particular appears to be a bit sticky:

No use, reproduction or redistribution of this software partially or totally may be done in any manner that would subject this software to any Open Source Terms.

Perhaps I could snip the structs / functions from ST out into a separate file that includes this licence, then the rest of the C code here can be under the normal micropython licence.

As I suggested in previous comment I've split out all the re-used ST code into a separate C file which references a copy of the matching ST licence file.

While it would certainly be possibly to re-implement the functionality from this file (it's just accessing a bunch of CPU registers really) I think this module is most useful as an example of being able to directly re-use vendor code when suitable functions are provided this way.

Point 5 in particular appears to be a bit sticky

Yes it does. It says the code cannot be used in a way that it would be subject to Open Source Terms, and in particular it mentions the MIT license. I take this to mean that the MIT license cannot be made to apply to this ST code. That makes sense, but I'm not sure if including the code in this repo (which has an MIT license at the top level) violates that point (5).

The code in this file is still under the ST license, we don't intend to change it to MIT.

I'm really not sure here.

If we do go ahead with keeping this code, IMO the license text should just be pasted at the top of the ST file (and not in a separate ST_LICENSE file).

After some discussion with more knowledgeable people on this topic, it's too dangerous to include this code with this license. I think the main intent of this part of the license is to prevent it being incorporated in Linux as a driver, and I think it's safest if we just don't include it in this repo at all.

(That said, don't close this PR just yet!)

Yeah it's a pretty fuzzy distinction going on in that licence. The writing there would definitely block usage within a gpl project as they enforce that everything is gpl compatible. Usage in something mit based might be different as mixed licencing is allowed there... but I definitely don't want to get you into hot water here!

This example would likely be safer split out into a separate repo, acting as a useful guide on how to have out-of-tree C module builds, perhaps then with a handy main.py file to set up the wb55 board/dongle as external radio for Unix micropython ble...

dpgeorge · 2022-08-25T07:37:56Z

examples/natmod/stm32wb55_transparent_vcp/_stm32wb55_transparent.c

+// Don't enable this if stdio is used for transp comms.
+#define DEBUG_printf(...)  // mp_printf(&mp_plat_print, "rfcore_transp: " __VA_ARGS__)
+
+// TODO Check ble_init_params vs SHCI_C2_BLE_Init /


Can this TODO be removed?

This was a reminder to me to compare the rfcore / cpu2 init options in

micropython/ports/stm32/rfcore.c

Line 568 in 203dae4

} ble_init_params = {

to the comparable settings in the transparent mode project from ST found (mostly) here: https://github.com/STMicroelectronics/STM32CubeWB/blob/83aacadecbd5136ad3194f39e002ff50a5439ad9/Projects/P-NUCLEO-WB55.USBDongle/Applications/BLE/BLE_TransparentModeVCP/Core/Inc/app_conf.h#L107

While there are differences, most of them are things that are set to 0 in micropython but higher numbers in the examples. These are settings that relate to the Host portion of the stack though, which we're not using from ST. The main discrepancy is MICROPY_HW_RFCORE_BLE_NUM_LINK (1) compared to the examples generally being set to 2. That's a question for a different discussion however, and the ``MICROPY` settings can be overridden for particular projects if needed anyway.

Either way, the TODO is removed now.

dpgeorge · 2022-08-25T07:38:33Z

examples/natmod/stm32wb55_transparent_vcp/_stm32wb55_transparent.c

+mp_obj_t stream_out_o;
+const mp_stream_p_t *stream_out_p;
+
+mp_uint_t mpstream_write(const void *buf, size_t len) {


Please call it mp_stream_write, also same for other 2 functions below.

Also I think it might be cleaner to pass in as the first argument the stream object, instead of having global variables.

See eg examples/natmod/btree/btree_c.c:mp_stream_posix_write

Ah yes, definitely. This was originally used in the callback in parse_hci_info_t parse directly in rfcore.c when I was using that directly. I forgot to unwind all of this once I added the rfcore_ble_hci micropython interface, now that it's always called directly it'll be easy enough to pass the stream through.

Yep this is fixed now making it much cleaner overall.

dpgeorge · 2022-08-25T07:40:59Z

examples/natmod/stm32wb55_transparent_vcp/_stm32wb55_transparent.c

+            } else {
+                // forward packet to rfcore
+                DEBUG_printf("rfcore_ble_hci_cmd\n");
+                mp_obj_t cmd = mp_obj_new_bytes(buf, rx);


Instead of create a new bytes object, can use:

mp_obj_array_t cmd = {{&mp_type_bytearray}, BYTEARRAY_TYPECODE, 0, rx, buf};

dpgeorge · 2022-08-25T07:41:36Z

ports/stm32/rfcore.c

@@ -762,4 +761,40 @@ STATIC mp_obj_t rfcore_sys_hci(size_t n_args, const mp_obj_t *args) {
 }
 MP_DEFINE_CONST_FUN_OBJ_VAR_BETWEEN(rfcore_sys_hci_obj, 3, 4, rfcore_sys_hci);

+STATIC void rfcore_ble_hci_response_to_buffer(void *env, const uint8_t *buf, size_t len) {
+    // mp_buffer_info_t *respbufinfo = (mp_buffer_info_t *)env;


please remove this line

dpgeorge · 2022-08-25T07:41:49Z

ports/stm32/rfcore.c

+        mp_int_t ogf = mp_obj_get_int(args[0]);
+        mp_int_t ocf = mp_obj_get_int(args[1]);
+        mp_get_buffer_raise(args[2], &bufinfo, MP_BUFFER_READ);
+        size_t len = tl_ble_hci_cmd_resp(HCI_OPCODE(ogf, ocf), bufinfo.buf, bufinfo.len);


this should be ssize_t

This whole section is now removed as it was just to support the other (unused) function branch.

dpgeorge · 2023-03-09T06:31:55Z

ports/stm32/rfcore.c

+            mp_raise_OSError(-len);
+        }
+        memcpy(bufinfo.buf, buf, len);
+        bufinfo.len = len;


this line does nothing

dpgeorge · 2023-03-09T06:33:30Z

examples/natmod/stm32wb55_transparent_vcp/stm32wb55_transparent.c

+
+    while (true) {
+        // Sleep briefly to allow micropython background processing.
+        mp_call_function_n_kw(sleep_ms_obj, 1, 0, (mp_obj_t[]) {MP_OBJ_NEW_SMALL_INT(1)});


I feel like this function should just do one iteration and then return to Python. Then it could, for example, be used with asyncio.

But that's a pretty big change, so I don't expect it to be made. Could be done in the future if anyone wanted to do it.

github-actions · 2023-03-10T11:41:49Z

Code size report:

   bare-arm:    +0 +0.000% 
minimal x86:    +0 +0.000% 
   unix x64:    +0 +0.000% standard
      stm32:    +0 +0.000% PYBV10
        rp2:    +0 +0.000% PICO

dpgeorge · 2023-03-10T11:48:44Z

I rebased this.

dpgeorge · 2023-03-20T00:25:53Z

Steps:

fix up comment above about bufinfo.len
merge changes to stm32 and docs
create a new repo just for the example
try to get as many files from Zephyr for the ST specific code
copy these needed 3rd-party files directly (ie no concatenating)
record the source/git hash of those files

projectgus · 2024-03-07T23:51:39Z

This is an automated heads-up that we've just merged a Pull Request
that removes the STATIC macro from MicroPython's C API.

See #13763

A search suggests this PR might apply the STATIC macro to some C code. If it
does, then next time you rebase the PR (or merge from master) then you should
please replace all the STATIC keywords with static.

Although this is an automated message, feel free to @-reply to me directly if
you have any questions about this.

…03-13 update frozen libraries for 9.0.0-rc.1

andrewleech force-pushed the wb55-transparent-mode-nm branch 2 times, most recently from 42fe637 to 0abc022 Compare August 13, 2022 02:33

andrewleech force-pushed the wb55-transparent-mode-nm branch 3 times, most recently from 3b8232a to 5443323 Compare August 17, 2022 01:22

dpgeorge added the port-stm32 label Aug 17, 2022

andrewleech force-pushed the wb55-transparent-mode-nm branch 2 times, most recently from c5d448f to 1455b37 Compare August 17, 2022 05:17

dpgeorge reviewed Aug 25, 2022

View reviewed changes

andrewleech force-pushed the wb55-transparent-mode-nm branch 4 times, most recently from 6cc193a to ab7db68 Compare September 6, 2022 07:24

dpgeorge reviewed Mar 9, 2023

View reviewed changes

ports/stm32/rfcore.c

mp_raise_OSError(-len);

}

memcpy(bufinfo.buf, buf, len);

bufinfo.len = len;

Copy link

Member

dpgeorge Mar 9, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this line does nothing

dpgeorge reviewed Mar 9, 2023

View reviewed changes

pi-anl added 3 commits March 10, 2023 22:33

py/dynruntime: Expose bytearray definition for dynamic C modules.

352f253

stm32/rfcore: Add stm.rfcore_ble_hci function for raw BLE HCI comms.

34e7f6f

examples/natmod: Add stm32wb55_transparent_vcp.

f8fb56a

dpgeorge force-pushed the wb55-transparent-mode-nm branch from ab7db68 to f8fb56a Compare March 10, 2023 11:35

dpgeorge mentioned this pull request Mar 10, 2023

stm32/wb55: Add usb/bluetooth transparent mode to stm32wb55. #7703

Open

Gadgetoid mentioned this pull request Feb 29, 2024

global: Remove the STATIC macro. #13763

Merged

RetiredWizard pushed a commit to RetiredWizard/micropython that referenced this pull request Mar 14, 2024

Merge pull request micropython#9046 from dhalbert/frozen-update-2024-…

39b16fe

…03-13 update frozen libraries for 9.0.0-rc.1

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

stm32/wb55: Add usb/bluetooth transparent mode to stm32wb55 via native module. #9046

stm32/wb55: Add usb/bluetooth transparent mode to stm32wb55 via native module. #9046

andrewleech commented Aug 13, 2022 •

edited

Loading

codecov-commenter commented Aug 13, 2022 •

edited by codecov bot

Loading

dpgeorge Aug 25, 2022

andrewleech Sep 6, 2022

dpgeorge Aug 25, 2022

andrewleech Sep 1, 2022

dpgeorge Aug 25, 2022

andrewleech Sep 6, 2022

dpgeorge Aug 25, 2022

andrewleech Sep 1, 2022

andrewleech Sep 6, 2022

dpgeorge Aug 25, 2022

andrewleech Sep 1, 2022 •

edited

Loading

andrewleech Sep 6, 2022

dpgeorge Mar 9, 2023

dpgeorge Mar 9, 2023

dpgeorge Mar 16, 2023

andrewleech Mar 16, 2023 •

edited

Loading

dpgeorge Aug 25, 2022

andrewleech Sep 6, 2022

dpgeorge Aug 25, 2022

andrewleech Sep 2, 2022

andrewleech Sep 6, 2022

dpgeorge Aug 25, 2022

dpgeorge Aug 25, 2022

dpgeorge Aug 25, 2022

andrewleech Sep 6, 2022

dpgeorge Mar 9, 2023

dpgeorge Mar 9, 2023

github-actions bot commented Mar 10, 2023

dpgeorge commented Mar 10, 2023

dpgeorge commented Mar 20, 2023

projectgus commented Mar 7, 2024

		stm32wb55_transparent_vcp.py \


		# $(MPY_DIR)/ports/stm32/rfcore.c

stm32/wb55: Add usb/bluetooth transparent mode to stm32wb55 via native module. #9046

Are you sure you want to change the base?

stm32/wb55: Add usb/bluetooth transparent mode to stm32wb55 via native module. #9046

Conversation

andrewleech commented Aug 13, 2022 • edited Loading

codecov-commenter commented Aug 13, 2022 • edited by codecov bot Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andrewleech Sep 1, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andrewleech Mar 16, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Mar 10, 2023

dpgeorge commented Mar 10, 2023

dpgeorge commented Mar 20, 2023

projectgus commented Mar 7, 2024

andrewleech commented Aug 13, 2022 •

edited

Loading

codecov-commenter commented Aug 13, 2022 •

edited by codecov bot

Loading

andrewleech Sep 1, 2022 •

edited

Loading

andrewleech Mar 16, 2023 •

edited

Loading