Add delegate support to task queue (#1851)

**Add `ENABLE_TASK_COUNT`** to control task counter, disable by default.
    
When stressed with periodic interrupts, the only way to get a reliable count
is to disable interrupts whilst updating the task counts.
As this is a diagnostic feature, leaving it disabled by default seems appropriate.

Fortunately, `noInterrupts()` already returns the existing interrupt level,
so we just need to add `restoreInterrupts()` so we can safely use it
from interrupt context.

**Fix 'ready' state tracking**
    
If no `onReady` handlers are set then `state` will never be set to ready.
Use the original `init_done_cb` callback to handle this separately.
Note: this gets called first before any queued task callbacks.

**Change standard TaskCallback parameter** to `void*`
    
Makes it easier to use and provides consistency with timer callbacks.
Avoids casting in many cases, and only requires `static_cast` for pointers.
Add `InterruptCallback` overload.
Note: Retaining existing `uint32_t` parameter overload.

**Add Delegate support** for System::queueCallback()

**Expand documentation** with notes on memory, multitasking, tasks

Sming/Arch/Esp8266/Components/esp8266/include/esp_systemapi.h

@@ -116,6 +116,7 @@ extern void ets_isr_unmask(unsigned intr);
 #include "xtensa/xtruntime.h"
 
 /** @brief  Disable interrupts
+ *  @retval Current interrupt level
  *  @note Hardware timer is unaffected if operating in non-maskable mode
  */
 #define noInterrupts() XTOS_SET_INTLEVEL(15)
@@ -124,6 +125,10 @@ extern void ets_isr_unmask(unsigned intr);
 */
 #define interrupts() XTOS_SET_INTLEVEL(0)
 
+/** @brief Restore interrupts to level saved from previous noInterrupts() call
+ */
+#define restoreInterrupts(level) XTOS_RESTORE_INTLEVEL(level)
+
 #ifdef __cplusplus
 }
 #endif

Sming/Arch/Esp8266/Components/gdbstub/gdbstub.cpp

@@ -753,7 +753,7 @@ void ATTR_GDBEXTERNFN commandLoop(bool waitForStart, bool allowDetach)
 	}
 
 	if(gdb_state.attached != initiallyAttached) {
-		System.queueCallback(TaskCallback(gdb_on_attach), gdb_state.attached);
+		System.queueCallback(TaskCallback32(gdb_on_attach), gdb_state.attached);
 	}
 
 	debug_i("<< LEAVE CMDLOOP");

Sming/Arch/Esp8266/Components/gdbstub/gdbsyscall.cpp

@@ -253,7 +253,7 @@ bool ATTR_GDBEXTERNFN gdb_syscall_complete(const char* data)
 	}
 
 	if(syscall_info.callback != nullptr) {
-		System.queueCallback(TaskCallback(syscall_info.callback), uint32_t(&syscall_info));
+		System.queueCallback(TaskCallback(syscall_info.callback), &syscall_info);
 	}
 	gdb_state.syscall = syscall_ready;
 

Sming/Arch/Esp8266/Components/gdbstub/gdbuart.cpp

@@ -203,7 +203,7 @@ void ATTR_GDBEXTERNFN gdbFlushUserData()
 
 #if GDBSTUB_ENABLE_UART2
 
