Skip to content

STM32 ThreadX target platform #969

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 11 commits into from
Jun 6, 2025

Conversation

g4sp3r
Copy link
Contributor

@g4sp3r g4sp3r commented Jun 2, 2025

Hi,
At Ubiquity robotics we are happy users of zenoh-pico running on multiple of our stm32 based embedded projects 😍

We would like to contribute back this pull request that adds new STM32 platform with ThreadX system implementation.

Currently only supports serial transport. Implementation is with circular DMA for data reception.

Copy link

github-actions bot commented Jun 2, 2025

PR missing one of the required labels: {'documentation', 'bug', 'new feature', 'internal', 'enhancement', 'breaking-change', 'dependencies'}

@JEnoch JEnoch added the new feature Something new is needed label Jun 2, 2025
@JEnoch JEnoch requested a review from jean-roland June 2, 2025 13:01
@jean-roland
Copy link
Contributor

Hello @g4sp3r, thanks for your contribution, it seems the PR has two issues currently:

  • Formatting is not correct, please run find include/ src/ tests/ examples/ tools/ -iname "*.ino" -o -iname "*.h" -o -iname "*.c" | xargs clang-format -i in your zenoh-pico repo, you'll need to install clang-format if you haven't.
  • Your threadX files seem to be visible from other systems, make sure the includes are guarded against that.

@jean-roland
Copy link
Contributor

Highly possible the cpp CI failure is due to a flaky test and not related to your changes, so don't worry about it for now.

Copy link
Member

@sashacmc sashacmc left a comment

Choose a reason for hiding this comment

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

Dear @g4sp3r, thanks for your contribution!
Some cosmetic fixes.

I'm also curious, it seems strange that we can't do without copying the source files and manually excluding some of them from the project. Doesn't the build system provide an option to use a proper CMake file to do this, as it does for other platforms?

@g4sp3r
Copy link
Contributor Author

g4sp3r commented Jun 5, 2025

Hey @sashamc! Thanks for your fixes. They are accepted and PR is updated.

About the project setup, I am also not a fan and understand your concerns.

In my experience, most of the users will probably work with STMCubeIDE (eclipse) using CDT managed build and gui tools to configure their project and peripherals. This is also how our projects were configured from start and is also the way most of the examples for stm32 you can find online are done.

You can see the examples in this PR don't do any peripheral or kernel initialization so user needs to provide those layers by themselves. But this means examples could be run on any target that the manufacturer toolchain supports.

The main issue I see for CMake support is that the landscape is wider than for other platforms. To give some examples:

  • Correct hal drivers would need to be downloaded from ST for which license agreements need to be accepted.
  • threadx low level initialization is done in assembly and generated by the project configuration tool.

I am not saying its impossible to do a CMake build for stm32 target, I just think this way will be better suited for majority of users.

At some point support for libopencm3 can be added but it will surely support less targets.

Maybe the instructions are not clear, but zenoh-pico could also be located outside your project folder. What is required is adding one source and one include path to the project configuration together with other defines. The exclusion pattern is used because it requires less operations from user than adding each folder in src/ individually. If all the system.c and network.c files from every platform would have #ifdef guards, then this exclusion pattern would not be needed.

@sashacmc sashacmc merged commit 78f4773 into eclipse-zenoh:main Jun 6, 2025
73 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new feature Something new is needed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants