Skip to content
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

STM32Cube support #872

Open
3 tasks
2bndy5 opened this issue Sep 9, 2022 · 27 comments
Open
3 tasks

STM32Cube support #872

2bndy5 opened this issue Sep 9, 2022 · 27 comments

Comments

@2bndy5
Copy link
Member

2bndy5 commented Sep 9, 2022

Preliminary work to get this lib working in the IDE was done by @colombojrj in an unprompted PR #863

This thread is a continuation of the discussion started from that PR. Because that PR did not follow our utility/template driver files, The PR was closed and the work was cloned to a branch here upstream: stm32cube-support

Todo

  • document how to integrate the RF24 lib into the IDE
  • create a CI workflow to test build/compile a gettingStarted.cpp example RF24 app using the STM32 framework. This may be optional since each STM32Cube project is specific to a user-defined chip/board. It may be better to have an example repo since 1 simple STM32Cube project has so many files.
  • refactor initial work to be more readable using the utility/template paradigm (while also conforming to the .clang-format style guide).

Notes

This project is predominantly CMake driven now. Any Makefiles generated for this project are geared for the armhf compiler on Linux (RPi OS 32-bit). The STM32Cube IDE defaults to using Makefiles for the arm-none-eabi-gcc/++ compilers. The new CI may benefit from a CMake driven example, but it may not be necessary if it is a project that can be imported by the STM32Cube IDE. Using PlatformIO + STM32Cube framework may help in this regard as there should be minimal setup & configuration needed in this new CI workflow.

All use of printf() in RF24 is for debugging output only. Historically, the easiest approach to debugging programs in the Arduino IDE has been using the USB Serial monitor. However, the STM32Cube IDE largely relies on using an ST-Link debugging probe. Because an app developed in the STM32Cube IDE does not automatically include enumerating a USB CDC Serial device (which is board specific), the use of printf() is redundant and should only be used if USB CDC is setup for the STM32 project. Otherwise sprintfPrettyDetails() should be more flexible in that the buffer of debugging info can be directed to any bus (possibly even through the ST-Link). Another alternative is using the encodeRadioDetails() which dumps the radio's registers' values to a uint8_t array for external interpretation.

Lastly, all documentation provided by STM is scattered (based on components or chip family) or just non-existent. Our own document on getting RF24 started with STM32 devices may require linking to community answers for additional resources 😞.

@2bndy5
Copy link
Member Author

2bndy5 commented Sep 9, 2022

I think easiest way to integrate RF24 lib into a STM32Cube project is to add it as an included directory (-I).

Copying all the files into the project would break builds because the build system seems to compile all files under the Core/Src folder (with gcc command by default). I had to explicitly change the build command for the Core/Src/main.c file to g++ so it could #include "RF24.h which is a C++ lib (incapable of compiling with strictly a C compiler).

@colombojrj How did you try integrating RF24 into your STM32Cube project?

CMake support is secondary since STM32Cube has no support for it (by default).

@colombojrj
Copy link

colombojrj commented Sep 10, 2022

Hi, sorry for the delay. Firstly, I would like to thank for the opportunity of contributing with this library.

How I would like to integrate RF24 into my stm32 code? (aka my dream)

  1. If using eclipse (stm32cubeide or cubemx) I would like to clone RF24 repository, add it into my project code, disable unwanted folders (RPi, MRAA, ATTiny, etc) and adding a compiler definition in eclipse: -DRF24_STM32 (or something like that). This approach is consistent with other libraries to stm32, for example ee and ssd1306.
  2. If using CMake, then I would like to clone RF24 repository, add it into my project code and add two lines in my CMakeLists.txt: add_compiler_definitions(-DRF24_STM32) and add_subdirectory(rf24).

How to add examples into the RF24 library?

When working on stm32 projects, I (usually) keep in git only the ioc file. I (usually) do not add the STM32F1 ST library. I believe we could create examples using the STM32CubeIDE software and add only the changed files (basically main.c) and the ioc file. It may be possible to run some command to automatically generate code to that ioc file from the shell. This would allow for the automatic build tests.

Example of how I actually integrate RF24 into my code

I keep a project called Nrfusb, basically a custom BluePill with the RF24 firmware library. It is very useful to our robotics team. More specifically, here you will find how I integrated the RF24 library into my code.

What do you think? I believe that I can help with the examples.

@2bndy5
Copy link
Member Author

2bndy5 commented Sep 10, 2022

Help with the examples would be appreciated. Although I'm still not sure how to generate Makefiles that are used to drive the compiler.

After looking into the CMake support in the IDE, it seems that STM should implement official CMake support if we go that route. I don't want to rely on third party solutions/tutorials to instruct users on how to use CMake in STM32 projects.