-static void sendUserDataTask(uint32_t)
+static void sendUserDataTask()
 {
 	sendUserDataQueued = false;
 
@@ -321,7 +321,7 @@ static void IRAM_ATTR gdb_uart_callback(uart_t* uart, uint32_t status)
 			if(breakCheck && c == '\x03') {
 				if(break_requests++ == 0) {
 					// First attempt, break within a task callback
-					System.queueCallback(TaskCallback(doCtrlBreak));
+					System.queueCallback(doCtrlBreak);
 				} else if(break_requests == 3) {
 					// Application failed to stop, break immediately
 					doCtrlBreak();

Sming/Arch/Host/Components/esp_hal/include/esp_systemapi.h

@@ -44,13 +44,28 @@ struct ip_addr {
 #include <assert.h>
 
 #include "debug_progmem.h"
+#define debugf debug_i
 
 #define SYSTEM_ERROR(fmt, ...) hostmsg("ERROR: " fmt "\r\n", ##__VA_ARGS__)
 
-#define noInterrupts()
-#define interrupts()
+__forceinline unsigned noInterrupts()
+{
+	ets_intr_lock();
+	return 1;
+}
+
+__forceinline void interrupts()
+{
+	ets_intr_unlock();
+}
+
+__forceinline void restoreInterrupts(unsigned level)
+{
+	(void)level;
+	interrupts();
+}
 
-#define BIT(nr)         (1UL << (nr))
+#define BIT(nr) (1UL << (nr))
 
 #ifdef __cplusplus
 }

Sming/Core/HardwareSerial.cpp

@@ -159,9 +159,9 @@ unsigned HardwareSerial::getStatus()
 /*
  * Called via task queue
  */
-void HardwareSerial::staticOnStatusChange(uint32_t param)
+void HardwareSerial::staticOnStatusChange(void* param)
 {
-	auto serial = reinterpret_cast<HardwareSerial*>(param);
+	auto serial = static_cast<HardwareSerial*>(param);
 	if(serial != nullptr) {
 		serial->invokeCallbacks();
 	}
@@ -181,7 +181,7 @@ void HardwareSerial::staticCallbackHandler(uart_t* uart, uint32_t status)
 
 	// If required, queue a callback
 	if((status & serial->statusMask) != 0 && !serial->callbackQueued) {
-		System.queueCallback(staticOnStatusChange, uint32_t(serial));
+		System.queueCallback(staticOnStatusChange, serial);
 		serial->callbackQueued = true;
 	}
 }

Sming/Core/HardwareSerial.h

@@ -461,7 +461,7 @@ class HardwareSerial : public ReadWriteStream
 	 * @param status UART status flags indicating cause(s) of interrupt
 	 */
 	static void IRAM_ATTR staticCallbackHandler(uart_t* uart, uint32_t status);
-	static void staticOnStatusChange(uint32_t param);
+	static void staticOnStatusChange(void* param);
 	void invokeCallbacks();
 
 	/**

Sming/Platform/System.cpp

@@ -11,86 +11,111 @@
 #include "Platform/System.h"
 #include "Timer.h"
 
+#ifndef TASK_QUEUE_LENGTH
+/** @brief default number of tasks in global queue
+ *  @note tasks are usually short-lived and executed very promptly, so a large queue is
+ *  normally un-necessry. If queue overrun is suspected, check `SystemClass::getMaxTaskCount()`.
+ */
+#define TASK_QUEUE_LENGTH 10
+#endif
+
 SystemClass System;
 
 SystemState SystemClass::state = eSS_None;
 os_event_t SystemClass::taskQueue[TASK_QUEUE_LENGTH];
+
+#ifdef ENABLE_TASK_COUNT
 volatile uint8_t SystemClass::taskCount;
 volatile uint8_t SystemClass::maxTaskCount;
+#endif
 
 /** @brief OS calls this function which invokes user-defined callback
- *  @note callback function pointer is placed in event->sig, with parameter
- *  in event->par.
+ *  @note callback function pointer is placed in event->sig, with parameter in event->par.
  */
 void SystemClass::taskHandler(os_event_t* event)
 {
-	auto callback = reinterpret_cast<TaskCallback>(event->sig);
-	if(callback) {
-		// If we get interrupt during adjustment of the counter, do it again
-		uint8_t oldCount = taskCount;
-		--taskCount;
-		if(taskCount != oldCount - 1)
-			--taskCount;
+#ifdef ENABLE_TASK_COUNT
+	auto level = noInterrupts();
+	--taskCount;
+	restoreInterrupts(level);
+#endif
+	auto callback = reinterpret_cast<TaskCallback32>(event->sig);
+	if(callback != nullptr) {
 		callback(event->par);
 	}
 }
 
 bool SystemClass::initialize()
 {
-	if(state != eSS_None)
+	if(state != eSS_None) {
 		return false;
+	}
 
 	state = eSS_Intializing;
 
 	// Initialise the global task queue
-	return system_os_task(taskHandler, USER_TASK_PRIO_1, taskQueue, TASK_QUEUE_LENGTH);
+	if(!system_os_task(taskHandler, USER_TASK_PRIO_1, taskQueue, TASK_QUEUE_LENGTH)) {
+		return false;
+	}
+
+#ifdef ARCH_ESP8266
+	system_init_done_cb([]() { state = eSS_Ready; });
+#else
+	state = eSS_Ready;
+#endif
+
+	return true;
 }
 
-bool SystemClass::queueCallback(TaskCallback callback, uint32_t param)
+bool SystemClass::queueCallback(TaskCallback32 callback, uint32_t param)
 {
 	if(callback == nullptr) {
 		return false;
 	}
 
-	if(++taskCount > maxTaskCount) {
+#ifdef ENABLE_TASK_COUNT
+	auto level = noInterrupts();
+	++taskCount;
+	if(taskCount > maxTaskCount) {
 		maxTaskCount = taskCount;
 	}
+	restoreInterrupts(level);
+#endif
 
 	return system_os_post(USER_TASK_PRIO_1, reinterpret_cast<os_signal_t>(callback), param);
 }
 
-void SystemClass::onReady(SystemReadyDelegate readyHandler)
+bool SystemClass::queueCallback(TaskDelegate callback)
 {
-	if(readyHandler) {
-		auto handler = new SystemReadyDelegate(readyHandler);
-		queueCallback(
-			[](uint32_t param) {
-				SystemClass::state = eSS_Ready;
-				auto handler = reinterpret_cast<SystemReadyDelegate*>(param);
-				(*handler)();
-				delete handler;
-			},
-			reinterpret_cast<uint32_t>(handler));
+	if(!callback) {
+		return false;
 	}
-}
 
-void SystemClass::onReady(ISystemReadyHandler* readyHandler)
-{
-	if(readyHandler) {
-		queueCallback(
-			[](uint32_t param) {
-				SystemClass::state = eSS_Ready;
-				auto handler = reinterpret_cast<ISystemReadyHandler*>(param);
-				handler->onSystemReady();
-			},
-			reinterpret_cast<uint32_t>(readyHandler));
+	// @todo consider failing immediately if called from interrupt context
+
+	auto delegate = new TaskDelegate(callback);
+	if(delegate == nullptr) {
+		return false;
+	}
+
+	auto delegateHandler = [](void* param) {
+		auto delegate = static_cast<TaskDelegate*>(param);
+		(*delegate)();
+		delete delegate;
+	};
+
+	if(!queueCallback(delegateHandler, delegate)) {
+		delete delegate;
+		return false;
 	}
+
+	return true;
 }
 
 void SystemClass::restart(unsigned deferMillis)
 {
 	if(deferMillis == 0) {
-		queueCallback([](uint32_t) { system_restart(); });
+		queueCallback(system_restart);
 	} else {
 		auto timer = new AutoDeleteTimer;
 		timer->initializeMs(deferMillis, system_restart).startOnce();