[presto] Fix review findings

- Fix tuple touch normalization when pressed state is falsy
- Reconnect WiFi before retrying failed record loads
- Redraw on wake only after successful reconnect
- Remove unused STATE_ERROR and _visible_count state
- Remove ignored row-bottom return from record row drawing
- Share wrapping logic and collapse repeated whitespace
- Align drag threshold handling with scroll redraw delta
- Close jpegdec objects when firmware exposes close()
- Add timeout to wait_for_touch_release()
- Document intentional no-op behavior for record row taps
This commit is contained in:
Claudio Ortolina
2026-05-06 09:50:49 +01:00
parent f4729ba9fc
commit 4558bb3a16
+54 -52
View File
@@ -131,6 +131,7 @@ MONTH_NAMES = [
DEBOUNCE_MS = 300 DEBOUNCE_MS = 300
DRAG_REDRAW_MS = 40 DRAG_REDRAW_MS = 40
DRAG_REDRAW_PX = 8 DRAG_REDRAW_PX = 8
TOUCH_RELEASE_TIMEOUT_MS = 3_000
# WiFi connection timeout (seconds) # WiFi connection timeout (seconds)
WIFI_TIMEOUT = 30 WIFI_TIMEOUT = 30
@@ -157,7 +158,6 @@ view_month = 5
STATE_STARTUP = 0 STATE_STARTUP = 0
STATE_MONTH = 1 STATE_MONTH = 1
STATE_DAY = 2 STATE_DAY = 2
STATE_ERROR = 3
state = STATE_STARTUP state = STATE_STARTUP
# Day view state # Day view state
@@ -166,7 +166,6 @@ records = [] # List of record dicts from API
records_error = False # True if API call failed records_error = False # True if API call failed
# Scroll state # Scroll state
scroll_offset = 0 # Pixel scroll position for day view scroll_offset = 0 # Pixel scroll position for day view
_visible_count = 1 # How many records fit on screen (set during draw)
_dragging = False # True while fast scroll redraws are active _dragging = False # True while fast scroll redraws are active
_content_height = 0 # Cached total record list height _content_height = 0 # Cached total record list height
@@ -236,28 +235,7 @@ def draw_wrapped(text, x, y, max_width, pen, scale=1):
PicoGraphics doesn't support automatic wrapping, so we manually break PicoGraphics doesn't support automatic wrapping, so we manually break
long strings. Returns the Y position after the last line drawn. long strings. Returns the Y position after the last line drawn.
""" """
words = text.split(" ") lines = _wrapped_lines(text, max_width, scale=scale)
lines = []
current_line = ""
for word in words:
test_line = current_line + (" " if current_line else "") + word
w = display.measure_text(test_line, scale=scale)
if w <= max_width:
current_line = test_line
else:
if current_line:
lines.append(current_line)
# If a single word is too long, we draw it anyway (truncation
# would hide information).
if display.measure_text(word, scale=scale) > max_width:
lines.append(word)
current_line = ""
else:
current_line = word
if current_line:
lines.append(current_line)
line_height = 8 * scale + 4 # bitmap8 is 8px tall, plus spacing line_height = 8 * scale + 4 # bitmap8 is 8px tall, plus spacing
cy = y cy = y
@@ -306,10 +284,18 @@ def display_text(text):
def _wrapped_line_count(text, max_width, scale=1): def _wrapped_line_count(text, max_width, scale=1):
"""Return how many wrapped lines the text will occupy.""" """Return how many wrapped lines the text will occupy."""
words = text.split(" ") return max(1, len(_wrapped_lines(text, max_width, scale=scale)))
lines = 0
def _wrapped_lines(text, max_width, scale=1):
"""Return wrapped lines for text using the display font metrics."""
words = str(text).split()
lines = []
current_line = "" current_line = ""
if not words:
return lines
for word in words: for word in words:
test_line = current_line + (" " if current_line else "") + word test_line = current_line + (" " if current_line else "") + word
w = display.measure_text(test_line, scale=scale) w = display.measure_text(test_line, scale=scale)
@@ -317,17 +303,17 @@ def _wrapped_line_count(text, max_width, scale=1):
current_line = test_line current_line = test_line
else: else:
if current_line: if current_line:
lines += 1 lines.append(current_line)
if display.measure_text(word, scale=scale) > max_width: if display.measure_text(word, scale=scale) > max_width:
lines += 1 lines.append(word)
current_line = "" current_line = ""
else: else:
current_line = word current_line = word
if current_line: if current_line:
lines += 1 lines.append(current_line)
return max(1, lines) return lines
# ============================================================================ # ============================================================================
@@ -478,8 +464,8 @@ def wake_display():
_display_awake = True _display_awake = True
if not wifi_connected(): if not wifi_connected():
ensure_wifi_connected() if ensure_wifi_connected():
redraw_current_view() redraw_current_view()
def wifi_connected(): def wifi_connected():
@@ -590,6 +576,17 @@ def _jpeg_scale_options():
return ((full, 1), (half, 2), (quarter, 4), (eighth, 8)) return ((full, 1), (half, 2), (quarter, 4), (eighth, 8))
def _close_jpeg(jpeg):
"""Release a jpegdec object if the firmware exposes close()."""
if jpeg is None:
return
try:
jpeg.close()
except Exception:
pass
def draw_jpeg(data, x, y, max_w, max_h): def draw_jpeg(data, x, y, max_w, max_h):
"""Decode and draw a JPEG image, scaled to fit within max_w x max_h """Decode and draw a JPEG image, scaled to fit within max_w x max_h
and centered in the bounding box. and centered in the bounding box.
@@ -603,6 +600,7 @@ def draw_jpeg(data, x, y, max_w, max_h):
# Try jpegdec module with smart scaling # Try jpegdec module with smart scaling
if _HAS_JPEGDEC: if _HAS_JPEGDEC:
jpeg = None
try: try:
jpeg = _jpegdec_lib.JPEG(display) jpeg = _jpegdec_lib.JPEG(display)
try: try:
@@ -629,14 +627,17 @@ def draw_jpeg(data, x, y, max_w, max_h):
ox = x + (max_w - sw) // 2 ox = x + (max_w - sw) // 2
oy = y + (max_h - sh) // 2 oy = y + (max_h - sh) // 2
jpeg.decode(ox, oy, scale) jpeg.decode(ox, oy, scale)
_close_jpeg(jpeg)
return return
except Exception: except Exception:
pass pass
# Fallback: decode at quarter scale (works for most cover art) # Fallback: decode at quarter scale (works for most cover art)
jpeg.decode(x, y, getattr(_jpegdec_lib, "JPEG_SCALE_QUARTER", 2)) jpeg.decode(x, y, getattr(_jpegdec_lib, "JPEG_SCALE_QUARTER", 2))
_close_jpeg(jpeg)
return return
except Exception: except Exception:
_close_jpeg(jpeg)
pass pass
# Method 2: PicoGraphics built-in JPEG support # Method 2: PicoGraphics built-in JPEG support
@@ -939,7 +940,7 @@ def _draw_day_empty():
def _draw_record_list(): def _draw_record_list():
"""Draw the scrollable list of record rows with cover thumbnails. """Draw the scrollable list of record rows with cover thumbnails.
Uses dynamic row heights so wrapped text doesn't break layout.""" Uses dynamic row heights so wrapped text doesn't break layout."""
global scroll_offset, _visible_count global scroll_offset
max_offset = _max_scroll_offset() max_offset = _max_scroll_offset()
scroll_offset = min(max(0, scroll_offset), max_offset) scroll_offset = min(max(0, scroll_offset), max_offset)
@@ -951,7 +952,6 @@ def _draw_record_list():
display.text("^", WIDTH // 2 - 8, RECORD_START_Y - 28, scale=1) display.text("^", WIDTH // 2 - 8, RECORD_START_Y - 28, scale=1)
cy = RECORD_START_Y - scroll_offset cy = RECORD_START_Y - scroll_offset
visible_count = 0
for rec in records: for rec in records:
row_h = rec.get("_row_height", THUMB_SIZE + 8) row_h = rec.get("_row_height", THUMB_SIZE + 8)
@@ -965,7 +965,6 @@ def _draw_record_list():
break break
_draw_record_row(rec, cy) _draw_record_row(rec, cy)
visible_count += 1
cy = row_bottom cy = row_bottom
# Down arrow if more records below # Down arrow if more records below
@@ -974,10 +973,6 @@ def _draw_record_list():
display.set_font("bitmap14_outline") display.set_font("bitmap14_outline")
display.text("v", WIDTH // 2 - 8, HEIGHT - 24, scale=1) display.text("v", WIDTH // 2 - 8, HEIGHT - 24, scale=1)
# Store how many fit for scroll logic
_visible_count = max(1, visible_count)
def _max_scroll_offset(): def _max_scroll_offset():
"""Return the maximum pixel scroll offset for the current records.""" """Return the maximum pixel scroll offset for the current records."""
viewport_h = HEIGHT - RECORD_START_Y - 28 viewport_h = HEIGHT - RECORD_START_Y - 28
@@ -1088,10 +1083,11 @@ def _record_thumbnail_url(rec):
def _draw_record_row(rec, y): def _draw_record_row(rec, y):
"""Draw a single record row. Returns the Y position after the row """Draw a single record row using its cached layout height."""
(including separator), so the next row can be positioned correctly row_h = rec.get("_row_height", None)
even when title/artist text wraps to multiple lines.""" if row_h is None:
min_h = THUMB_SIZE + ROW_PAD_Y * 2 + ROW_SEPARATOR_H row_h = _record_row_height(rec)
row_bottom = y + row_h
# Fetch and draw thumbnail (top-aligned in row) # Fetch and draw thumbnail (top-aligned in row)
thumb_y = y + ROW_PAD_Y thumb_y = y + ROW_PAD_Y
@@ -1121,10 +1117,6 @@ def _draw_record_row(rec, y):
ty += 2 ty += 2
ty = draw_wrapped(meta_text, TEXT_X, ty, TEXT_W, _pen_dim_text, scale=1) ty = draw_wrapped(meta_text, TEXT_X, ty, TEXT_W, _pen_dim_text, scale=1)
# Bottom of content: at least thumbnail bottom, at least min_h
content_bottom = max(thumb_y + THUMB_SIZE, ty) + ROW_PAD_Y + ROW_SEPARATOR_H
row_bottom = max(y + min_h, content_bottom)
# Separator line at the computed bottom # Separator line at the computed bottom
display.set_pen(_pen_cell_other) display.set_pen(_pen_cell_other)
display.line( display.line(
@@ -1134,8 +1126,6 @@ def _draw_record_row(rec, y):
row_bottom - ROW_SEPARATOR_H row_bottom - ROW_SEPARATOR_H
) )
return row_bottom
# ============================================================================ # ============================================================================
# TOUCH HANDLING # TOUCH HANDLING
@@ -1178,7 +1168,7 @@ def _normalise_touch(point):
if isinstance(point, (tuple, list)): if isinstance(point, (tuple, list)):
if len(point) == 2: if len(point) == 2:
return int(point[0]), int(point[1]) return int(point[0]), int(point[1])
if len(point) >= 3 and point[0]: if len(point) >= 3 and point[0] is not None:
return int(point[1]), int(point[2]) return int(point[1]), int(point[2])
return None return None
@@ -1192,8 +1182,12 @@ def _normalise_touch(point):
def wait_for_touch_release(): def wait_for_touch_release():
"""Block until the active touch is released.""" """Block until the active touch is released."""
start = time.ticks_ms()
while read_touch() is not None: while read_touch() is not None:
if time.ticks_diff(time.ticks_ms(), start) >= TOUCH_RELEASE_TIMEOUT_MS:
return False
time.sleep(0.02) time.sleep(0.02)
return True
def touch_to_calendar_cell(x, y): def touch_to_calendar_cell(x, y):
@@ -1282,9 +1276,17 @@ def handle_day_touch(x, y):
# Tap in error state: retry fetching records # Tap in error state: retry fetching records
if records_error: if records_error:
draw_status("Retrying...") draw_status("Retrying...")
if not ensure_wifi_connected():
set_day_records([], True)
draw_day_view()
return
recs, had_error = fetch_records(view_year, view_month, selected_day) recs, had_error = fetch_records(view_year, view_month, selected_day)
set_day_records(recs, had_error) set_day_records(recs, had_error)
draw_day_view() draw_day_view()
return
# Non-error row taps are intentionally ignored; there is no detail view.
# ============================================================================ # ============================================================================
@@ -1395,7 +1397,7 @@ def main():
break break
current_y = touch_point[1] current_y = touch_point[1]
if abs(current_y - drag_start_y) > 5: if abs(current_y - drag_start_y) >= DRAG_REDRAW_PX:
dragged = True dragged = True
pending_delta += last_drag_y - current_y pending_delta += last_drag_y - current_y
@@ -1414,7 +1416,7 @@ def main():
last_drag_y = current_y last_drag_y = current_y
time.sleep(0.01) time.sleep(0.01)
if pending_delta: if dragged and pending_delta:
max_offset = _max_scroll_offset() max_offset = _max_scroll_offset()
new_offset = min(max(0, scroll_offset + pending_delta), max_offset) new_offset = min(max(0, scroll_offset + pending_delta), max_offset)
if new_offset != scroll_offset: if new_offset != scroll_offset: