Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feature request: Option to change the network timeout for Zephyr #86

Closed
chirambaht opened this issue Jan 14, 2025 · 4 comments
Closed
Assignees

Comments

@chirambaht
Copy link

One of the key function used when collecting data from the Memfault servers is prv_poll_socket. In this function, a constant, pre-defined value of 5000ms is used for the network connection timeout. In the ESP-IDF port, I saw that this has already been done for some of the other networking functions using CONFIG_MEMFAULT_HTTP_CLIENT_TIMEOUT_MS. It would be nice to also have this value be defined by a Kconfig in much the same way for the Zephyr port as well.

@chirambaht
Copy link
Author

To add more context to this issue:
I have a device which I am currently trying to update and quite often, the device reports back the following logs:

[00:14:07.288,000] <dbg> mflt: memfault_platform_log: Downloaded 62976/2097152 bytes (3%)
[00:14:16.726,000] <err> mflt: Timeout waiting for socket event(s): event(s)=1, errno=11
[00:14:16.730,000] <err> mflt: Error upgrading firmware, rv=-1
[00:12:49.693,000] <err> net_tcp: conn: 0x3fcb7278, new bytes 2825 during TIME-WAIT state sending reset

I then decided to increase the timeout manually and found that my device could now download the image on its first try. I was simultaneously running a device which took multiple attempts to download the same image (same cohort). The higher timeout downloaded it on its first try while the device with the default timeout eventually downloaded the payload but after multiple re-attempts.

@noahp
Copy link
Contributor

noahp commented Jan 14, 2025

@chirambaht thanks for reporting! great idea, we will add it to the next SDK release. Let me know if there's urgency for a new SDK with the changes, and we can expedite it- otherwise, it's scheduled for sometime next week.

@noahp noahp self-assigned this Jan 16, 2025
@noahp
Copy link
Contributor

noahp commented Jan 30, 2025

Hi @chirambaht - just to confirm, is this change what you were thinking of?

diff --git a/ports/zephyr/Kconfig b/ports/zephyr/Kconfig
index db6b65bd7b..c9c75eb2c6 100644
--- a/ports/zephyr/Kconfig
+++ b/ports/zephyr/Kconfig
@@ -403,6 +403,14 @@ config MEMFAULT_PROJECT_KEY
           Token used to communicate with Memfault. Find it at
           https://mflt.io/project-key
 
+config MEMFAULT_HTTP_CLIENT_TIMEOUT_MS
+        int "The HTTP client timeout in milliseconds"
+        default 5000
+        help
+          The Memfault HTTP client timeout in milliseconds. This is the
+          maximum amount of time the HTTP client will wait for a response from
+          the server.
+
 endif # MEMFAULT_HTTP_ENABLE
 
 config MEMFAULT_ZEPHYR_FOTA_DOWNLOAD_CALLBACK_CUSTOM
diff --git a/ports/zephyr/common/memfault_platform_http.c b/ports/zephyr/common/memfault_platform_http.c
index c368181af3..8b7bae9155 100644
--- a/ports/zephyr/common/memfault_platform_http.c
+++ b/ports/zephyr/common/memfault_platform_http.c
@@ -75,7 +75,7 @@
 #endif /* CONFIG_MEMFAULT_HTTP_USES_MBEDTLS */
 
 // Timeout for the socket file descriptor to become available for I/0
-#define POLL_TIMEOUT_MS 5000
+#define POLL_TIMEOUT_MS CONFIG_MEMFAULT_HTTP_CLIENT_TIMEOUT_MS
 
 // The nRF-Connect SDK has an implementation of this structure
 #if !defined(CONFIG_MEMFAULT_NCS_PROJECT_KEY)

@noahp
Copy link
Contributor

noahp commented Feb 10, 2025

This is fixed in 1.20.0. Let me know if there's any issues!

@noahp noahp closed this as completed Feb 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants