Fix esp32 event callback serialisation (#2913)

This PR addresses the issue raised in #2912 by pushing events into the tcpip thread for handling. Network applications therefore 'live' in the tcpip task and the **Sming** task is created only for non-networked applications. Sming uses a separate event callback queue instead of the IDF one. - Use custom queue for sming messaging - Serialise idf messages to sming queue in networking code - Use idle task watchdogs instead of custom one - Push task callbacks to tcpip thread in network builds - Fix stack sizes. Network builds just use default size for event task stack - Use heap for initial partition table load. Not enough room on event task stack for this. - Add a semaphore to `smg_uart_write` so debug output doesn't get garbled between tasks (e.g. wifi).
SmingHub · Nov 25, 2024 · 502e304 · 502e304
1 parent 6d94935
commit 502e304
Show file tree

Hide file tree

Showing 25 changed files with 497 additions and 538 deletions.
diff --git a/Sming/Arch/Esp32/Components/driver/uart.cpp b/Sming/Arch/Esp32/Components/driver/uart.cpp
@@ -191,6 +191,28 @@ smg_uart_t* get_standard_uart(smg_uart_t* uart)
 	return is_standard_uart(uart) ? uart : nullptr;
 }
 
+class Lock
+{
+public:
+	Lock()
+	{
+		if(!mutex) {
+			mutex = xSemaphoreCreateMutex();
+		}
+		xSemaphoreTake(mutex, portMAX_DELAY);
+	}
+
+	~Lock()
+	{
+		xSemaphoreGive(mutex);
+	}
+
+private:
+	static SemaphoreHandle_t mutex;
+};
+
+SemaphoreHandle_t Lock::mutex;
+
 #if UART_ID_SERIAL_USB_JTAG
 
 /**
@@ -544,6 +566,8 @@ size_t smg_uart_write(smg_uart_t* uart, const void* buffer, size_t size)
 
 	auto buf = static_cast<const uint8_t*>(buffer);
 
+	Lock lock;
+
 	while(written < size) {
 		// If TX buffer not in use or it's empty then write directly to hardware FIFO
 		if(uart->tx_buffer == nullptr || uart->tx_buffer->isEmpty()) {

diff --git a/Sming/Arch/Esp32/Components/esp32/README.rst b/Sming/Arch/Esp32/Components/esp32/README.rst
@@ -45,20 +45,6 @@ or if multiple versions are installed. By default, the most current version will
    Location of ESP-IDF python.
 
 
-.. envvar:: CREATE_EVENT_TASK
-
-   default: disabled
-
-   .. warning::
-
-      This setting is provided for debugging purposes ONLY.
-
-   Sming uses a custom event loop to ensure that timer and task callbacks are all executed in the same
-   thread context.
-
-   Sometimes this behaviour can cause issues with IDF code.
-   Setting this to 1 will create the event loop in a separate thread, which is standard IDF behaviour.
-
 
 Background
 ----------

diff --git a/Sming/Arch/Esp32/Components/esp32/component.mk b/Sming/Arch/Esp32/Components/esp32/component.mk
@@ -8,11 +8,7 @@ COMPONENT_INCDIRS := src/include include
 # Applications can provide file with custom SDK configuration settings
 CACHE_VARS += SDK_CUSTOM_CONFIG
 
-COMPONENT_RELINK_VARS += DISABLE_NETWORK DISABLE_WIFI CREATE_EVENT_TASK
-
-ifeq ($(CREATE_EVENT_TASK),1)
-COMPONENT_CPPFLAGS += -DCREATE_EVENT_TASK
-endif
+COMPONENT_RELINK_VARS += DISABLE_NETWORK DISABLE_WIFI
 
 ifneq (,$(filter v4.%,$(IDF_VERSION)))
 IDF_VERSION_4x := 1
@@ -392,14 +388,6 @@ EXTRA_LDFLAGS := \
 	$(call LinkerScript,rom.api) \
 	$(call LinkerScript,rom.libgcc) \
 	$(call LinkerScript,rom.newlib-nano) \
-	$(call Wrap,\
-		esp_event_loop_create_default \
-		esp_event_handler_register \
-		esp_event_handler_unregister \
-		esp_event_handler_instance_register \
-		esp_event_handler_instance_unregister \
-		esp_event_post \
-		esp_event_isr_post) \
 	$(LDFLAGS_$(ESP_VARIANT)) \
 	$(call Undef,$(SDK_UNDEF_SYMBOLS)) \
 	$(call Wrap,$(SDK_WRAP_SYMBOLS))

diff --git a/Sming/Arch/Esp32/Components/esp32/sdk/config/common b/Sming/Arch/Esp32/Components/esp32/sdk/config/common
@@ -15,7 +15,7 @@
 CONFIG_COMPILER_OPTIMIZATION_SIZE=y
 
 # Mandatory LWIP changes
-CONFIG_LWIP_TCPIP_TASK_STACK_SIZE=8192
+CONFIG_LWIP_TCPIP_TASK_STACK_SIZE=16384
 CONFIG_ESP_NETIF_TCPIP_ADAPTER_COMPATIBLE_LAYER=n
 
 # Ethernet
@@ -39,7 +39,6 @@ CONFIG_BT_NIMBLE_MESH=n
 CONFIG_ESP32_WIFI_SW_COEXIST_ENABLE=n
 
 # Mandatory Sming framework changes
-CONFIG_ESP_SYSTEM_EVENT_TASK_STACK_SIZE=16384
 CONFIG_ESP_TASK_WDT_TIMEOUT_S=8
 
 # Required by HardwareSPI library
@@ -58,8 +57,5 @@ CONFIG_MBEDTLS_CERTIFICATE_BUNDLE=n
 # Don't change provided flash configuration information
 CONFIG_ESPTOOLPY_FLASHSIZE_DETECT=n
 
-# We'll handle WDT initialisation ourselves thankyouverymuch
-CONFIG_ESP_TASK_WDT_INIT=n
-
 # Issues with dual-core CPU, see #2653
 CONFIG_FREERTOS_UNICORE=y
diff --git a/Sming/Arch/Esp32/Components/esp32/sdk/config/debug b/Sming/Arch/Esp32/Components/esp32/sdk/config/debug
@@ -27,3 +27,6 @@ CONFIG_FREERTOS_WATCHPOINT_END_OF_STACK=y
 CONFIG_FREERTOS_USE_TRACE_FACILITY=y
 CONFIG_FREERTOS_GENERATE_RUN_TIME_STATS=y
 CONFIG_FREERTOS_VTASKLIST_INCLUDE_COREID=y
+
+# Watchdog
+CONFIG_ESP_TASK_WDT_PANIC=y
diff --git a/Sming/Arch/Esp32/Components/esp32/src/event_loop.cpp b/Sming/Arch/Esp32/Components/esp32/src/event_loop.cpp
diff --git a/Sming/Arch/Esp32/Components/esp32/src/startup.cpp b/Sming/Arch/Esp32/Components/esp32/src/startup.cpp
@@ -10,8 +10,6 @@
 
 #include <esp_system.h>
 #include <esp_log.h>
-#include <esp_task_wdt.h>
-#include <esp_task.h>
 #include <esp_event.h>
 #include <debug_progmem.h>
 #include <Platform/System.h>
@@ -20,73 +18,30 @@
 #include <Storage.h>
 
 extern void init();
-extern esp_event_loop_handle_t sming_create_event_loop();
 extern void esp_network_initialise();
+extern void start_sming_task_loop();
 
-namespace
-{
-void main(void*)
+extern "C" void __wrap_esp_newlib_init_global_stdio(const char*)
 {
-	int err;
-	(void)err;
-#if ESP_IDF_VERSION_MAJOR < 5
-	err = esp_task_wdt_init(CONFIG_ESP_TASK_WDT_TIMEOUT_S, true);
-#else
-	esp_task_wdt_config_t twdt_config{
-		.timeout_ms = 8000,
-		.trigger_panic = true,
-	};
-	err = esp_task_wdt_init(&twdt_config);
-#endif
-	assert(err == ESP_OK);
-
-	err = esp_task_wdt_add(nullptr);
-	assert(err == ESP_OK);
+}
 
+extern "C" void app_main(void)
+{
 	hw_timer_init();
 
 	smg_uart_detach_all();
 	esp_log_set_vprintf(m_vprintf);
 
-	auto loop = sming_create_event_loop();
-
 #ifndef DISABLE_WIFI
 	esp_network_initialise();
 #endif
 
 	System.initialize();
 	Storage::initialize();
-	init();
-
-	constexpr unsigned maxEventLoopInterval{1000 / portTICK_PERIOD_MS};
-	while(true) {
-		esp_task_wdt_reset();
-#ifdef CREATE_EVENT_TASK
-		vTaskDelay(100);
-#else
-		esp_event_loop_run(loop, maxEventLoopInterval);
-#endif
-	}
-}
 
-} // namespace
-
-extern "C" void __wrap_esp_newlib_init_global_stdio(const char*)
-{
-}
-
-extern void sming_create_task(TaskFunction_t);
-
-extern "C" void app_main(void)
-{
-#if defined(SOC_ESP32) && !CONFIG_FREERTOS_UNICORE
-	constexpr unsigned core_id{1};
-#else
-	constexpr unsigned core_id{0};
-#endif
+	// Application gets called outside main thread at startup
+	// Things like smartconfig won't work if called via task queue
+	init();
 
-#if ESP_IDF_VERSION_MAJOR < 5
-	esp_task_wdt_delete(xTaskGetIdleTaskHandleForCPU(core_id));
-#endif
-	xTaskCreatePinnedToCore(main, "Sming", ESP_TASKD_EVENT_STACK, nullptr, ESP_TASKD_EVENT_PRIO, nullptr, core_id);
+	start_sming_task_loop();
 }
diff --git a/Sming/Arch/Esp32/Components/esp32/src/system.cpp b/Sming/Arch/Esp32/Components/esp32/src/system.cpp
@@ -1,7 +1,7 @@
 #include <esp_system.h>
 #include <sys/time.h>
 #include <esp_timer.h>
-#include <esp_task_wdt.h>
+#include <esp_task.h>
 #include <sming_attr.h>
 #include <string.h>
 #if ESP_IDF_VERSION_MAJOR >= 5
@@ -75,7 +75,8 @@ void system_restart(void)
 
 void system_soft_wdt_feed(void)
 {
-	esp_task_wdt_reset();
+	// Allow the IDLE task to run
+	vTaskDelay(1);
 }
 
 void system_soft_wdt_stop(void)