diff --git a/main/drivers/display/display.c b/main/drivers/display/display.c index de7f1dc..435529d 100644 --- a/main/drivers/display/display.c +++ b/main/drivers/display/display.c @@ -60,6 +60,16 @@ static const char *TAG = "display"; static esp_lcd_panel_handle_t s_panel; static void *s_fb0, *s_fb1; +/* + * Vsync handshake for tear-free buffer swaps (the pattern from Espressif's + * own RGB-panel LVGL example). present_frame gives s_sem_present_pending after + * queueing the swap; the vsync ISR converts that into s_sem_swap_done only + * when a present is actually waiting, so a stale vsync can never satisfy a + * later wait. + */ +static SemaphoreHandle_t s_sem_present_pending; +static SemaphoreHandle_t s_sem_swap_done; + /* ------------------------------------------------------------ backlight -- */ static esp_err_t backlight_init(void) @@ -122,6 +132,52 @@ static esp_err_t panel_power_and_reset(void) return ESP_OK; } +/* + * Runs from the RGB panel's frame ISR. CONFIG_LCD_RGB_ISR_IRAM_SAFE=y requires + * this to live in IRAM. + */ +IRAM_ATTR static bool on_vsync_cb(esp_lcd_panel_handle_t panel, + const esp_lcd_rgb_panel_event_data_t *edata, + void *user_ctx) +{ + (void)panel; + (void)edata; + (void)user_ctx; + + BaseType_t hpw = pdFALSE; + if (xSemaphoreTakeFromISR(s_sem_present_pending, &hpw) == pdTRUE) { + xSemaphoreGiveFromISR(s_sem_swap_done, &hpw); + } + return hpw == pdTRUE; +} + +esp_err_t display_present_frame(void *fb) +{ + if (!s_panel || !fb) { + return ESP_ERR_INVALID_STATE; + } + + esp_err_t err = esp_lcd_panel_draw_bitmap(s_panel, 0, 0, + DISPLAY_H_RES, DISPLAY_V_RES, fb); + if (err != ESP_OK) { + return err; + } + + /* Wait for the vsync at which the queued switch latches. The timeout is a + * guard so a stalled panel degrades to tearing instead of wedging the + * LVGL task; one frame at these timings is ~15ms. */ + xSemaphoreGive(s_sem_present_pending); + if (xSemaphoreTake(s_sem_swap_done, pdMS_TO_TICKS(100)) != pdTRUE) { + static bool warned; + if (!warned) { + warned = true; + ESP_LOGW(TAG, "vsync wait timed out — panel not producing frames?"); + } + } + + return ESP_OK; +} + /* --------------------------------------------------------------- init ---- */ static esp_err_t send_init_sequence(void) @@ -243,6 +299,19 @@ static esp_err_t rgb_panel_init(void) esp_lcd_rgb_panel_get_frame_buffer(s_panel, 2, &s_fb0, &s_fb1), TAG, "get framebuffers"); + s_sem_present_pending = xSemaphoreCreateBinary(); + s_sem_swap_done = xSemaphoreCreateBinary(); + if (!s_sem_present_pending || !s_sem_swap_done) { + return ESP_ERR_NO_MEM; + } + + esp_lcd_rgb_panel_event_callbacks_t cbs = { + .on_vsync = on_vsync_cb, + }; + ESP_RETURN_ON_ERROR( + esp_lcd_rgb_panel_register_event_callbacks(s_panel, &cbs, NULL), + TAG, "vsync callback"); + ESP_LOGI(TAG, "RGB panel up: %dx%d @ %d MHz, fb0=%p fb1=%p", DISPLAY_H_RES, DISPLAY_V_RES, LCD_PCLK_HZ / 1000000, s_fb0, s_fb1); return ESP_OK; @@ -321,6 +390,6 @@ void display_test_pattern(void) } } - esp_lcd_panel_draw_bitmap(s_panel, 0, 0, DISPLAY_H_RES, DISPLAY_V_RES, fb); + display_present_frame(fb); ESP_LOGI(TAG, "colour bars drawn — this is the PRD phase-2 check"); } diff --git a/main/drivers/display/display.h b/main/drivers/display/display.h index d2dd69d..c1c7fa0 100644 --- a/main/drivers/display/display.h +++ b/main/drivers/display/display.h @@ -40,6 +40,18 @@ esp_err_t display_init(void); /* The esp_lcd panel handle, for the LVGL port. */ esp_lcd_panel_handle_t display_panel(void); +/* + * Hand a completed frame (one of the two framebuffers) to the panel and block + * until the swap has latched at vsync. + * + * esp_lcd_panel_draw_bitmap() only *queues* the framebuffer switch — the RGB + * driver latches it at the next frame boundary. Returning before that point + * lets the caller start rendering into the other buffer while it is still + * being scanned out, which is exactly the persistent-corruption race PRD 8.2 + * warns about. All frame presentation goes through here so that cannot happen. + */ +esp_err_t display_present_frame(void *fb); + /* Both framebuffers, for LVGL to render into directly. */ esp_err_t display_get_framebuffers(void **fb0, void **fb1); diff --git a/main/drivers/encoder/encoder.c b/main/drivers/encoder/encoder.c index bb24a5a..3b23fff 100644 --- a/main/drivers/encoder/encoder.c +++ b/main/drivers/encoder/encoder.c @@ -1,5 +1,6 @@ #include "encoder.h" +#include "driver/gpio.h" #include "driver/pulse_cnt.h" #include "esp_check.h" #include "esp_log.h" @@ -125,6 +126,16 @@ esp_err_t encoder_init(void) PCNT_CHANNEL_LEVEL_ACTION_INVERSE), TAG, "chan b level"); + /* + * The vendor's Arduino example uses bare INPUT, implying external pullups + * on the board, but their ESPHome config enables internal ones as well. + * Enabling them here costs nothing and guards against a floating line + * reading as phantom rotation. (Done after channel creation so the PCNT + * driver's own GPIO setup cannot undo it.) + */ + gpio_pullup_en(ENCODER_PIN_A); + gpio_pullup_en(ENCODER_PIN_B); + ESP_RETURN_ON_ERROR(pcnt_unit_enable(s_unit), TAG, "enable"); ESP_RETURN_ON_ERROR(pcnt_unit_clear_count(s_unit), TAG, "clear"); ESP_RETURN_ON_ERROR(pcnt_unit_start(s_unit), TAG, "start"); diff --git a/main/hvac/control.c b/main/hvac/control.c index ffc18e8..efc1cee 100644 --- a/main/hvac/control.c +++ b/main/hvac/control.c @@ -310,7 +310,16 @@ static void control_task(void *arg) last_call = st.calling_for_heat; } - vTaskDelay(pdMS_TO_TICKS(CONTROL_INTERVAL_S * 1000)); + /* + * Sleep in 1s slices, feeding the task watchdog each one. A single + * CONTROL_INTERVAL_S delay would finish fractionally *after* the TWDT + * deadline every iteration (both are 10s) and trip it spuriously — + * while still keeping the evaluation cadence at CONTROL_INTERVAL_S. + */ + for (int i = 0; i < CONTROL_INTERVAL_S; i++) { + esp_task_wdt_reset(); + vTaskDelay(pdMS_TO_TICKS(1000)); + } } } diff --git a/main/hvac/relay.c b/main/hvac/relay.c index 46f9704..3f980d7 100644 --- a/main/hvac/relay.c +++ b/main/hvac/relay.c @@ -31,6 +31,7 @@ static bool s_initialised; static bool s_heat_on; static bool s_requested; +static bool s_ever_requested; /* control loop has called in at least once */ static relay_fault_t s_fault; static int64_t s_boot_us; @@ -221,7 +222,15 @@ static void safety_task(void *arg) if (!control_alive) { if (s_heat_on) { turn_off_locked(RELAY_FAULT_WATCHDOG); - } else if (s_fault == RELAY_FAULT_NONE) { + } else if (s_fault == RELAY_FAULT_NONE && s_ever_requested) { + /* + * Only fault once the control loop has *stopped* talking. + * Before its first-ever call this is just boot ordering — the + * relay is held off regardless (the seeded-stale watchdog is + * the bounded-startup interlock), and flashing "control loop + * stopped" on every power-up would teach the user to ignore + * the one fault that matters most. + */ s_fault = RELAY_FAULT_WATCHDOG; } s_requested = false; @@ -269,6 +278,7 @@ esp_err_t relay_init(void) s_last_start_us = 0; s_heat_on = false; s_requested = false; + s_ever_requested = false; s_fault = RELAY_FAULT_NONE; /* @@ -303,6 +313,7 @@ void relay_request_heat(bool want_heat) xSemaphoreTake(s_lock, portMAX_DELAY); s_requested = want_heat; + s_ever_requested = true; s_last_request_us = now_us(); /* Recovering from a watchdog trip: the control loop is talking again. */ diff --git a/main/net/wifi_manager.c b/main/net/wifi_manager.c index 8d93fda..4864e40 100644 --- a/main/net/wifi_manager.c +++ b/main/net/wifi_manager.c @@ -9,6 +9,7 @@ #include "esp_log.h" #include "esp_netif.h" #include "esp_netif_sntp.h" +#include "esp_timer.h" #include "esp_wifi.h" #include "freertos/FreeRTOS.h" #include "freertos/event_groups.h" @@ -24,18 +25,31 @@ static EventGroupHandle_t s_events; static volatile bool s_connected; static int s_retry_ms = RETRY_MIN_MS; static bool s_sntp_started; +static esp_timer_handle_t s_reconnect_timer; +static void reconnect_timer_cb(void *arg) +{ + (void)arg; + esp_wifi_connect(); +} + +/* + * Schedule the retry on a timer rather than sleeping here. This runs on the + * shared system event task — a vTaskDelay would stall every queued WiFi/IP + * event behind it (up to 30s at full backoff) and can overflow the default + * event queue during an outage. + */ static void schedule_reconnect(void) { ESP_LOGW(TAG, "reconnecting in %d ms", s_retry_ms); - vTaskDelay(pdMS_TO_TICKS(s_retry_ms)); + + esp_timer_stop(s_reconnect_timer); /* no-op if not running */ + esp_timer_start_once(s_reconnect_timer, (uint64_t)s_retry_ms * 1000); s_retry_ms *= 2; if (s_retry_ms > RETRY_MAX_MS) { s_retry_ms = RETRY_MAX_MS; } - - esp_wifi_connect(); } static void event_handler(void *arg, esp_event_base_t base, @@ -49,11 +63,6 @@ static void event_handler(void *arg, esp_event_base_t base, } else if (base == WIFI_EVENT && id == WIFI_EVENT_STA_DISCONNECTED) { s_connected = false; xEventGroupClearBits(s_events, CONNECTED_BIT); - /* - * Runs on the event task, so the delay in schedule_reconnect() blocks - * only WiFi event processing — never the control loop, which lives on - * its own task and does not consult connection state anyway. - */ schedule_reconnect(); } else if (base == IP_EVENT && id == IP_EVENT_STA_GOT_IP) { @@ -92,6 +101,12 @@ esp_err_t wifi_manager_init(void) setenv("TZ", APP_TIMEZONE, 1); tzset(); + const esp_timer_create_args_t timer_args = { + .callback = reconnect_timer_cb, + .name = "wifi_retry", + }; + ESP_ERROR_CHECK(esp_timer_create(&timer_args, &s_reconnect_timer)); + ESP_ERROR_CHECK(esp_netif_init()); ESP_ERROR_CHECK(esp_event_loop_create_default()); esp_netif_create_default_wifi_sta(); diff --git a/main/net/zigbee_client.c b/main/net/zigbee_client.c index 8c418f7..cfcc8f8 100644 --- a/main/net/zigbee_client.c +++ b/main/net/zigbee_client.c @@ -103,10 +103,37 @@ static bool parse_sensor_age(const cJSON *root, uint32_t *age_s) seen = (time_t)(ls->valuedouble / 1000.0); } else if (cJSON_IsString(ls) && ls->valuestring) { struct tm tm = {0}; - if (!strptime(ls->valuestring, "%Y-%m-%dT%H:%M:%S", &tm)) { + const char *rest = strptime(ls->valuestring, "%Y-%m-%dT%H:%M:%S", &tm); + if (!rest) { return false; } - seen = (time_t)tm_to_epoch_utc(&tm); + int64_t epoch = tm_to_epoch_utc(&tm); + + /* + * Honour the offset suffix. z2m's ISO_8601 setting emits UTC ("Z"), + * but ISO_8601_local emits local time with an offset ("-05:00"). + * Ignoring that suffix would shift every reading's apparent age by + * the local offset — on a US install that is five or six hours, which + * makes live sensors look stale and trips the fallback ladder + * permanently. local = UTC + offset, so subtract it. + */ + if (*rest == '.') { /* fractional seconds */ + rest++; + while (*rest >= '0' && *rest <= '9') { + rest++; + } + } + if (*rest == '+' || *rest == '-') { + int sign = (*rest == '-') ? -1 : 1; + int oh = 0, om = 0; + if (sscanf(rest + 1, "%2d:%2d", &oh, &om) < 1) { + return false; + } + epoch -= (int64_t)sign * (oh * 3600 + om * 60); + } + /* 'Z' or nothing: already UTC. */ + + seen = (time_t)epoch; } else { return false; } diff --git a/main/ui/lvgl_port.c b/main/ui/lvgl_port.c index 8c81db8..8d1d383 100644 --- a/main/ui/lvgl_port.c +++ b/main/ui/lvgl_port.c @@ -36,12 +36,12 @@ static void flush_cb(lv_display_t *disp, const lv_area_t *area, uint8_t *px_map) (void)area; /* - * px_map is one of the panel's own framebuffers, so this does not copy — - * the RGB driver just switches which buffer is being scanned out and waits - * for the switch to take effect. + * px_map is one of the panel's own framebuffers, so no copy happens — the + * RGB driver switches which buffer is scanned out. present_frame blocks + * until that switch has latched at vsync; releasing LVGL any earlier would + * let it render the next frame into the buffer still on screen. */ - esp_lcd_panel_handle_t panel = display_panel(); - esp_lcd_panel_draw_bitmap(panel, 0, 0, DISPLAY_H_RES, DISPLAY_V_RES, px_map); + display_present_frame(px_map); lv_display_flush_ready(disp); } diff --git a/main/ui/ui_manager.c b/main/ui/ui_manager.c index 52ef25e..b9a3f91 100644 --- a/main/ui/ui_manager.c +++ b/main/ui/ui_manager.c @@ -179,7 +179,7 @@ static void build_screen(void) static void refresh_temperatures(const control_state_t *st) { - char buf[32]; + char buf[40]; if (isnan(st->governing_temp_c)) { set_text_if_changed(s_lbl_temp, "--" DEG); @@ -189,15 +189,27 @@ static void refresh_temperatures(const control_state_t *st) } /* - * Show the effective setpoint, not the requested one. When the fallback - * ladder has clamped it, the screen must say what the thermostat is - * actually doing — the fault text below explains why. + * The setpoint shown must come from settings, live — not from control + * state, which only refreshes every CONTROL_INTERVAL_S. Rendering the + * control loop's copy makes the knob feel dead for up to 10 seconds per + * detent. When the fallback ladder has clamped the working setpoint, show + * both: what the user asked for and what the thermostat is actually + * holding — the fault text below explains why they differ. */ - snprintf(buf, sizeof(buf), "Set %.0f%s", - c_to_display(st->effective_setpoint_c), unit_suffix()); + float live_sp = control_get_setpoint(); + bool capped = st->effective_setpoint_c < live_sp - 0.05f; + + if (capped) { + snprintf(buf, sizeof(buf), "Set %.0f%s (cap %.0f)", + c_to_display(live_sp), unit_suffix(), + c_to_display(st->effective_setpoint_c)); + } else { + snprintf(buf, sizeof(buf), "Set %.0f%s", + c_to_display(live_sp), unit_suffix()); + } set_text_if_changed(s_lbl_setpoint, buf); - int pct = (int)((st->effective_setpoint_c - CONTROL_SETPOINT_MIN_C) * 100.0f / + int pct = (int)((live_sp - CONTROL_SETPOINT_MIN_C) * 100.0f / (CONTROL_SETPOINT_MAX_C - CONTROL_SETPOINT_MIN_C)); if (pct < 0) { pct = 0; @@ -216,13 +228,21 @@ static void refresh_room_and_mode(const control_state_t *st) : "No sensor"; set_text_if_changed(s_lbl_room, room); + /* + * While a selection is in progress, read the *settings* value, not control + * state: the control loop only re-evaluates every CONTROL_INTERVAL_S, and + * showing its lagging copy would make the knob appear to do nothing. + * control_get_mode()/control_get_selected_room() go straight to settings. + */ char buf[48]; switch (s_focus) { case FOCUS_ROOM: - snprintf(buf, sizeof(buf), "> SELECT ROOM <"); + snprintf(buf, sizeof(buf), "> ROOM: %s <", + zigbee_client_room_name(control_get_selected_room())); break; case FOCUS_MODE: - snprintf(buf, sizeof(buf), "> MODE: %s <", control_mode_str(st->mode)); + snprintf(buf, sizeof(buf), "> MODE: %s <", + control_mode_str(control_get_mode())); break; default: snprintf(buf, sizeof(buf), "%s", control_mode_str(st->mode)); diff --git a/sdkconfig.defaults b/sdkconfig.defaults index 981ce0d..22632c3 100644 --- a/sdkconfig.defaults +++ b/sdkconfig.defaults @@ -71,8 +71,13 @@ CONFIG_GDMA_CTRL_FUNC_IN_IRAM=y # --- Watchdogs --------------------------------------------------------------- # The task watchdog is part of the safety story: a hung control task panics the # chip, and the reboot path de-asserts the relay within about a second. +# ESP_TASK_WDT_PANIC is what makes that true — without it the TWDT only logs a +# warning and a hung task keeps hanging. (The relay layer's own 30s watchdog +# has already dropped the heat call by then; this reboots the chip so the +# thermostat comes back instead of staying wedged.) CONFIG_ESP_TASK_WDT_INIT=y CONFIG_ESP_TASK_WDT_TIMEOUT_S=10 +CONFIG_ESP_TASK_WDT_PANIC=y CONFIG_ESP_TASK_WDT_CHECK_IDLE_TASK_CPU0=n CONFIG_ESP_TASK_WDT_CHECK_IDLE_TASK_CPU1=n CONFIG_ESP_SYSTEM_PANIC_PRINT_REBOOT=y