Fix for situation where smp shell transport can respond on a different device than it receives from #80926
Conversation
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
zakport
force-pushed
the
smp_respond_on_shell_device
branch
from
281d7a9 to
83e5228
Compare
nordicjm
requested changes
Nov 5, 2024
Contributor
nordicjm
left a comment
nordicjm
left a comment
There was a problem hiding this comment.
Whilst this is fixing an issue, this function is used to transmit 2 or 3 bytes per call, you are adding a huge overhead by doing this by calling an extra function, and adding a bug because you could switch shell backends part the way through a transfer. So this needs to be fixed properly by getting the shell once for transmitting a whole packet
Responses are currently set to the shell device that was configured in the device tree. The shell_uart driver allows for changing it's device during runtime which leads to a situation where we recieve packets on one device and respond on another. This patch causes smp_shell_tx_raw to use the shell_uart device Signed-off-by: Zak Portnoy <zakportnoy@gmail.com>
zakport
force-pushed
the
smp_respond_on_shell_device
branch
from
83e5228 to
97a6043
Compare
nordicjm
approved these changes
Nov 6, 2024
Contributor
|
If you want this fix in zephyr 4.0, it will need a github issue that this marks as fixing, if so could you open one? |
de-nordic
approved these changes
Nov 7, 2024
dkalowsk
approved these changes
Nov 7, 2024
Contributor
|
@zakport , can you add a Zephyr issue with details about the problem seen and link it to this PR. |
Contributor
Author
|
Hey, |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
7 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.
The smp shell transport is configured to work over uart shell.
The uart shell's device can be reconfigured at runtime.
The Smp shell driver currently responds on the device defined as shell in the device tree while receiving data from the uart shell whose device can be changed during runtime.
A similar fix from 2020:
#27871
The issue came back here when the shell structures were changed.
6d2e3b5
This pull request takes the device from the uart_shell structures.
Fixes: #81144