shell: rtt: Add detection of host presence #68941
Conversation
nordic-krch
force-pushed
the
fix_shell_rtt_block
branch
from
9fcd05e
to
031cd3c
Compare
|
Does this also fix #68709? |
nordic-krch
force-pushed
the
fix_shell_rtt_block
branch
from
031cd3c
to
05b5a67
Compare
@nordic-krch Ping on the above question? |
I created #68709. I copied the changes from @nordic-krch into my local Zephyr v3.4.0. These changes still do not address my issue.
|
Much appreciated, thank you for testing. |
I don't want to clutter this issue up, but if it helps, I think my issue still brings up a valid point about this else statement. If the host was present but became disconnected here, execution would be stuck forever. I would suggest to review the logic below in the zephyr/subsys/logging/backends/log_backend_rtt.c Lines 174 to 184 in 77244aa |
henrikbrixandersen
added
the
backport v3.6-branch
nordic-krch
force-pushed
the
fix_shell_rtt_block
branch
from
05b5a67
to
3bb9904
Compare
|
I took the solution from logging. @JeremiahGillis can you verify if that works for you? |
Thank you. Yes, this works for my issue. I tested with and without the host present. It also works when the host is connected or disconnected during operation. |
|
@henrikbrixandersen @jakub-uC can you take a look? @JeremiahGillis confirmed that it solved his issue. |
subsys/shell/backends/shell_rtt.c
Outdated
| static int write(const struct shell_transport *transport, | ||
| const void *data, size_t length, size_t *cnt) | ||
| { | ||
| struct shell_rtt *sh_rtt = (struct shell_rtt *)transport->ctx; | ||
| const uint8_t *data8 = (const uint8_t *)data; | ||
| static bool host_present; |
There was a problem hiding this comment.
I am a bit concerned about name shadowing. You have already created host_present as a global variable. Was this intentional?"
There was a problem hiding this comment.
good finding. That was not intentional. Removed.
If host is not reading RTT data (because there is no PC connection or RTT reading application is not running on the host), thread will stuck continuously trying to write to RTT. All threads with equal or lower priority are blocked then. Adding detection of that case and if host is not reading data for configurable period then data is dropped until host accepts new data. Similar solution is using in RTT logging backend. Signed-off-by: Krzysztof Chruściński <krzysztof.chruscinski@nordicsemi.no>
nordic-krch
force-pushed
the
fix_shell_rtt_block
branch
from
3bb9904
to
fb17d0e
Compare
| @@ -25,7 +31,8 @@ SHELL_DEFINE(shell_rtt, CONFIG_SHELL_PROMPT_RTT, &shell_transport_rtt, | |||
|
|
|||
| LOG_MODULE_REGISTER(shell_rtt, CONFIG_SHELL_RTT_LOG_LEVEL); | |||
|
|
|||
| static bool rtt_blocking; | |||
| static bool panic_mode; | |||
There was a problem hiding this comment.
This variable is never set?
There was a problem hiding this comment.
@nordic-krch @fabiobaltieri : It seems that api for settting panic mode is missing i shell_transport_api. In NCS setup this causes an attempt to lock mutex ISR context, when entering from log_panic.
There was a problem hiding this comment.
@aescov could you open either a PR with the fix or an issue so we can assign this?
If host is not reading RTT data (because there is no PC connection or RTT reading application is not running on the host), thread will stuck continuously trying to write to RTT. All threads with equal or lower priority are blocked then. Adding detection of that case and if host is not reading data for configurable period then data is dropped until host accepts new data.
Fixes #49390.