Fix brittle self-tests and add missing coverage

Author: divadiow

src/driver/drv_openWeatherMap.h

@@ -23,5 +23,6 @@ typedef struct weatherChannels_s {
 
 weatherData_t *Weather_GetData();
 const char *Weather_GetReply();
+void Weather_SetReply(const char *s);
 
 #endif // __DRV_OPENWEATHERMAP_H__

src/selftest/selftest_DHT.c

@@ -20,6 +20,10 @@ void Test_DHT() {
 	PIN_SetPinChannel2ForPinIndex(9, 2);
 
 	Sim_RunSeconds(5.0f, false);
+	// After removing the role the driver should no longer update the channels.
+	// They must remain at the last values sampled while the DHT11 role was active.
+	SELFTEST_ASSERT_CHANNEL(1, 190);
+	SELFTEST_ASSERT_CHANNEL(2, 67);
 }
 
 

src/selftest/selftest_enums.c

@@ -24,8 +24,8 @@ void Test_Enum_LowMidH() {
 
 	SELFTEST_ASSERT_HAS_MQTT_JSON_SENT_ANY_4KEY("homeassistant", true, 0, 0,
 		"state_topic", "~/13/get",
-		"uniq_id", "Windows_Enum_select_13",
-		"uniq_id", "Windows_Enum_select_13",
+		"command_topic", "~/13/set",
+		"~", "obkEnumDemo",
 		"uniq_id", "Windows_Enum_select_13");
 
 	SELFTEST_ASSERT_HAS_MQTT_JSON_SENT_ANY_4KEY("homeassistant", true, 0, 0,
@@ -63,8 +63,8 @@ void Test_Enum_LowMidHighOff() {
 
 	SELFTEST_ASSERT_HAS_MQTT_JSON_SENT_ANY_4KEY("homeassistant", true, 0, 0,
 		"state_topic", "~/12/get",
-		"uniq_id", "Windows_Enum_select_12",
-		"uniq_id", "Windows_Enum_select_12",
+		"command_topic", "~/12/set",
+		"~", "obkEnumDemo",
 		"uniq_id", "Windows_Enum_select_12");
 
 	SELFTEST_ASSERT_HAS_MQTT_JSON_SENT_ANY_4KEY("homeassistant", true, 0, 0,
@@ -123,8 +123,8 @@ void Test_Enum_3opts() {
 
 	SELFTEST_ASSERT_HAS_MQTT_JSON_SENT_ANY_4KEY("homeassistant", true, 0, 0,
 		"state_topic", "~/14/get",
-		"unique_id", "14",
-		"unique_id", "14",
+		"command_topic", "~/14/set",
+		"~", "obkEnumDemo",
 		"uniq_id", "Windows_Enum_select_14");
 
 	SELFTEST_ASSERT_HAS_MQTT_JSON_SENT_ANY_4KEY("homeassistant", true, 0, 0,
@@ -195,8 +195,8 @@ void Test_Enum_BadOk() {
 
 	SELFTEST_ASSERT_HAS_MQTT_JSON_SENT_ANY_4KEY("homeassistant", true, 0, 0,
 		"state_topic", "~/4/get",
-		"unique_id", "4",
-		"unique_id", "4",
+		"command_topic", "~/4/set",
+		"~", "obkEnumDemo",
 		"uniq_id", "Windows_Enum_select_4");
 
 	SELFTEST_ASSERT_HAS_MQTT_JSON_SENT_ANY_4KEY("homeassistant", true, 0, 0,

src/selftest/selftest_flags.c

@@ -49,7 +49,7 @@ void Test_Flags() {
 		bool bSet = (i == 33 || i == 0 || i == 1 || i == 55 || i == 63);
 		SELFTEST_ASSERT_FLAG(i, bSet);
 	}
-	// clear flag 63
+	// clear flag 55
 	CMD_ExecuteCommand("SetFlag 55 0", 0);
 	for (int i = 0; i < 64; i++) {
 		bool bSet = (i == 33 || i == 0 || i == 1 || i == 63);

src/selftest/selftest_if_inside_backlog.c

@@ -14,9 +14,32 @@ void Test_IF_Inside_Backlog() {
 	PIN_SetPinRoleForPinIndex(26, IOR_PWM);
 	PIN_SetPinChannelForPinIndex(26, 2);
 
+	// Non-LED test: verify that "if" works correctly when nested inside a backlog alias.
+	// Uses plain channels so this runs regardless of ENABLE_LED_BASIC.
+	CMD_ExecuteCommand("alias pick_val backlog if $CH10>7&&$CH10<19 then setChannel 20 200; if $CH10<=7||$CH10>=19 then setChannel 20 500;", 0);
 
-#if ENABLE_LED_BASIC
+	// hour 10 -> day -> channel 20 = 200
+	CMD_ExecuteCommand("setChannel 10 10", 0);
+	CMD_ExecuteCommand("pick_val", 0);
+	SELFTEST_ASSERT_CHANNEL(20, 200);
 
+	// hour 3 -> night -> channel 20 = 500
+	CMD_ExecuteCommand("setChannel 10 3", 0);
+	CMD_ExecuteCommand("pick_val", 0);
+	SELFTEST_ASSERT_CHANNEL(20, 500);
+
+	// hour 12 -> day -> channel 20 = 200
+	CMD_ExecuteCommand("setChannel 10 12", 0);
+	CMD_ExecuteCommand("pick_val", 0);
+	SELFTEST_ASSERT_CHANNEL(20, 200);
+
+	// hour 22 -> night -> channel 20 = 500
+	CMD_ExecuteCommand("setChannel 10 22", 0);
+	CMD_ExecuteCommand("pick_val", 0);
+	SELFTEST_ASSERT_CHANNEL(20, 500);
+
+#if ENABLE_LED_BASIC
+	// LED-specific variant: same logic but driving led_temperature instead
 	CMD_ExecuteCommand("led_enableAll 1", 0);
 
 	CMD_ExecuteCommand("alias day_lights backlog led_temperature 200", 0);

src/selftest/selftest_openWeatherMap.c

@@ -37,12 +37,25 @@ const char *reply_no_weather =
 "HTTP/1.1 200 OK\r\n\r\n"
 "{\"coord\":{\"lon\":10,\"lat\":50},\"main\":{\"temp\":20,\"pressure\":900,\"humidity\":60}}";
 
+const char *owm_rain_reply =
+"HTTP/1.1 200 OK\r\n\r\n"
+"{"
+"  \"coord\": {\"lon\": 11.0, \"lat\": 51.0},"
+"  \"weather\": [{\"id\":500,\"main\":\"Rain\",\"description\":\"light rain\",\"icon\":\"10d\"}],"
+"  \"main\": {\"temp\": 18.0, \"pressure\": 901, \"humidity\": 70},"
+"  \"timezone\": 3600,"
+"  \"sys\": {\"sunrise\":1604960000,\"sunset\":1605000000}"
+"}";
+
+void Test_OpenWeatherMap_StaleWeatherStringsFollowLatestFullReply();
+
 void Test_OpenWeatherMap() {
+	weatherData_t *w;
 
 	// HTTP header + json
 	Weather_SetReply(owm_sample_reply);
 
-	weatherData_t *w = Weather_GetData();
+	w = Weather_GetData();
 	SELFTEST_ASSERT_FLOATCOMPARE(w->humidity, 85);
 	SELFTEST_ASSERT_FLOATCOMPARE(w->pressure, 998);
 	SELFTEST_ASSERT_FLOATCOMPARE(w->temp, 23);
@@ -51,7 +64,50 @@ void Test_OpenWeatherMap() {
 	SELFTEST_ASSERT_FLOATCOMPARE(w->lat, 50);
 	SELFTEST_ASSERT_FLOATCOMPARE(w->lon, 10);
 
+	// Test with a response that has no "weather" array (edge case)
+	Weather_SetReply(reply_no_weather);
+	w = Weather_GetData();
+	// Numeric fields from "main" and "coord" must still parse
+	SELFTEST_ASSERT_FLOATCOMPARE(w->humidity, 60);
+	SELFTEST_ASSERT_FLOATCOMPARE(w->pressure, 900);
+	SELFTEST_ASSERT_FLOATCOMPARE(w->temp, 20);
+	SELFTEST_ASSERT_FLOATCOMPARE(w->lat, 50);
+	SELFTEST_ASSERT_FLOATCOMPARE(w->lon, 10);
+	// Current implementation only updates these strings when weather[0] exists,
+	// so a reply without a weather array keeps the previous parsed values.
+	SELFTEST_ASSERT_STRING(w->main_weather, "Clear");
+	SELFTEST_ASSERT_STRING(w->description, "clear sky");
 
+	Test_OpenWeatherMap_StaleWeatherStringsFollowLatestFullReply();
+}
+
+void Test_OpenWeatherMap_StaleWeatherStringsFollowLatestFullReply() {
+	weatherData_t *w;
+
+	Weather_SetReply(owm_sample_reply);
+	w = Weather_GetData();
+	SELFTEST_ASSERT_STRING(w->main_weather, "Clear");
+	SELFTEST_ASSERT_STRING(w->description, "clear sky");
+
+	Weather_SetReply(owm_rain_reply);
+	w = Weather_GetData();
+	SELFTEST_ASSERT_FLOATCOMPARE(w->humidity, 70);
+	SELFTEST_ASSERT_FLOATCOMPARE(w->pressure, 901);
+	SELFTEST_ASSERT_FLOATCOMPARE(w->temp, 18);
+	SELFTEST_ASSERT_FLOATCOMPARE(w->lat, 51);
+	SELFTEST_ASSERT_FLOATCOMPARE(w->lon, 11);
+	SELFTEST_ASSERT_STRING(w->main_weather, "Rain");
+	SELFTEST_ASSERT_STRING(w->description, "light rain");
+
+	Weather_SetReply(reply_no_weather);
+	w = Weather_GetData();
+	SELFTEST_ASSERT_FLOATCOMPARE(w->humidity, 60);
+	SELFTEST_ASSERT_FLOATCOMPARE(w->pressure, 900);
+	SELFTEST_ASSERT_FLOATCOMPARE(w->temp, 20);
+	SELFTEST_ASSERT_FLOATCOMPARE(w->lat, 50);
+	SELFTEST_ASSERT_FLOATCOMPARE(w->lon, 10);
+	SELFTEST_ASSERT_STRING(w->main_weather, "Rain");
+	SELFTEST_ASSERT_STRING(w->description, "light rain");
 }
 
 

src/selftest/selftest_script.c

@@ -129,9 +129,17 @@ void Test_Scripting_Loop3() {
 		SELFTEST_ASSERT_INTEGER(CMD_GetCountActiveScriptThreads(), 1);
 		SELFTEST_ASSERT_CHANNEL(0, 1);
 	}
-	for (int i = 0; i < 2; i++) {
-		Sim_RunSeconds(1, false);
-	}
+	// Second 15: the script delay counts down to 0, but the final if/setChannel
+	// pair does not run until the following scheduler tick.
+	Sim_RunSeconds(1, false);
+	SELFTEST_ASSERT_INTEGER(CMD_GetCountActiveScriptThreads(), 1);
+	SELFTEST_ASSERT_CHANNEL(0, 1);
+	// After second 16 the final loop body has resumed and the script must have ended.
+	Sim_RunSeconds(1, false);
+	SELFTEST_ASSERT_INTEGER(CMD_GetCountActiveScriptThreads(), 0);
+	// Channel 0 should now be 0 because the script's final command has run.
+	SELFTEST_ASSERT_CHANNEL(0, 0);
+	// Run a couple more seconds to confirm nothing changes again
 	for (int i = 0; i < 3; i++) {
 		Sim_RunSeconds(1, false);
 		SELFTEST_ASSERT_INTEGER(CMD_GetCountActiveScriptThreads(), 0);

src/selftest/selftest_shutters.c

@@ -16,17 +16,30 @@ void Test_Shutters() {
 	PIN_SetPinRoleForPinIndex(7, IOR_Button_ShutterDown);
 
 	CMD_ExecuteCommand("startDriver Shutters", 0);
+	// Run 1 second so DRV_Shutters_RunEverySecond fires and registers the shutter struct.
+	// The struct is lazily created the first time RunEverySecond scans the pins.
+	Sim_RunSeconds(1.0f, false);
+	// Initially both pins should be off (shutter is stopped at start)
 	SELFTEST_ASSERT_PIN_BOOLEAN(4, false);
 	SELFTEST_ASSERT_PIN_BOOLEAN(5, false);
+
+	// Issue an open command (100% = fully open = SHUTTER_OPENING direction)
 	CMD_ExecuteCommand("ShutterMove0 100", 0);
+	// After issuing the open command: ShutterA=1 (opening), ShutterB=0
+	SELFTEST_ASSERT_PIN_BOOLEAN(4, true);
+	SELFTEST_ASSERT_PIN_BOOLEAN(5, false);
+
 	Sim_RunSeconds(1.0f, false);
-	//Sim_RunFrames(1, false);
-	//SELFTEST_ASSERT_PIN_BOOLEAN(4, true);
-	//SELFTEST_ASSERT_PIN_BOOLEAN(5, false);
+	// Still moving after 1 second (default open time is 10s)
+	SELFTEST_ASSERT_PIN_BOOLEAN(4, true);
+	SELFTEST_ASSERT_PIN_BOOLEAN(5, false);
+
+	// Stop the shutter
 	CMD_ExecuteCommand("ShutterMove0 Stop", 0);
 	Sim_RunFrames(1, false);
-	//SELFTEST_ASSERT_PIN_BOOLEAN(4, false);
-	//SELFTEST_ASSERT_PIN_BOOLEAN(5, false);
+	// After Stop: both pins must be off
+	SELFTEST_ASSERT_PIN_BOOLEAN(4, false);
+	SELFTEST_ASSERT_PIN_BOOLEAN(5, false);
 
 
 

src/selftest/selftest_tclAC.c

@@ -2,30 +2,68 @@
 
 #include "selftest_local.h"
 
+extern void TCL_UART_TryToGetNextPacket(void);
+extern void TCL_UART_RunEverySecond(void);
+
+static const char *valid_status_reply =
+	"BB 01 00 04 2D 04 00 11 21 00 00 00 FF 00 00 00 00 1D CA FF 00 00 00 00 00 F0 FF 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 7F";
+
+static const char *invalid_status_reply =
+	"BB 01 00 04 2D 04 00 11 21 00 00 00 FF 00 00 00 00 1D CA FF 00 00 00 00 00 F0 FF 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00";
+
+void Test_Driver_TCL_AC_InvalidReplyDoesNotBlockPolling() {
+	SIM_ClearOBK(0);
+	CMD_ExecuteCommand("lfs_format", 0);
+	SIM_ClearUART();
+	SIM_UART_InitReceiveRingBuffer(256);
+
+	CMD_ExecuteCommand("startDriver TCL", 0);
+
+	CMD_ExecuteCommand(va("uartFakeHex %s", invalid_status_reply), 0);
+	TCL_UART_TryToGetNextPacket();
+	SELFTEST_ASSERT_HAS_UART_EMPTY();
+
+	TCL_UART_RunEverySecond();
+	SELFTEST_ASSERT_HAS_SOME_DATA_IN_UART();
+}
 
 void Test_Driver_TCL_AC() {
+	Test_Driver_TCL_AC_InvalidReplyDoesNotBlockPolling();
+
 	// reset whole device
 	SIM_ClearOBK(0);
 	CMD_ExecuteCommand("lfs_format", 0);
 	SIM_ClearUART();
+	// Required to set up the receive ring buffer before feeding bytes with uartFakeHex
+	SIM_UART_InitReceiveRingBuffer(256);
 
 	CMD_ExecuteCommand("startDriver TCL", 0);
 
+	// ACMode 1 = COOL mode: builds a 35-byte set command and flags it for sending
 	CMD_ExecuteCommand("ACMode 1", 0);
 	TCL_UART_RunEverySecond();
+	// TCL set command is always 35 bytes — verify bytes were sent
+	SELFTEST_ASSERT_HAS_SOME_DATA_IN_UART();
 	SIM_ClearUART();
 
+	// FANMode 1 = FAN_1: rebuilds and queues another 35-byte set command
 	CMD_ExecuteCommand("FANMode 1", 0);
 	TCL_UART_RunEverySecond();
+	SELFTEST_ASSERT_HAS_SOME_DATA_IN_UART();
 	SIM_ClearUART();
 
-
-	CMD_ExecuteCommand("uartFakeHex BB 01 00 04 2D 04 00 00 00 00 00 00 FF 00 00 00 00 00 FF FF 00 00 00 00 00 00 F0 FF 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 6A", 0);
+	// Feed a valid get-response packet into the driver UART buffer and parse it directly.
+	// Then run the normal every-second path once to ensure the driver continues polling.
+	CMD_ExecuteCommand(va("uartFakeHex %s", valid_status_reply), 0);
+	TCL_UART_TryToGetNextPacket();
+	// The fake response should be fully consumed by the parser.
+	SELFTEST_ASSERT_HAS_UART_EMPTY();
 	TCL_UART_RunEverySecond();
+	// After parsing, the regular poll path should still send the next request frame.
+	SELFTEST_ASSERT_HAS_SOME_DATA_IN_UART();
 }
 
 
 
 #endif
 
-