OpenThread HDLC RCP communication support #75986

xavraz · 2024-07-17T08:45:37Z

This PR adds support to an RCP design, the core of OpenThread lives on the host processor connected to an RCP radio controller over a HDLC interface.

OpenThread HDLC RCP communication added with its hdlc_api interface APIs and a NXP driver
Add of spinel hdlc rcp interface for an OpenThread rcp host
Adapt openthread.c file to support RCP interface

Tested on a board using NXP's RW612 component.

github-actions · 2024-07-17T08:46:15Z

Hello @xavraz, and thank you very much for your first pull request to the Zephyr project!
Our Continuous Integration pipeline will execute a series of checks on your Pull Request commit messages and code, and you are expected to address any failures by updating the PR. Please take a look at our commit message guidelines to find out how to format your commit messages, and at our contribution workflow to understand how to update your Pull Request. If you haven't already, please make sure to review the project's Contributor Expectations and update (by amending and force-pushing the commits) your pull request if necessary.
If you are stuck or need help please join us on Discord and ask your question there. Additionally, you can escalate the review when applicable. 😊

canisLupus1313 · 2024-07-17T09:11:28Z

@xavraz thanks for your contribution.
It seems that You have included some of the Openthread files in the Zephyr code. I would say would be better to include form openthread module.
More over I don't think that C++ code is accepted in zephyr core.

Also, can you explain what is the purpose of this PR? The RCP architecture is supported in zephyr.

xavraz · 2024-07-18T08:53:25Z

@canisLupus1313, thank you for your comment.

OT-RCP Architecture

This PR corresponds to an ot-rcp host support. The existing support in Zephyr is for an ot-rcp device, as follows :
https://developer.nordicsemi.com/nRF_Connect_SDK/doc/1.7.1/nrf/ug_thread_architectures.html#thread-architectures-designs-cp-rcp.
This means Zephyr supports only a radio coprocessor associated with a host processor.

Zephyr code

Concerning your question "included some of the Openthread files in the Zephyr code."
openthread\platform\radio_spinel.cpp and spinel_hdlc.cpp files are interfaces :

radio_spinel.cpp
Interface between net\l2\openthread HOST layer and the spinel driver from the openthread library.
zephyr\subsys\net\l2\openthread\openthread.c (not in modules\lib\openthread)
↓
zephyr\modules\openthread\platform\radio_spinel.cpp
↓
modules\lib\openthread\src\lib\spinel\spinel_driver.cpp
spinel_hdlc.cpp
Interface between the spinel driver from the openthread library and the HDLC interface.
modules\lib\openthread\src\lib\spinel\spinel_driver.cpp
↓
zephyr\modules\openthread\platform\spinel_hdlc.cpp
↓
zephyr\drivers\hdlc_rcp_if\hdlc_rcp_if_nxp.c

C++ code in Zephyr

There are already few files in zephyr directory as follows for example

rlubos

THe commit message titles need to follow https://docs.zephyrproject.org/latest/contribute/guidelines.html#commit-message-guidelines

modules/openthread/platform/radio_spinel.cpp

modules/openthread/platform/platform_spinel.c

modules/openthread/platform/platform.h

canisLupus1313 · 2024-07-18T10:00:33Z

drivers/hdlc_rcp_if/Kconfig.nxp

+# Openthead HDLC RCP communication Interface configuration options
+# Copyright 2024 NXP
+# All rights reserved.
+# SPDX-License-Identifier: BSD-3-Clause


BSD license should be changed to Apache-2.0

I will change the licences following all your comments.

Note that you can only change the license if you are the author. If you took the code from somewhere else, then you need to use the original license.

canisLupus1313 commented 2 days ago
@xavraz thanks for your contribution.
It seems that You have included some of the Openthread files in the Zephyr code. I would say would be better to include form openthread module.

Concerning the OpenThread files included in the Zephyr code, this has been done to ensure the compatibility if the OpenThread library version is modified. This permits to go back to an older OpenThread version. Otherwise, if the source code is set in modules\lib\openthread\src\xxx, then, it is not possible to go back to an older OpenThread version.

canisLupus1313 · 2024-07-18T10:17:13Z

modules/openthread/CMakeLists.txt

+    ${ZEPHYR_BASE}/arch/arm/include
+    ${ZEPHYR_BASE}/../modules/lib/openthread/src
+    ${ZEPHYR_BASE}/../modules/lib/openthread/src/core
+    ${ZEPHYR_BASE}/../modules/lib/openthread/third_party/mbedtls/repo/include


Zephyr is using it's own mbed TLS: https://github.com/zephyrproject-rtos/mbedtls not exactly same as the on in OT.

drivers/hdlc_rcp_if/hdlc_rcp_if_nxp.c

canisLupus1313 · 2024-07-18T10:28:21Z

modules/openthread/platform/platform.h

+#define OT_PLAT_DBG_LEVEL_NONE 0
+#define OT_PLAT_DBG_LEVEL_ERR 1
+#define OT_PLAT_DBG_LEVEL_WARNING 2
+#define OT_PLAT_DBG_LEVEL_INFO 3
+#define OT_PLAT_DBG_LEVEL_DEBUG 4


Why are you defining those?
It should be possible to reuse logging from https://docs.zephyrproject.org/latest/services/logging/index.html

modules/openthread/platform/spinel_hdlc.hpp

canisLupus1313 · 2024-07-18T10:40:23Z

@canisLupus1313, thank you for your comment.

OT-RCP Architecture

This PR corresponds to an ot-rcp host support. The existing support in Zephyr is for an ot-rcp device, as follows : https://developer.nordicsemi.com/nRF_Connect_SDK/doc/1.7.1/nrf/ug_thread_architectures.html#thread-architectures-designs-cp-rcp. This means Zephyr supports only a radio coprocessor associated with a host processor.

Zephyr code

Concerning your question "included some of the Openthread files in the Zephyr code." openthread\platform\radio_spinel.cpp and spinel_hdlc.cpp files are interfaces :

radio_spinel.cpp
Interface between net\l2\openthread HOST layer and the spinel driver from the openthread library.
zephyr\subsys\net\l2\openthread\openthread.c (not in modules\lib\openthread)
↓
zephyr\modules\openthread\platform\radio_spinel.cpp
↓
modules\lib\openthread\src\lib\spinel\spinel_driver.cpp

spinel_hdlc.cpp
Interface between the spinel driver from the openthread library and the HDLC interface.
modules\lib\openthread\src\lib\spinel\spinel_driver.cpp
↓
zephyr\modules\openthread\platform\spinel_hdlc.cpp
↓
zephyr\drivers\hdlc_rcp_if\hdlc_rcp_if_nxp.c

C++ code in Zephyr

There are already few files in zephyr directory as follows for example

• zephyr\modules\thrift\src\thrift\transport\TSSLSocket.cpp
• zephyr\modules\thrift\src\thrift\server\TServerFramework.cpp

@xavraz Thanks for clarification.
The idea of what You are trying to do seems great. I'm wondering if the parts of code corresponding to the Spinel interface wouldn't fit more into OT repository than zephyr's. Then You chould just provide the Platform API for sending the data. and altering integration in openthread.c as you have already done. Have You considered that?

drivers/hdlc_rcp_if/Kconfig

drivers/hdlc_rcp_if/Kconfig.nxp

modules/openthread/platform/spinel_hdlc.cpp

modules/openthread/platform/platform_spinel.c

modules/openthread/platform/spinel_hdlc.hpp

dleach02 · 2024-11-25T19:19:06Z

@rlubos @JA-NXP @George-Stefan I've updated the west.yml to reflect the HAL update. Need to get +1 from you reapplied so we can get this merged. Will need an override from @nashif to merge.

rlubos

There's still some compliance report about trailing whitespaces though.

Update the licensing list to include the BSD-3-clause OpenThread files. Approved by TSC zephyrproject-rtos#79882 Signed-off-by: David Leach <[email protected]>

github-actions · 2024-11-27T15:40:09Z

Hi @xavraz!
Congratulations on getting your very first Zephyr pull request merged 🎉🥳. This is a fantastic achievement, and we're thrilled to have you as part of our community!

To celebrate this milestone and showcase your contribution, we'd love to award you the Zephyr Technical Contributor badge. If you're interested, please claim your badge by filling out this form: Claim Your Zephyr Badge.

Thank you for your valuable input, and we look forward to seeing more of your contributions in the future! 🪁

sylvioalves · 2024-11-28T17:26:21Z

@xavraz @rlubos Sounds this PR is causing build issues. I can't tell for sure. Would you take a look?
https://github.com/zephyrproject-rtos/zephyr/actions/runs/12068796805/job/33660739639?pr=80785
Edit: all good, sorry for the noise.

xavraz requested review from mmahadevan108 and dleach02 as code owners July 17, 2024 08:45

zephyrbot added area: OpenThread platform: NXP Drivers NXP Semiconductors, drivers area: Networking labels Jul 17, 2024

zephyrbot requested review from canisLupus1313, danieldegrasse, dbaluta, decsny, edmont, jukkar, maciejbaczmanski, manuargue, mariuszpos, pdgendt, rlubos, ssharks and tbursztyka July 17, 2024 08:46

zephyrbot assigned rlubos Jul 17, 2024

rlubos reviewed Jul 18, 2024

View reviewed changes

modules/openthread/platform/radio_spinel.cpp Show resolved Hide resolved

modules/openthread/platform/platform_spinel.c Outdated Show resolved Hide resolved

modules/openthread/platform/platform.h Outdated Show resolved Hide resolved

canisLupus1313 reviewed Jul 18, 2024

View reviewed changes

drivers/hdlc_rcp_if/hdlc_rcp_if_nxp.c Show resolved Hide resolved

canisLupus1313 reviewed Jul 18, 2024

View reviewed changes

modules/openthread/platform/spinel_hdlc.hpp Outdated Show resolved Hide resolved

rlubos reviewed Jul 18, 2024

View reviewed changes

zephyrbot removed the DNM This PR should not be merged (Do Not Merge) label Nov 25, 2024

George-Stefan self-requested a review November 25, 2024 19:22

George-Stefan approved these changes Nov 25, 2024

View reviewed changes

dleach02 force-pushed the feature_openthread_hdlc_rcp branch 3 times, most recently from f3aee8d to 79bbca9 Compare November 25, 2024 22:04

dleach02 requested review from JA-NXP, mmahadevan108, rlubos and dleach02 November 25, 2024 22:25

dleach02 previously approved these changes Nov 25, 2024

View reviewed changes

JA-NXP approved these changes Nov 26, 2024

View reviewed changes

rlubos previously approved these changes Nov 26, 2024

View reviewed changes

doc: LICENSING: Add BSD-3-clause for OpenThread files

05aaaa5

Update the licensing list to include the BSD-3-clause OpenThread files. Approved by TSC zephyrproject-rtos#79882 Signed-off-by: David Leach <[email protected]>

dleach02 dismissed stale reviews from rlubos and themself via 05aaaa5 November 26, 2024 16:42

dleach02 force-pushed the feature_openthread_hdlc_rcp branch from 79bbca9 to 05aaaa5 Compare November 26, 2024 16:42

dleach02 requested review from rlubos, dleach02, George-Stefan and JA-NXP November 26, 2024 16:42

dleach02 approved these changes Nov 26, 2024

View reviewed changes

rlubos approved these changes Nov 26, 2024

View reviewed changes

George-Stefan approved these changes Nov 26, 2024

View reviewed changes

JA-NXP approved these changes Nov 26, 2024

View reviewed changes

nashif merged commit a690d6c into zephyrproject-rtos:main Nov 27, 2024
27 of 28 checks passed

lmajewski mentioned this pull request Dec 12, 2024

net: ot: OpenThread HOST to RCP communication via UART #82926

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

OpenThread HDLC RCP communication support #75986

OpenThread HDLC RCP communication support #75986

xavraz commented Jul 17, 2024

github-actions bot commented Jul 17, 2024

canisLupus1313 commented Jul 17, 2024

xavraz commented Jul 18, 2024

rlubos left a comment

canisLupus1313 Jul 18, 2024

xavraz Jul 18, 2024 •

edited

Loading

jukkar Jul 19, 2024

xavraz Jul 19, 2024

JA-NXP Sep 6, 2024

canisLupus1313 Jul 18, 2024

JA-NXP Sep 6, 2024

canisLupus1313 Jul 18, 2024

JA-NXP Sep 6, 2024

canisLupus1313 commented Jul 18, 2024

OT-RCP Architecture

Zephyr code

C++ code in Zephyr

dleach02 commented Nov 25, 2024

rlubos left a comment

github-actions bot commented Nov 27, 2024

sylvioalves commented Nov 28, 2024 •

edited

Loading

OpenThread HDLC RCP communication support #75986

OpenThread HDLC RCP communication support #75986

Conversation

xavraz commented Jul 17, 2024

github-actions bot commented Jul 17, 2024

canisLupus1313 commented Jul 17, 2024

xavraz commented Jul 18, 2024

OT-RCP Architecture

Zephyr code

C++ code in Zephyr

rlubos left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

xavraz Jul 18, 2024 • 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

canisLupus1313 commented Jul 18, 2024

OT-RCP Architecture

Zephyr code

C++ code in Zephyr

dleach02 commented Nov 25, 2024

rlubos left a comment

Choose a reason for hiding this comment

github-actions bot commented Nov 27, 2024

sylvioalves commented Nov 28, 2024 • edited Loading

xavraz Jul 18, 2024 •

edited

Loading

sylvioalves commented Nov 28, 2024 •

edited

Loading