pkg/openthread: fix code style, error handling, and synchronization#19638
Closed
maribu wants to merge 2 commits intoRIOT-OS:masterfrom
Closed
pkg/openthread: fix code style, error handling, and synchronization#19638maribu wants to merge 2 commits intoRIOT-OS:masterfrom
maribu wants to merge 2 commits intoRIOT-OS:masterfrom
Conversation
Various code style fix that shouldn't change the generated machine code. In addition, many internal functions provided symbols with no reason to do so. Those internal functions and variables are now declared as `static`.
Previously, there was zero error handling in the ot_command_t commands provided in pkg/openthread/contrib/platform_functions_wrapper.c - ignoring the return value even of functions that are marked with `OT_MUST_USE_RESULT`. In addition, `ot_call_command()` never synchronized with the thread executing the command before return the value. Given that there was no error handling anyways, this likely was never noticed. But now that the commands actually report an error, it is of utmost importance to actually wait for the command to complete before checking the commands return value.
maribu
added
Type: bug
4 tasks
benpicco
added
the
CI: ready for build
benpicco
changed the title
aabadie
changed the title
Member
|
FYI, those "ot_command" functions were deprecated some years ago. I'm surprised to see they still exist 😮 Since then OpenThread runs on an event queue, which automatically allows to call pkg API functions without synchronization issues (since all OpenThread API calls use tasklets internally to process function calls). See e.g https://github.com/RIOT-OS/RIOT/blob/master/examples/openthread/main.c#L31 |
Member
Author
|
Not needed anymore with #19685 merged |
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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.
Contribution description
pkg/openthread/contrib/platform_functions_wrapper.cot_call_command()wait for the command to actually completeot_call_command()would return before the command is executed if called from a higher priority thread than OT is running in or when the OT thread blocks. The latter is pretty likelyot_call_command()actually can be non-zero, it is important to wait for the command to complete before checking its error statusTesting procedure
Issues/PRs references
Split out of #19634