modem: pipe: Add explicit timeout to sync APIs #74325
Merged
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
The modem pipe APIs include synchronous calls to open/close, which internally use a fixed timeout of 10 seconds. The timeout should be configurable through the APIs, anywhere from K_NO_WAIT to K_FOREVER. This commit adds timeout parameters to the open/close APIs, and updates in-tree usage of the open/close APIs to explicitly provide the previously implicit timeout of 10 seconds. Signed-off-by: Bjarki Arge Andreasen <bjarki@arge-andreasen.me>
rerickson1
approved these changes
Jun 14, 2024
| @@ -188,21 +188,21 @@ static int gnss_luatos_air530z_init(const struct device *dev) | |||
|
|
|||
| luatos_air530z_init_dynamic_script(dev); | |||
|
|
|||
| ret = modem_pipe_open(data->uart_pipe); | |||
| ret = modem_pipe_open(data->uart_pipe, K_SECONDS(10)); | |||
There was a problem hiding this comment.
should the number be a kconfig option? (could be on a followup)
There was a problem hiding this comment.
Agreed could be nice to have a global default defined by a Kconfig option.
fabiobaltieri
approved these changes
Jun 14, 2024
|
This PR does break the modem pipe API, so I would like to not include it in 3.7.0 to emphasize modem subsystem stability for LTS for downstream users :) |
tomi-font
approved these changes
Jun 25, 2024
| @@ -87,6 +87,7 @@ void modem_pipe_init(struct modem_pipe *pipe, void *data, struct modem_pipe_api | |||
| * @brief Open pipe | |||
| * | |||
| * @param pipe Pipe instance | |||
| * @param timeout Timeout waiting for pipe to open | |||
There was a problem hiding this comment.
Suggested change
| * @param timeout Timeout waiting for pipe to open | |
| * @param timeout Time to wait for pipe to open |
| @@ -188,21 +188,21 @@ static int gnss_luatos_air530z_init(const struct device *dev) | |||
|
|
|||
| luatos_air530z_init_dynamic_script(dev); | |||
|
|
|||
| ret = modem_pipe_open(data->uart_pipe); | |||
| ret = modem_pipe_open(data->uart_pipe, K_SECONDS(10)); | |||
There was a problem hiding this comment.
Agreed could be nice to have a global default defined by a Kconfig option.
tomi-font
approved these changes
Jun 25, 2024
Sure :) I will add it after the release notes for 4.0.0 show up :) |
@bjarki-andreasen they are there now |
|
API calls missed are failing in CI and fixed in #76449 |
tomi-font
added a commit
to tomi-font/zephyr
that referenced
this pull request
Jul 30, 2024
Add the missing timeout parameter to `modem_pipe_open()` and `modem_pipe_close()` calls. 10 seconds is the default value used in the Zephyr tree. Fixes a regression introduced in zephyrproject-rtos#74325. Signed-off-by: Tomi Fontanilles <tomi.fontanilles@nordicsemi.no>
|
Also this is missing the migration guide entry we were talking about. |
tomi-font
added a commit
to tomi-font/zephyr
that referenced
this pull request
Jul 30, 2024
Add the missing timeout parameter to `modem_pipe_open()` and `modem_pipe_close()` calls. 10 seconds is the default value used in the Zephyr tree. Fixes a regression introduced in zephyrproject-rtos#74325. Signed-off-by: Tomi Fontanilles <tomi.fontanilles@nordicsemi.no>
aescolar
pushed a commit
that referenced
this pull request
Jul 30, 2024
Add the missing timeout parameter to `modem_pipe_open()` and `modem_pipe_close()` calls. 10 seconds is the default value used in the Zephyr tree. Fixes a regression introduced in #74325. Signed-off-by: Tomi Fontanilles <tomi.fontanilles@nordicsemi.no>
coreboot-org-bot
pushed a commit
to coreboot/zephyr-cros
that referenced
this pull request
Jul 30, 2024
Add the missing timeout parameter to `modem_pipe_open()` and `modem_pipe_close()` calls. 10 seconds is the default value used in the Zephyr tree. Fixes a regression introduced in zephyrproject-rtos/zephyr#74325. (cherry picked from commit 48d69b4) Original-Signed-off-by: Tomi Fontanilles <tomi.fontanilles@nordicsemi.no> GitOrigin-RevId: 48d69b4 Change-Id: I3c3938e8f1baa75e13b838b0a5a9b7c18f69ac29 Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/third_party/zephyr/+/5748700 Reviewed-by: Fabio Baltieri <fabiobaltieri@google.com> Commit-Queue: Fabio Baltieri <fabiobaltieri@google.com> Tested-by: Fabio Baltieri <fabiobaltieri@google.com> Tested-by: ChromeOS Prod (Robot) <chromeos-ci-prod@chromeos-bot.iam.gserviceaccount.com>
mariucker
pushed a commit
to mariucker/zephyr
that referenced
this pull request
Dec 12, 2024
Add the missing timeout parameter to `modem_pipe_open()` and `modem_pipe_close()` calls. 10 seconds is the default value used in the Zephyr tree. Fixes a regression introduced in zephyrproject-rtos#74325. Signed-off-by: Tomi Fontanilles <tomi.fontanilles@nordicsemi.no>
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 modem pipe APIs include synchronous calls to open/close, which internally use a fixed timeout of 10 seconds. The timeout should be configurable through the APIs, anywhere from K_NO_WAIT to K_FOREVER.
This commit adds timeout parameters to the open/close APIs, and updates in-tree usage of the open/close APIs to explicitly provide the previously implicit timeout of 10 seconds.
The timeouts can be optimized to be lower in some cases, but initially just keep the 10 second timeouts to not break anything :)