From 8805aa764d92edfa940978ae32c05f5a10d80fc4 Mon Sep 17 00:00:00 2001 From: Michael Sauer Date: Sat, 12 Sep 2026 00:59:40 +0200 Subject: [PATCH] =?UTF-8?q?fix:=20H=C3=A4rtungen=20aus=20dem=20Plan-2-Revi?= =?UTF-8?q?ew?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - wm_internal_fls.c: TLS_DBGPRT_ERR ist in diesem Build wegkompiliert, daher printf() für die fehlgeschlagene s_fls2_lock-Anlage; PATCHES.md korrigiert. - nl_wifi_w600.c: der SoftAP-Backstop (Zerstören eines unerwarteten SDK-Softap) lief bisher innerhalb von lock()/unlock() - tls_wifi_softap_destroy() und clear_softap_param() greifen aber auf SDK-Zustand zu, nicht auf s_sm/die Credential-Puffer, die s_lock eigentlich schützt. Jetzt wird im Lock nur noch ein lokales Flag gesetzt, die SDK-Aufrufe laufen danach. - clear_softap_param() liest TLS_PARAM_ID_WPROTOCOL jetzt aus SRAM (FALSE) statt Flash (TRUE) - das Schreiben bleibt weiterhin geflasht. - app_main.c füttert den Watchdog erst, wenn Licht-, Netz- und HTTP-Task jeweils seit dem letzten Feed einen Fortschritt gemeldet haben. Dafür bekommen nl_wifi_w600.c (nl_net_beat) und nl_http_server.c (nl_http_beat) je einen volatile Heartbeat-Zähler; der HTTP-Listen-Socket bekommt ein SO_RCVTIMEO von 2 s auf accept(), damit er auch ohne Anfragen tickt. --- sdk/PATCHES.md | 6 ++--- .../Drivers/internalflash/wm_internal_fls.c | 2 +- src/app_main.c | 26 ++++++++++++++++--- src/net/nl_http_server.c | 14 ++++++++++ src/net/nl_http_server.h | 7 +++++ src/net/nl_wifi.h | 4 +++ src/net/nl_wifi_w600.c | 21 ++++++++++----- 7 files changed, 65 insertions(+), 15 deletions(-) diff --git a/sdk/PATCHES.md b/sdk/PATCHES.md index 14027b5..46ff0ea 100644 --- a/sdk/PATCHES.md +++ b/sdk/PATCHES.md @@ -28,6 +28,6 @@ Lib/GNU, Bin/secboot.img, Tools/GNU/link_w600_2m.ld (→ ld/) und wm_tool.c (→ 5. `Platform/Drivers/internalflash/wm_internal_fls.c`: Semaphor `s_fls2_lock` serialisiert den Flash-Pfad oberhalb von 1 MB (Pin-Umschaltung via `tls_spifls_cs/ck/di/do_switch` war ungeschützt). T1 (Plan 2 final review): `tls_fls_init()` ignorierte den Rückgabewert von - `tls_os_sem_create(&s_fls2_lock, 1)` stillschweigend - jetzt eine `TLS_DBGPRT_ERR`-Meldung, - falls das Anlegen fehlschlägt (dann bliebe der Bereich oberhalb von 1 MB unprotokolliert - ungeschützt). + `tls_os_sem_create(&s_fls2_lock, 1)` stillschweigend - jetzt eine `printf()`-Meldung + (`TLS_DBGPRT_ERR` ist in diesem Build wegkompiliert), falls das Anlegen fehlschlägt + (dann bliebe der Bereich oberhalb von 1 MB unprotokolliert ungeschützt). diff --git a/sdk/Platform/Drivers/internalflash/wm_internal_fls.c b/sdk/Platform/Drivers/internalflash/wm_internal_fls.c index d3d8e9d..8b6f5c8 100644 --- a/sdk/Platform/Drivers/internalflash/wm_internal_fls.c +++ b/sdk/Platform/Drivers/internalflash/wm_internal_fls.c @@ -1174,7 +1174,7 @@ int tls_fls_init(void) return TLS_FLS_STATUS_ENOMEM; } if (tls_os_sem_create(&s_fls2_lock, 1) != TLS_OS_SUCCESS) - TLS_DBGPRT_ERR("create semaphore @s_fls2_lock fail! flash access above 1 MB is unprotected!\n"); + printf("wm_internal_fls: s_fls2_lock create failed\r\n"); inside_fls = fls; diff --git a/src/app_main.c b/src/app_main.c index 93db9e6..82bee8b 100644 --- a/src/app_main.c +++ b/src/app_main.c @@ -148,8 +148,9 @@ static void app_task(void *arg) nl_engine_cfg_t cfg; nl_light_state_t last, initial, pending; bool have_last, counter_cleared = false; + bool light_alive = false, net_alive = false, http_alive = false; uint8_t boots; - uint32_t boot_ms, beat_seen; + uint32_t boot_ms, light_seen, net_seen, http_seen; (void)arg; nl_flash_init(); @@ -200,7 +201,9 @@ static void app_task(void *arg) nl_http_server_start(nl_app_api_ctx()); boot_ms = nl_app_now_ms(); - beat_seen = s_light_beat; + light_seen = s_light_beat; + net_seen = nl_net_beat; + http_seen = nl_http_beat; for (;;) { uint32_t now; bool due; @@ -232,8 +235,23 @@ static void app_task(void *arg) } } - if (s_light_beat != beat_seen) { - beat_seen = s_light_beat; + /* Feed the watchdog only once the light, network and HTTP tasks have each made + * progress since the last feed - a single stuck task (e.g. nl_app_lock() held + * forever) must still starve the watchdog, not just the light task. */ + if (s_light_beat != light_seen) { + light_seen = s_light_beat; + light_alive = true; + } + if (nl_net_beat != net_seen) { + net_seen = nl_net_beat; + net_alive = true; + } + if (nl_http_beat != http_seen) { + http_seen = nl_http_beat; + http_alive = true; + } + if (light_alive && net_alive && http_alive) { + light_alive = net_alive = http_alive = false; tls_watchdog_clr(); } } diff --git a/src/net/nl_http_server.c b/src/net/nl_http_server.c index 0e89f45..89e7cf0 100644 --- a/src/net/nl_http_server.c +++ b/src/net/nl_http_server.c @@ -15,6 +15,10 @@ static char s_out[NL_API_BODY_MAX + 160]; static nl_api_response_t s_resp; static const nl_api_ctx_t *s_ctx; +volatile uint32_t nl_http_beat; + +#define NL_HTTP_ACCEPT_TIMEOUT_S 2 + static void send_all(int fd, const char *p, size_t n) { while (n) { @@ -95,17 +99,27 @@ static void server_task(void *arg) tls_os_time_delay(HZ); continue; } + { + /* Without traffic accept() would otherwise block forever, so this timeout is + * what keeps nl_http_beat ticking (and the watchdog fed) while idle. */ + struct timeval tv; + tv.tv_sec = NL_HTTP_ACCEPT_TIMEOUT_S; + tv.tv_usec = 0; + setsockopt(ls, SOL_SOCKET, SO_RCVTIMEO, &tv, sizeof tv); + } for (;;) { struct sockaddr_in peer; socklen_t plen = sizeof peer; int fd = accept(ls, (struct sockaddr *)&peer, &plen); if (fd < 0) { + nl_http_beat++; /* accept() timeout (or a transient error): task is still alive */ tls_os_time_delay(HZ / 10); continue; } s_resp.after = NL_API_AFTER_NONE; serve(fd); closesocket(fd); + nl_http_beat++; if (s_resp.after == NL_API_AFTER_REBOOT) { tls_os_time_delay(HZ / 2); /* let the TCP stack flush the response */ nl_app_reboot(); diff --git a/src/net/nl_http_server.h b/src/net/nl_http_server.h index 326e3dc..32b5f84 100644 --- a/src/net/nl_http_server.h +++ b/src/net/nl_http_server.h @@ -1,10 +1,17 @@ #ifndef NL_HTTP_SERVER_H #define NL_HTTP_SERVER_H +#include + #include "net/nl_api.h" #define NL_HTTP_PORT 3333 +/* Incremented once per accept() timeout and once per handled connection; app_main's + * watchdog feed checks it to make sure the HTTP task is still alive, not just the light + * task. The listen socket's 2 s accept timeout keeps this ticking even without requests. */ +extern volatile uint32_t nl_http_beat; + /* Serves one connection at a time (listen backlog 4, 5 s receive timeout). */ void nl_http_server_start(const nl_api_ctx_t *ctx); diff --git a/src/net/nl_wifi.h b/src/net/nl_wifi.h index e64a30d..6b3a11e 100644 --- a/src/net/nl_wifi.h +++ b/src/net/nl_wifi.h @@ -15,6 +15,10 @@ typedef struct { const char *error; /* result of the last onboarding test or NULL */ } nl_wifi_status_t; +/* Incremented once per net_task loop iteration; app_main's watchdog feed checks it to make + * sure the network task is still alive, not just the light task. */ +extern volatile uint32_t nl_net_beat; + /* Starts the network task. An empty ssid starts the onboarding AP. Strings are copied. */ void nl_wifi_start(const char *ssid, const char *pass, const char *ap_pass); /* Starts an onboarding test with new credentials (from the HTTP task). 0 = queued. */ diff --git a/src/net/nl_wifi_w600.c b/src/net/nl_wifi_w600.c index 03fb0f2..4ccf356 100644 --- a/src/net/nl_wifi_w600.c +++ b/src/net/nl_wifi_w600.c @@ -31,6 +31,8 @@ static char s_ap_pass[NL_PASS_LEN]; static bool s_ap_create_pending; /* M3: tls_wifi_softap_create() failed, retry it */ static uint32_t s_ap_create_retry_at; +volatile uint32_t nl_net_beat; + static void lock(void) { tls_os_mutex_acquire(s_lock, 0); } static void unlock(void) { tls_os_mutex_release(s_lock); } @@ -58,7 +60,7 @@ static void clear_softap_param(void) { u8 proto = 0; - tls_param_get(TLS_PARAM_ID_WPROTOCOL, &proto, TRUE); + tls_param_get(TLS_PARAM_ID_WPROTOCOL, &proto, FALSE); if (proto & IEEE80211_MODE_AP) { proto &= (u8)~IEEE80211_MODE_AP; tls_param_set(TLS_PARAM_ID_WPROTOCOL, &proto, TRUE); @@ -163,7 +165,7 @@ static void net_task(void *arg) void *msg = NULL; nl_wifi_actions_t ev, tick; uint32_t m = 0, now; - bool got_ip_event = false, connected_now, ap_wanted; + bool got_ip_event = false, connected_now, ap_wanted, unexpected_softap; memset(&ev, 0, sizeof ev); if (tls_os_queue_receive(s_queue, &msg, 0, HZ / 10) == TLS_OS_SUCCESS) @@ -185,15 +187,19 @@ static void net_task(void *arg) /* I1: a late/stray IP-up must not announce mDNS unless the SM actually settled on STA_CONNECTED. */ connected_now = s_sm.state == NL_WIFI_ST_STA_CONNECTED; ap_wanted = s_sm.ap_up; - if (m == (MSG_NETIF | NETIF_WIFI_SOFTAP_SUCCESS) && !ap_wanted) { - /* C2 backstop: the SDK brought its own softap up behind our back (e.g. the - * clear in nl_wifi_start raced a pending sys_net_status_changed()) while our - * state machine does not want an AP. Tear it down and clear the param again. */ + /* C2 backstop: the SDK brought its own softap up behind our back (e.g. the clear in + * nl_wifi_start raced a pending sys_net_status_changed()) while our state machine + * does not want an AP. Only latch the fact here; tls_wifi_softap_destroy() and + * clear_softap_param() touch the SDK's own state and must not run while s_lock + * (which only guards s_sm and the credential buffers) is held. */ + unexpected_softap = m == (MSG_NETIF | NETIF_WIFI_SOFTAP_SUCCESS) && !ap_wanted; + unlock(); + + if (unexpected_softap) { printf("wifi: unexpected SDK softap while AP should be down, destroying it\r\n"); tls_wifi_softap_destroy(); clear_softap_param(); } - unlock(); if (m & MSG_NETIF) printf("wifi: netif event %u\r\n", (unsigned)(m & 0xFFu)); @@ -207,6 +213,7 @@ static void net_task(void *arg) printf("wifi: retrying AP create\r\n"); start_ap(); } + nl_net_beat++; } }