Therefore, I think the first point (cloning RF24 into the project) should be our official approach. We can keep a few lines of code in the CMakeLists.txt, but that would be for advanced users only (& more driven by community support).

In fact, I think you actually outlined a rough draft of the document we need to create (about getting RF24 integrated into the IDE). 💯

I will do some more testing and push my changes...


I would also like to see better support for using a specified SPI bus, but that will come later.

@colombojrj
Copy link

colombojrj commented Sep 10, 2022 via email

@colombojrj
Copy link

colombojrj commented Sep 10, 2022 via email

@2bndy5
Copy link
Member Author

2bndy5 commented Sep 10, 2022

I imagine the "Drivers" folder (in the STM32Cube project) is the correct place to clone RF24 libs, yes?

@colombojrj
Copy link

colombojrj commented Sep 10, 2022 via email

@2bndy5
Copy link
Member Author

2bndy5 commented Sep 10, 2022

I think a separate utility/STM32/includes.h should be created for the various HAL files specific to STM chips. This should ease the contribution of patches to support STM32 chips that we might be currently overlooking.

@2bndy5
Copy link
Member Author

2bndy5 commented Sep 10, 2022

Ok. I refactored the original work to follow the utility/template better, ran clang-format on the added/modified sources, and started a how-to doc on the stm32cube-support branch.

It compiles in my local project following the steps outlined in the new doc. I relied on the IDE's auto-code generation feature, so I'm not sure if pin_mode() is actually needed.

I haven't actually run my project on my STM32F4 board (yet) because the code in main.c is not completely filled out. I just have the bare minimum to init the radio right now.


The SPI wrapper could be greatly improved because it now uses an OOP pointer. Meaning people should be able to use their own RF24_SPI object and pass it to the overloaded RF24::begin(_SPI* obj).

printf_P is currently a macro that takes no args. It could be defined under certain conditions (like #ifdef CDC_Transmit_FS for USB CDC Serial), but I need to learn more about the STM32Cube HALs.

@colombojrj
Copy link

Hi,
I have worked on the GettingStarted example. It depends on Serial.available() and Serial.read(). I removed the functionality of changing the radio role during operation because these methods are not available in the stm32 natively. Although we have USB_CDC_Receive, it has different purpose than Serial.read(). In short, Serial.read reads from a local buffer. USB_CDC_Receive is an interrupt/callback, invoked asynchronously. I believe that the main purpose of that example is to illustrate how to get things working. So, I am adding a lot of commentary about how the library is being integrated.

I will test the code today, and then I will be sharing it. I need to ask about how I should share the code. I have never reached this point in sharing code with people. Should I create a new branch and submit a pull request to the stm32 branch?

@2bndy5
Copy link
Member Author

2bndy5 commented Sep 12, 2022

It depends on Serial.available() and Serial.read(). I removed the functionality of changing the radio role during operation because these methods are not available in the stm32 natively.

Maybe we can use a builtin user button on the board (if it has one). My STM32F411 has a a user button labeled "key" (in addition to "boot" and "reset" buttons).

USB_CDC_Receive is an interrupt/callback, invoked asynchronously

Yeah, I think this might cause problems for an example that aims to be a simple demonstration. Alternatively, we could write comments to encourage users to change certain variables that affect the example's behavior, then they could simply compile and flash it to the chip.

I'm not very experienced with using semi-hosting, but maybe the ST-Link could be used as a Serial-like output. I'm not sure how it could be used to get user input though.

I need to ask about how I should share the code.

There are several ways. My first idea is create a gist on github and share the link. Although if you want to share more than 1 file, it might be better to create a PR from a branch on your fork. If you want to use a PR, then I would suggest you make it target the stm32Cube-support branch here on the the upstream RF24 repo (not our master branch).


A little off topic here, but I recently started exploring the Rust programming language which can also be deployed on microcontrollers. Apparently there is already enough HAL libs written in Rust to get the nRF24L01 working on various boards (including STM32 and nRF52 boards). I'm not giving up on this endeavor, but it may actually be easier for users to utilize Rust if they're trying to avoid using the STM32Cube IDE (which is basically Eclipse).

@colombojrj
Copy link

What do you think of this?

colombojrj@8b3374b

I am curious about Rust. I hope to try it soon.

@2bndy5
Copy link
Member Author

2bndy5 commented Sep 12, 2022

  1. Is it necessary to have 2 main files (main.c & main_cpp.cpp)? Is this supposed to help distinguish between using C or C++ compilers?
  2. If you rebase your branch on the changes I made to upstream stm32-cube-support, then declaring and instantiating an SPI object shouldn't be necessary (unless the app is using multiple SPI buses).

@colombojrj
Copy link

colombojrj commented Sep 13, 2022

  1. The extra file may not be called main_cpp.cpp. I do not know any other way of mixing C and C++ together. Is there any way of not using a separated C++ file?
  2. I am trying to rebase, but git is hard. If it is ok with you, I would like to keep the SPI declaration because one of the reasons to use the CubeHAL API is when you are using custom boards with many requirements. Once I developed a board with three SPI buses and each of them worked with different configuration (those modes in Arduino).

@2bndy5
Copy link
Member Author

2bndy5 commented Sep 13, 2022

Is there any way of not using a separated C++ file?

There are a few. In my test project I changed the properties of the main.c file to use g++ instead of the default gcc). But that started triggering a warning about specifying a C std version instead of a C++ std version, something like: "-std=gnu11 is an incompatible argument for the g++ compiler".

If having 2 separate files is more user friendly (for both project config and readability), then the cpp file should be named something more specific to the project.

This is something I forgot to address in the new how-to doc.


I am trying to rebase, but git is hard.

My apologies. I wasn't sure of your skill level in "git-fu". I think it would be easiest for your branch to just

git merge origin/stm32cube-support

If it is ok with you, I would like to keep the SPI declaration

You shouldn't need to have 3 SPI buses if all your changing is the bus config for each slave device - that is what Arduino's SPI::beginTransaction() is for. Using multiple SPI buses usually means that a slave device does not have a CS or CSN pin.

It doesn't make sense to declare an RF24_SPI object and never explicitly use it.

Furthermore, your code passes a SPIHandlerType to RF24::begin(&hspi1); this is conventionally flawed. The HAL specific implementation object should be passed to the wrapped RF24_SPI::begin(), so a customized RF24_SPI object can be passed to RF24::begin() to override using the default SPI bus.

Traditionally, if nothing is passed to RF24::begin(), then that means the hardware defaults are being used. I understand why that might be confusing in the STM32Cube IDE, but the library shouldn't suffer because the IDE & HAL has inadequate documentation.

@colombojrj
Copy link

Hi,

I think I have misunderstood. Do you mean something like this?

void setup() {
    // Initialize the RF24 SPI driver
    spi.begin(&hspi1);

    // initialize the transceiver on the SPI bus
    if (!radio.begin()) {
        print("radio hardware is not responding!!\n");

However, the spi.begin() still needs to be called with the hspi1 object handler. If not, the RF24_SPI class will pass nullptr to HAL_SPI_Init, which will fail:

HAL_StatusTypeDef HAL_SPI_Init(SPI_HandleTypeDef *hspi)
{
  /* Check the SPI handle allocation */
  if (hspi == NULL)
  {
    return HAL_ERROR;
  }

I am still not sure about do not pass the spi object to radio.begin(). I will test this code and post an update.

@2bndy5
Copy link
Member Author

2bndy5 commented Sep 13, 2022

If you use the refactored code that I pushed to stm32cube-support branch:

void RF24_SPI::begin()
{
HAL_SPI_Init(_hspi);
}
void RF24_SPI::begin(SPI_HandleTypeDef* hspi)
{
_hspi = hspi;
RF24_SPI::begin();
}

then you can

RF24_SPI spi();
spi.begin(&hspi);
radio.begin(&spi);

I haven't tested this yet, but this is how we handle SPI buses that are specified by the user.

@laserranger3000
Copy link

laserranger3000 commented Jan 15, 2023

Thanks for your work so far!
I'm trying to implement this to my project, but following the documentation is not working fully.
I have created a new C++ project for my F746ZG Nucleo board, copied the lib to the Drivers folder excluding every folder except utility/STM32, included "RF24.h" to my main.c and added the relevant symbols in the properties. I'm facing the following issues:

  • the F7 chips were missing from includes.h, therefore I've added:
#elif defined(STM32F7)
    #include "stm32f7xx_hal.h"
    #include "stm32f7xx_hal_gpio.h"
    #include "stm32f7xx_hal_spi.h"
  • compiling fails ("RF24.h: No such file or directory"), therefore I've added this path to the GCC and G++ compiler settings "../Drivers/RF24"
  • compiling still fails, because spi.h is being compiled by the GCC compiler despite being written in cpp

At this point I'm not sure if I'm missing something... Did I follow the instructions correctly?
Creating a dummy main.cpp file as workaround might work, but is that the solution?

@2bndy5
Copy link
Member Author

2bndy5 commented Jan 15, 2023

RF24 libs are C++ libs, so the main driver file should be a .cpp file. You can of course change the eclipse settings of the project to invoke g++ for certain headers, but that seems a bit tedious. I think if you increase the C++ std to 11 or 14 (in eclipse settings for g++ compiler args: -std=c++11), then I think it should auto switch to g++ for header files with a .cpp implementation file.

We didn't add support for the STM32F7 series because we didn't have a board to test it on. You could make a PR to the stm32cube-support branch which would include the snippet you posted and doc tweaks (as you see fit).

I haven't worked on this issue in a while because the build system isn't generated by CMake (which would make this a lot easier); ST devs don't seem interested in changing that. And, it seemed like the HAL API for each stm32fxxx series was slightly different in various ways (👎), so you may still run into compiler errors related to symbols used from the HAL API.

@2bndy5
Copy link
Member Author

2bndy5 commented Jan 16, 2023

@laserranger3000 This is a quote from an above comment that you might find helpful

Is there any way of not using a separated C++ file?

There are a few. In my test project I changed the properties of the main.c file to use g++ instead of the default gcc). But that started triggering a warning about specifying a C std version instead of a C++ std version, something like: "-std=gnu11 is an incompatible argument for the g++ compiler".

@laserranger3000
Copy link

It's compiling now by specifying g++ for main.c as suggested, thanks

@2bndy5
Copy link
Member Author

2bndy5 commented Jan 18, 2023

Let us know if you find further needed tweaks. Like I said, a PR would be welcome.

@colombojrj
Copy link

Hi, hope you are doing fine. I have worked very hard on testing the radio library with STM32 and (I believe) I have found a problem with my implementation.

The actual implementation of delayMicroseconds (to STM32) is:

#define delayMicroseconds(usecs) __usleep(usecs)

which is replaced with:

void __usleep(int32_t usecs)
{
    uint32_t now = rf24_get_time_us();
    uint32_t blocked_until = now + usecs;
    while (blocked_until > rf24_get_time_us()) {}
};

which depends on:

static uint32_t rf24_get_time_us()
{
    return 1000 * HAL_GetTick() + 1000 - (SysTick->VAL / (SystemCoreClock / 1000000));
}

But when attending the radio interrupts, the registers may not get updated, because they are updated in other interrupt, i.e., the internal value of HAL_GetTick() is not incremented. This results in a complete freeze. A simple solution is to add a timeout protection on the delayMicroseconds function, such as:

void __usleep(uint32_t usecs)
{
    const uint32_t deadlineUs = rf24_get_time_us() + usecs;
    uint32_t count = 4 * usecs;

    while ((deadlineUs > rf24_get_time_us()) && (count > 0))
    {
        count = count -1;
    }
}

This delay function (may) be used in csn function. It may be worth to check if other architectures also would suffer of similar problem. I hope to finish the examples code this year.

@2bndy5
Copy link
Member Author

2bndy5 commented Jul 26, 2023

Arduino platforms prohibit use of delay(), delayMicroseconds(), millis(), and micros() explicitly for this reason. It is safe to say that most architectures suffer from the experience you describe. Furthermore, we explicitly note this limitation in our docs about write() related functions (which should not be calling within an interrupt routine).

Instead of working around this common limitation in the library code, you should use a volatile bool variable as an event/state flag in the user-space code, so that the main loop can perform whatever reaction to the interrupt event is desired without the worrying about time-keeping inaccuracy.

Some pseudo-code to overcome any language barrier:

// a flag to keep track of the interrupt event
volatile bool hasRX = false;

// the callback function called by an interrupt:
void rx_interrupt() {
    hasRX = true;
}

// the main loop/function
void main() {
    if (hasRX) {
        radio.write( /* ... */ );
        hasRX = false;
    }
}

@colombojrj
Copy link

colombojrj commented Jul 26, 2023

I got the tip from your documentation. And I have done exactly that (your pseudo code) in my code. I was not sure if I should add a timeout in the code.

@colombojrj
Copy link

I also need to ask another question. Would you have some kind of benchmark? I have tested the AckPayloads and the write method takes something like 800 us to run. In ping-pong example (where the radios change roles), I got something about 3300 us between sending and receiving an answer. Are these values too differente from arduino?

@2bndy5
Copy link
Member Author

2bndy5 commented Jul 26, 2023

I would discourage adding any error checking to the delayMicroseconds() implementation because getting incorrect timing information during an interrupt routine is actually expected behavior.


Are these values too differente from arduino?

It really depends on the radio settings and local environment (like temperature, surrounding objects, etc), but you should strive to get something close to what the Arduino examples show. 800 us sounds about right, but 3300 us is something I would expect from a slower platform (like Circuitpython). I suspect there's something weird going on there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants