[spinel] create spinel interface based on the radio url protocol #9393
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Size Report of OpenThread
|
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #9393 +/- ##
==========================================
- Coverage 85.81% 85.66% -0.15%
==========================================
Files 559 560 +1
Lines 73991 74037 +46
==========================================
- Hits 63497 63427 -70
- Misses 10494 10610 +116
|
abtink
reviewed
Sep 6, 2023
There was a problem hiding this comment.
Thanks @zhanglongxia. This is a great improvement.
Looks great overall. Some suggestions below:
zhanglongxia
force-pushed
the
spinel/dynamic-spinel-interface
branch
from
81f3752
to
24833c4
Compare
abtink
approved these changes
Sep 22, 2023
bukepo
reviewed
Sep 25, 2023
bukepo
reviewed
Sep 25, 2023
zhanglongxia
force-pushed
the
spinel/dynamic-spinel-interface
branch
from
b30d753
to
96dc31c
Compare
bukepo
reviewed
Oct 9, 2023
The posix platform is able to support the HDLC, SPI and vendor spinel interfaces, but the spinel interface type can't be changed dynamically. It is inconvenient to use different spinel interfaces for Thread stack in Android. This commit enables the posix platform to support the HDLC, SPI and vendor interface at the same time, and the final spinel interface type is determined by the radio url protocol. Some other changes: <1> Not use CRTP style for the radio spinel. <2> Deprecate the OT_POSIX_CONFIG_RCP_BUS and OPENTHREAD_POSIX_CONFIG_RCP_TIME_SYNC_INTERVAL.
zhanglongxia
force-pushed
the
spinel/dynamic-spinel-interface
branch
from
95b1208
to
05a0976
Compare
bukepo
approved these changes
Oct 9, 2023
jwhui
reviewed
Oct 10, 2023
jwhui
approved these changes
Oct 10, 2023
romacdon
reviewed
Oct 13, 2023
| @@ -61,6 +61,7 @@ set(COMMON_INCLUDES | |||
| ) | |||
|
|
|||
| set(COMMON_SOURCES | |||
| radio_spinel.cpp | |||
There was a problem hiding this comment.
@zhanglongxia - I know this PR is already merged but I'm curious why we need this file to be included in NCP/RCP builds? From what I see this file should only need to be included in host side builds. Am I missing something?
There was a problem hiding this comment.
The file radio_spinel.cpp is renamed from the file radio_spinel_impl.hpp, and it was included by both the NCP and RCP builds before this commit is merged. That's the reason why I added the radio_spinel.cpp to the COMMON_SOURCES. Thanks for your notification, I think it is reasonable to remove the file radio_spinel.cpp from the lib openthread-spinel-ncp. I have submitted a commit #9530 to resolve this issue.
There was a problem hiding this comment.
@zhanglongxia thanks for the quick response and for opening the new PR. I did have one comment on the new PR which I added there. Also, you mention that radio_spinel_impl.hpp was included by both the NCP and RCP builds before this commit was merged, however, I don't see that. radio_spinel_impl.hpp is included by radio_spinel.hpp and I only see radio_spinel.hpp being included by the posix platform code.
wgtdkp
pushed a commit
to wgtdkp/openthread
that referenced
this pull request
Nov 6, 2023
…nthread#9393) The posix platform is able to support the HDLC, SPI and vendor spinel interfaces, but the spinel interface type can't be changed dynamically. It is inconvenient to use different spinel interfaces for Thread stack in Android. This commit enables the posix platform to support the HDLC, SPI and vendor interface at the same time, and the final spinel interface type is determined by the radio url protocol. Some other changes: 1. Not use CRTP style for the radio spinel. 2. Deprecate the OT_POSIX_CONFIG_RCP_BUS and OPENTHREAD_POSIX_CONFIG_RCP_TIME_SYNC_INTERVAL. (cherry picked from commit 7568e31) Change-Id: I56da30c92b11620d27c02e6adfda1da5965f23a7
wgtdkp
pushed a commit
to wgtdkp/openthread
that referenced
this pull request
Nov 6, 2023
wgtdkp
pushed a commit
to wgtdkp/openthread
that referenced
this pull request
Jan 16, 2024
…ol (openthread#9393)" into main am: ff54b9b Original change: https://android-review.googlesource.com/c/platform/external/openthread/+/2784091 Change-Id: I2ec69c8ee059237bca26ca1d651e50597c0fc25c Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>
wgtdkp
pushed a commit
to wgtdkp/openthread
that referenced
this pull request
Jan 16, 2024
…ol (openthread#9393)" into main am: ff54b9b am: ca79b7d Original change: https://android-review.googlesource.com/c/platform/external/openthread/+/2784091 Change-Id: I3aa5de055277f4f550186c3e73cafae9dad0b58e Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>
wgtdkp
pushed a commit
to wgtdkp/openthread
that referenced
this pull request
Jan 16, 2024
…ol (openthread#9393)" into main am: ff54b9b am: ca79b7d am: 89c203d Original change: https://android-review.googlesource.com/c/platform/external/openthread/+/2784091 Change-Id: I5380a2822bd57ecaeddeb93beef90985cd2bb043 Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>
wgtdkp
pushed a commit
to wgtdkp/openthread
that referenced
this pull request
Jan 16, 2024
…ol (openthread#9393)" into main am: ff54b9b am: ca79b7d am: 89c203d am: 36fe612 Original change: https://android-review.googlesource.com/c/platform/external/openthread/+/2784091 Change-Id: I25e09ca7721a23cd880173aaa2f7c8b3677b274b Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The posix platform is able to support the HDLC, SPI and vendor spinel interfaces, but the spinel interface type can't be changed dynamically. It is inconvenient to use different spinel interfaces for Thread stack in Android. This commit enables the posix platform to support the HDLC, SPI and vendor interface at the same time, and the final spinel interface type is determined by the radio url protocol.
Some other changes:
<1> Not use CRTP style for the radio spinel.
<2> Deprecate the OT_POSIX_CONFIG_RCP_BUS and OPENTHREAD_POSIX_CONFIG_RCP_TIME_SYNC_INTERVAL.