Stop the constant repaint that made the left-edge artefacts permanent
The artefacts went from occasional to continuous, and the cause was my own
mitigation for them.
lv_obj_set_local_style_prop() does NOT skip identical values -- it calls
lv_obj_refresh_style() unconditionally, which invalidates the object. A
comment in ui_set_stale() claimed the opposite and that assumption spread.
ui_update_thermostat() is called once a second by the control task and was
writing seven style properties every time, so the panel was redrawing
something forever, and glitched lines were being regenerated as fast as
anything could clear them.
Two full-screen invalidates made it worse still: one after every HA poll,
one 600ms after every touch. The premise was that LVGL never reinvalidates
pixels it does not know are wrong, so repainting would clean them up. But
the README already had the answer -- the corruption scales with how much is
written, because LVGL's PSRAM writes compete with the panel's DMA reads out
of the same PSRAM. A full repaint is 768KB of exactly the contention that
causes the problem. Both are removed.
What is left is the thing that actually helps: write less.
- ui_update_thermostat() returns without taking the lock when nothing
moved, and otherwise touches only the widgets whose values changed.
- ui_set_stale() is guarded on the previous value.
- Condition and trend icons only get a colour write when the glyph itself
changed.
Also raises the bounce buffer from 16 to 20 scanlines. The comment saying
20 "fails to allocate once WiFi is up" was true when display_init() ran
after wifi_init_sta(); it does not any more. display_alloc() takes the
buffers from a virgin heap with ~200KB free, so the extra 12.8KB is
affordable, and each larger chunk buys the refill ISR more time to fetch the
next one before the LCD needs it. That attacks the mechanism rather than the
symptom. ~24KB internal heap remains once WiFi is up; the "Free internal
heap" log line is the thing to watch if it needs to come back down.
Both gotchas are now in the README so the next person does not reach for a
repaint again.
Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
1 parent
3aabcac90a
commit
7a58f9838a
5 files changed
+212
-129
No files matched your search
+154
-104
@@ -195,33 +195,20 @@ static bool slider_touched[HA_LIGHT_COUNT];
|
||||
static int32_t slider_sent_pct[HA_LIGHT_COUNT];
|
||||
|
||||
/*
|
||||
* One-shot "repair" repaint, rescheduled by every interaction to fire 600ms
|
||||
* after the last one.
|
||||
* There is deliberately no "repair" repaint here any more.
|
||||
*
|
||||
* Redrawing is itself what glitches the panel: the flush's PSRAM writes
|
||||
* contend with the LCD's scan-out, and a burst of interactive redraws is the
|
||||
* most likely moment for the bounce ISR to lose (README, PSRAM bandwidth).
|
||||
* The glitched lines then persist because LVGL believes those pixels are
|
||||
* fine. One full repaint after the burst settles cleans them up within
|
||||
* ~0.6s instead of leaving them until the next poll's repaint, up to 60s
|
||||
* away. It runs once per burst, not periodically -- a constant repaint loop
|
||||
* would be a constant supply of fresh glitch opportunities.
|
||||
* An earlier version scheduled a full-screen invalidate 600ms after each
|
||||
* interaction, on the theory that repainting would clear glitched pixels
|
||||
* LVGL does not know are wrong. It made the artefacts worse and constant
|
||||
* rather than occasional, for the reason the README already gave: the
|
||||
* corruption scales with how much is written, because LVGL's PSRAM writes
|
||||
* compete with the panel's DMA reads out of the same PSRAM. Repainting to
|
||||
* fix corruption caused by repainting does not converge.
|
||||
*
|
||||
* The lever that actually works is writing less. Everything below is
|
||||
* change-guarded: if a value has not moved, no style is written and nothing
|
||||
* is invalidated.
|
||||
*/
|
||||
static lv_timer_t *repair_timer;
|
||||
|
||||
static void repair_timer_cb(lv_timer_t *t)
|
||||
{
|
||||
lv_obj_invalidate(lv_screen_active());
|
||||
lv_timer_pause(t);
|
||||
}
|
||||
|
||||
static void schedule_repair(void)
|
||||
{
|
||||
if (repair_timer != NULL) {
|
||||
lv_timer_reset(repair_timer);
|
||||
lv_timer_resume(repair_timer);
|
||||
}
|
||||
}
|
||||
|
||||
/* --------------------------------------------------------- condition icons */
|
||||
|
||||
@@ -348,18 +335,24 @@ static void trend_render(lv_obj_t *icon, ha_trend_t t)
|
||||
return;
|
||||
}
|
||||
|
||||
switch (t) {
|
||||
case HA_TREND_RISING:
|
||||
label_set_if_changed(icon, ICON_TREND_UP);
|
||||
/*
|
||||
* The colour write is skipped when the glyph did not change. Style writes
|
||||
* invalidate unconditionally, so an unguarded one here would repaint
|
||||
* three arrows on every poll to redraw the same pixels.
|
||||
*/
|
||||
const char *glyph = (t == HA_TREND_RISING) ? ICON_TREND_UP
|
||||
: (t == HA_TREND_FALLING) ? ICON_TREND_DOWN
|
||||
: "";
|
||||
const char *current = lv_label_get_text(icon);
|
||||
if (current != NULL && strcmp(current, glyph) == 0) {
|
||||
return;
|
||||
}
|
||||
|
||||
lv_label_set_text(icon, glyph);
|
||||
if (t == HA_TREND_RISING) {
|
||||
lv_obj_set_style_text_color(icon, lv_color_hex(COL_ACCENT_WARM), LV_PART_MAIN);
|
||||
break;
|
||||
case HA_TREND_FALLING:
|
||||
label_set_if_changed(icon, ICON_TREND_DOWN);
|
||||
} else if (t == HA_TREND_FALLING) {
|
||||
lv_obj_set_style_text_color(icon, lv_color_hex(COL_ACCENT), LV_PART_MAIN);
|
||||
break;
|
||||
default:
|
||||
label_set_if_changed(icon, "");
|
||||
break;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -409,7 +402,6 @@ static void therm_button_clicked(lv_event_t *e)
|
||||
if (therm_cb != NULL) {
|
||||
therm_cb(cmd);
|
||||
}
|
||||
schedule_repair();
|
||||
}
|
||||
|
||||
/* ------------------------------------------------------------ light tiles */
|
||||
@@ -469,7 +461,6 @@ static void light_tile_clicked(lv_event_t *e)
|
||||
: -1,
|
||||
};
|
||||
light_apply_optimistic(idx, expect);
|
||||
schedule_repair();
|
||||
|
||||
/* -1: a tap is on/off only. Brightness is the slider's job, and sending
|
||||
* the current brightness here would fight a user who dimmed the light
|
||||
@@ -525,7 +516,6 @@ static void light_slider_changed(lv_event_t *e)
|
||||
lv_color_hex(pct > 0 ? COL_ON : COL_TEXT_DIM),
|
||||
LV_PART_MAIN);
|
||||
|
||||
schedule_repair();
|
||||
|
||||
if (light_cb == NULL || !lv_obj_has_state(slider, LV_STATE_PRESSED)) {
|
||||
return;
|
||||
@@ -552,7 +542,6 @@ static void light_slider_released(lv_event_t *e)
|
||||
const bool touched = slider_touched[idx];
|
||||
slider_touched[idx] = false;
|
||||
|
||||
schedule_repair();
|
||||
|
||||
if (touched && pct == slider_sent_pct[idx]) {
|
||||
return; /* tap without drag: the press command already said this */
|
||||
@@ -1070,9 +1059,12 @@ static void update_forecast(const ha_data_t *d)
|
||||
|
||||
if (icon_font_26 != NULL) {
|
||||
const cond_icon_t *ci = cond_icon_lookup(f->condition);
|
||||
label_set_if_changed(fc_icon[i], ci->icon);
|
||||
lv_obj_set_style_text_color(fc_icon[i], lv_color_hex(ci->colour),
|
||||
LV_PART_MAIN);
|
||||
const char *current = lv_label_get_text(fc_icon[i]);
|
||||
if (current == NULL || strcmp(current, ci->icon) != 0) {
|
||||
lv_label_set_text(fc_icon[i], ci->icon);
|
||||
lv_obj_set_style_text_color(fc_icon[i], lv_color_hex(ci->colour),
|
||||
LV_PART_MAIN);
|
||||
}
|
||||
}
|
||||
|
||||
if (f->high_f.valid && f->low_f.valid) {
|
||||
@@ -1105,8 +1097,12 @@ void ui_update(const ha_data_t *data)
|
||||
if (icon_font_26 != NULL) {
|
||||
const cond_icon_t *ci = cond_icon_lookup(
|
||||
data->condition[0] ? data->condition : NULL);
|
||||
label_set_if_changed(cond_icon, ci->icon);
|
||||
lv_obj_set_style_text_color(cond_icon, lv_color_hex(ci->colour), LV_PART_MAIN);
|
||||
/* Colour only when the glyph moved -- see trend_render(). */
|
||||
const char *current = lv_label_get_text(cond_icon);
|
||||
if (current == NULL || strcmp(current, ci->icon) != 0) {
|
||||
lv_label_set_text(cond_icon, ci->icon);
|
||||
lv_obj_set_style_text_color(cond_icon, lv_color_hex(ci->colour), LV_PART_MAIN);
|
||||
}
|
||||
}
|
||||
/* The label's width changed with its words; re-hang it off the icon. */
|
||||
lv_obj_align_to(cond_label, cond_icon, LV_ALIGN_OUT_LEFT_MID, -8, 0);
|
||||
@@ -1138,19 +1134,34 @@ void ui_update(const ha_data_t *data)
|
||||
|
||||
void ui_set_stale(bool stale)
|
||||
{
|
||||
/*
|
||||
* Guarded, because setting an identical style value is NOT free --
|
||||
* lv_obj_set_local_style_prop() calls lv_obj_refresh_style()
|
||||
* unconditionally and invalidates the object. An earlier comment here
|
||||
* claimed the opposite and was wrong; called every poll, it repainted the
|
||||
* temperature for no reason.
|
||||
*/
|
||||
static bool last_stale;
|
||||
static bool last_stale_valid;
|
||||
|
||||
if (last_stale_valid && last_stale == stale) {
|
||||
return;
|
||||
}
|
||||
|
||||
if (!lvgl_port_lock(UI_LOCK_TIMEOUT_MS)) {
|
||||
ESP_LOGW(TAG, "Dropped stale-state update: LVGL lock timed out");
|
||||
return;
|
||||
}
|
||||
|
||||
if (temp_label != NULL) {
|
||||
/* Unlike lv_label_set_text(), setting an identical style value does
|
||||
* not invalidate, so this is safe to call on every poll. */
|
||||
lv_obj_set_style_text_color(temp_label,
|
||||
lv_color_hex(stale ? COL_STALE : COL_TEXT),
|
||||
LV_PART_MAIN);
|
||||
}
|
||||
|
||||
last_stale = stale;
|
||||
last_stale_valid = true;
|
||||
|
||||
lvgl_port_unlock();
|
||||
}
|
||||
|
||||
@@ -1180,12 +1191,37 @@ void ui_set_therm_cb(ui_therm_cmd_cb_t cb)
|
||||
therm_cb = cb;
|
||||
}
|
||||
|
||||
/*
|
||||
* Previous state pushed to the thermostat card. This is not an optimisation,
|
||||
* it is the fix for a real bug: this function is called once a second by the
|
||||
* control task, and lv_obj_set_local_style_prop() does NOT skip identical
|
||||
* values -- it calls lv_obj_refresh_style() unconditionally, which
|
||||
* invalidates. Writing seven styles per second was seven redraws per second
|
||||
* forever, which is what turned the left-edge artefacts from occasional into
|
||||
* permanent.
|
||||
*/
|
||||
static therm_status_t last_therm;
|
||||
static bool last_therm_valid;
|
||||
|
||||
void ui_update_thermostat(const therm_status_t *st)
|
||||
{
|
||||
if (st == NULL) {
|
||||
return;
|
||||
}
|
||||
|
||||
const bool first = !last_therm_valid;
|
||||
|
||||
/* The common case by far: nothing has moved, so touch nothing and do not
|
||||
* even take the lock. */
|
||||
if (!first &&
|
||||
st->state == last_therm.state &&
|
||||
st->mode == last_therm.mode &&
|
||||
st->fan == last_therm.fan &&
|
||||
st->lockout_remaining_s == last_therm.lockout_remaining_s &&
|
||||
(int)lroundf(st->setpoint_f) == (int)lroundf(last_therm.setpoint_f)) {
|
||||
return;
|
||||
}
|
||||
|
||||
if (!lvgl_port_lock(UI_LOCK_TIMEOUT_MS)) {
|
||||
ESP_LOGW(TAG, "Dropped thermostat update: LVGL lock timed out");
|
||||
return;
|
||||
@@ -1193,61 +1229,81 @@ void ui_update_thermostat(const therm_status_t *st)
|
||||
|
||||
char buf[32];
|
||||
|
||||
snprintf(buf, sizeof(buf), "%d°", (int)lroundf(st->setpoint_f));
|
||||
label_set_if_changed(setpoint_label, buf);
|
||||
|
||||
thermostat_state_text(st, buf, sizeof(buf));
|
||||
label_set_if_changed(therm_state_label, buf);
|
||||
|
||||
/*
|
||||
* The dot colour is the at-a-glance channel: amber means the burner is
|
||||
* lit, red means something is wrong and no heat is coming, dim means
|
||||
* everything is fine and there is nothing to do.
|
||||
*/
|
||||
uint32_t dot = COL_TEXT_FAINT;
|
||||
uint32_t text = COL_TEXT_DIM;
|
||||
switch (st->state) {
|
||||
case THERM_STATE_HEATING:
|
||||
dot = COL_ON;
|
||||
text = COL_ON;
|
||||
break;
|
||||
case THERM_STATE_LOCKOUT:
|
||||
dot = COL_ACCENT_WARM;
|
||||
text = COL_ACCENT_WARM;
|
||||
break;
|
||||
case THERM_STATE_NO_SENSOR:
|
||||
dot = 0xE74C3C;
|
||||
text = 0xE74C3C;
|
||||
break;
|
||||
default:
|
||||
break;
|
||||
}
|
||||
lv_obj_set_style_bg_color(therm_state_dot, lv_color_hex(dot), LV_PART_MAIN);
|
||||
lv_obj_set_style_text_color(therm_state_label, lv_color_hex(text), LV_PART_MAIN);
|
||||
|
||||
const bool heat_mode = (st->mode == THERM_MODE_HEAT);
|
||||
label_set_if_changed(mode_btn_label, heat_mode ? "Heat" : "Off");
|
||||
lv_obj_set_style_bg_color(mode_btn,
|
||||
lv_color_hex(heat_mode ? COL_ACCENT_WARM : COL_PANEL_HI),
|
||||
LV_PART_MAIN);
|
||||
lv_obj_set_style_text_color(mode_btn_label,
|
||||
lv_color_hex(heat_mode ? 0x2A1A05 : COL_TEXT_DIM),
|
||||
LV_PART_MAIN);
|
||||
const bool mode_changed = first || st->mode != last_therm.mode;
|
||||
|
||||
const bool fan_on = (st->fan == THERM_FAN_ON);
|
||||
label_set_if_changed(fan_btn_label, fan_on ? "Fan on" : "Fan auto");
|
||||
lv_obj_set_style_bg_color(fan_btn,
|
||||
lv_color_hex(fan_on ? COL_ACCENT : COL_PANEL_HI),
|
||||
LV_PART_MAIN);
|
||||
lv_obj_set_style_text_color(fan_btn_label,
|
||||
lv_color_hex(fan_on ? 0x05131F : COL_TEXT),
|
||||
LV_PART_MAIN);
|
||||
if (first || mode_changed ||
|
||||
(int)lroundf(st->setpoint_f) != (int)lroundf(last_therm.setpoint_f)) {
|
||||
snprintf(buf, sizeof(buf), "%d°", (int)lroundf(st->setpoint_f));
|
||||
label_set_if_changed(setpoint_label, buf);
|
||||
|
||||
/* Setpoint greys out when the mode is off -- it still adjusts, it just
|
||||
* is not controlling anything. */
|
||||
lv_obj_set_style_text_color(setpoint_label,
|
||||
lv_color_hex(heat_mode ? COL_ACCENT_WARM : COL_TEXT_FAINT),
|
||||
LV_PART_MAIN);
|
||||
/* Greys out when the mode is off -- it still adjusts, it just is not
|
||||
* controlling anything. */
|
||||
lv_obj_set_style_text_color(setpoint_label,
|
||||
lv_color_hex(heat_mode ? COL_ACCENT_WARM
|
||||
: COL_TEXT_FAINT),
|
||||
LV_PART_MAIN);
|
||||
}
|
||||
|
||||
/* Text changes every second while a lockout counts down, but it is one
|
||||
* small label rather than the whole card. */
|
||||
if (first || st->state != last_therm.state ||
|
||||
st->lockout_remaining_s != last_therm.lockout_remaining_s) {
|
||||
thermostat_state_text(st, buf, sizeof(buf));
|
||||
label_set_if_changed(therm_state_label, buf);
|
||||
}
|
||||
|
||||
if (first || st->state != last_therm.state) {
|
||||
/*
|
||||
* The dot colour is the at-a-glance channel: amber means the burner
|
||||
* is lit, warm orange means it wants to run but a protection timer
|
||||
* says not yet, red means no heat is coming at all.
|
||||
*/
|
||||
uint32_t dot = COL_TEXT_FAINT;
|
||||
uint32_t text = COL_TEXT_DIM;
|
||||
switch (st->state) {
|
||||
case THERM_STATE_HEATING:
|
||||
dot = COL_ON;
|
||||
text = COL_ON;
|
||||
break;
|
||||
case THERM_STATE_LOCKOUT:
|
||||
dot = COL_ACCENT_WARM;
|
||||
text = COL_ACCENT_WARM;
|
||||
break;
|
||||
case THERM_STATE_NO_SENSOR:
|
||||
dot = 0xE74C3C;
|
||||
text = 0xE74C3C;
|
||||
break;
|
||||
default:
|
||||
break;
|
||||
}
|
||||
lv_obj_set_style_bg_color(therm_state_dot, lv_color_hex(dot), LV_PART_MAIN);
|
||||
lv_obj_set_style_text_color(therm_state_label, lv_color_hex(text), LV_PART_MAIN);
|
||||
}
|
||||
|
||||
if (mode_changed) {
|
||||
label_set_if_changed(mode_btn_label, heat_mode ? "Heat" : "Off");
|
||||
lv_obj_set_style_bg_color(mode_btn,
|
||||
lv_color_hex(heat_mode ? COL_ACCENT_WARM : COL_PANEL_HI),
|
||||
LV_PART_MAIN);
|
||||
lv_obj_set_style_text_color(mode_btn_label,
|
||||
lv_color_hex(heat_mode ? 0x2A1A05 : COL_TEXT_DIM),
|
||||
LV_PART_MAIN);
|
||||
}
|
||||
|
||||
if (first || st->fan != last_therm.fan) {
|
||||
const bool fan_on = (st->fan == THERM_FAN_ON);
|
||||
label_set_if_changed(fan_btn_label, fan_on ? "Fan on" : "Fan auto");
|
||||
lv_obj_set_style_bg_color(fan_btn,
|
||||
lv_color_hex(fan_on ? COL_ACCENT : COL_PANEL_HI),
|
||||
LV_PART_MAIN);
|
||||
lv_obj_set_style_text_color(fan_btn_label,
|
||||
lv_color_hex(fan_on ? 0x05131F : COL_TEXT),
|
||||
LV_PART_MAIN);
|
||||
}
|
||||
|
||||
last_therm = *st;
|
||||
last_therm_valid = true;
|
||||
|
||||
lvgl_port_unlock();
|
||||
}
|
||||
@@ -1280,12 +1336,6 @@ void ui_init(void)
|
||||
lv_obj_set_flex_flow(scr, LV_FLEX_FLOW_COLUMN);
|
||||
lv_obj_remove_flag(scr, LV_OBJ_FLAG_SCROLLABLE);
|
||||
|
||||
/* Created paused; interactions reset-and-resume it. See schedule_repair. */
|
||||
repair_timer = lv_timer_create(repair_timer_cb, 600, NULL);
|
||||
if (repair_timer != NULL) {
|
||||
lv_timer_pause(repair_timer);
|
||||
}
|
||||
|
||||
build_header(scr);
|
||||
build_body(scr);
|
||||
build_lights(scr);
|
||||
|
||||
Reference in new issue
Block a user