Backlog cleanup
This commit is contained in:
+43
@@ -0,0 +1,43 @@
|
||||
---
|
||||
id: ML-171
|
||||
title: >-
|
||||
Fix Oban Web crons page crash: MatchError in sparkline from SQLite datetime
|
||||
string
|
||||
status: Done
|
||||
assignee: []
|
||||
created_date: "2026-05-09 05:28"
|
||||
updated_date: "2026-05-11 13:22"
|
||||
labels:
|
||||
- bug
|
||||
- oban-web
|
||||
- ready
|
||||
dependencies: []
|
||||
references:
|
||||
- >-
|
||||
https://github.com/oban-bg/oban_web/blob/main/lib/oban/web/live/crons/table_component.ex#L78-L85
|
||||
- >-
|
||||
https://github.com/oban-bg/oban_web/blob/main/lib/oban/web/queries/cron_query.ex#L164-L177
|
||||
modified_files:
|
||||
- lib/music_library_web/live/oban/crons/table_component.ex
|
||||
priority: medium
|
||||
---
|
||||
|
||||
## Description
|
||||
|
||||
<!-- SECTION:DESCRIPTION:BEGIN -->
|
||||
|
||||
## Summary
|
||||
|
||||
The `/dev/oban/crons` dashboard page crashes with `Elixir.MatchError` when rendering cron job history sparklines. This happens because Oban Web's `TableComponent.sparkline/1` calls `DateTime.from_naive!/2` which expects a `NaiveDateTime` struct, but receives a raw ISO 8601 string from SQLite.
|
||||
|
||||
This will be resolved when oban_web ships https://github.com/oban-bg/oban_web/commit/9e9b7ad470ac87cfcad67bec9140dc318549dc09
|
||||
|
||||
<!-- SECTION:DESCRIPTION:END -->
|
||||
|
||||
## Acceptance Criteria
|
||||
|
||||
<!-- AC:BEGIN -->
|
||||
|
||||
- [ ] #1 Visiting /dev/oban/crons no longer crashes with MatchError
|
||||
- [ ] #2 Cron history sparklines render correctly showing job state colors
|
||||
<!-- AC:END -->
|
||||
@@ -0,0 +1,40 @@
|
||||
---
|
||||
id: ML-176
|
||||
title: "Presto: add search application with on-screen keyboard"
|
||||
status: Done
|
||||
assignee: []
|
||||
created_date: "2026-05-10 13:34"
|
||||
updated_date: "2026-05-11 06:47"
|
||||
labels:
|
||||
- presto
|
||||
milestone: m-0
|
||||
dependencies:
|
||||
- ML-176.1
|
||||
- ML-176.2
|
||||
priority: medium
|
||||
ordinal: 3000
|
||||
---
|
||||
|
||||
## Description
|
||||
|
||||
<!-- SECTION:DESCRIPTION:BEGIN -->
|
||||
|
||||
Add a search feature to the Presto application that lets users type a query on an on-screen QWERTY keyboard, search the collection via a new API query parameter, and browse results reusing the existing record list and detail views.
|
||||
|
||||
The app is unified with the existing "Records On This Day" calendar: boot shows a splash screen with two entry points — "Search Collection" and "Today's Records".
|
||||
|
||||
<!-- SECTION:DESCRIPTION:END -->
|
||||
|
||||
## Acceptance Criteria
|
||||
|
||||
<!-- AC:BEGIN -->
|
||||
|
||||
- [ ] #1 Splash screen on boot shows 'Search Collection' and 'Today's Records' entry points
|
||||
- [ ] #2 User can search the collection by typing on an on-screen QWERTY keyboard and tapping OK
|
||||
- [ ] #3 Search results display as a scrollable list of records with cover art, title, artist, format, and year
|
||||
- [ ] #4 Tapping a search result opens the record detail view (same as day view detail)
|
||||
- [ ] #5 Back navigation is context-aware: returns to search results from search detail, to day view from day detail
|
||||
- [ ] #6 Existing 'Records On This Day' calendar flow is unchanged
|
||||
- [ ] #7 GET /api/v1/collection accepts optional q parameter for FTS5 full-text search
|
||||
- [ ] #8 Empty query on API returns all records (backward compatible)
|
||||
<!-- AC:END -->
|
||||
+189
@@ -0,0 +1,189 @@
|
||||
---
|
||||
id: ML-176.1
|
||||
title: "API: accept search query parameter on GET /api/v1/collection"
|
||||
status: Done
|
||||
assignee: []
|
||||
created_date: "2026-05-10 13:35"
|
||||
updated_date: "2026-05-11 06:47"
|
||||
labels:
|
||||
- api
|
||||
milestone: m-0
|
||||
dependencies: []
|
||||
references:
|
||||
- lib/music_library/records/search.ex (FTS5 search implementation)
|
||||
- lib/music_library_web/controllers/collection_json.ex (response format)
|
||||
modified_files:
|
||||
- lib/music_library_web/controllers/collection_controller.ex
|
||||
parent_task_id: ML-176
|
||||
priority: medium
|
||||
---
|
||||
|
||||
## Description
|
||||
|
||||
<!-- SECTION:DESCRIPTION:BEGIN -->
|
||||
|
||||
Extend the existing `GET /api/v1/collection` endpoint to accept an optional `q` query parameter for full-text search.
|
||||
|
||||
When `q` is present and non-empty, pass it to `Collection.search_records/2` instead of the current empty-string default. When absent or empty, preserve existing behavior (return all collection records).
|
||||
|
||||
The existing FTS5 search in `Records.Search` handles the query — no new search logic is needed. It searches across all indexed fields (title, artists, genres) and supports the `artist:`, `album:`, etc. structured syntax, though the Presto client will only send plain text.
|
||||
|
||||
**Response format**: unchanged — the existing `CollectionJSON.index/1` already returns `{total, limit, offset, records}` with all fields the Presto needs.
|
||||
|
||||
**Key decisions** (from user):
|
||||
|
||||
- Collection-only scope (purchased_at IS NOT NULL) — already the default for this endpoint
|
||||
- Alphabetical ordering — already the default
|
||||
- Plain text query — passed as-is to FTS5
|
||||
- Always returns paginated (limit/offset) — existing behavior
|
||||
<!-- SECTION:DESCRIPTION:END -->
|
||||
|
||||
## Acceptance Criteria
|
||||
|
||||
<!-- AC:BEGIN -->
|
||||
|
||||
- [x] #1 GET /api/v1/collection returns all records when q is absent (existing behavior unchanged)
|
||||
- [x] #2 GET /api/v1/collection?q=beatles returns only records matching FTS5 query for 'beatles'
|
||||
- [x] #3 GET /api/v1/collection?q= returns all records (empty string treated as no query)
|
||||
- [x] #4 GET /api/v1/collection?q=beatles&limit=5&offset=0 respects pagination with search
|
||||
- [x] #5 Response format matches existing schema: {total, limit, offset, records} with all record fields
|
||||
- [x] #6 Controller test covers: no q param, empty q, populated q, q with no matches, q with pagination
|
||||
<!-- AC:END -->
|
||||
|
||||
## Implementation Plan
|
||||
|
||||
<!-- SECTION:PLAN:BEGIN -->
|
||||
|
||||
## Implementation Plan
|
||||
|
||||
### 1. Objective alignment
|
||||
|
||||
Extend `GET /api/v1/collection` to accept an optional `q` query parameter for FTS5 full-text search. When `q` is present and non-empty, pass it to the existing `Collection.search_records/2` and `search_records_count/1` pipeline. When absent or empty, preserve the current behavior of returning all collection records. No new search logic is required — `Records.Search` already handles both empty and populated queries.
|
||||
|
||||
### 2. Simplicity and alternatives considered
|
||||
|
||||
**Chosen approach: extract `q` from conn params in the controller, pass it directly.**
|
||||
|
||||
The controller's `index/2` action currently hardcodes `""` as the query. Changing it to read `params["q"]` and normalize empty/absent to `""` is a ~5-line change. The entire search pipeline (`Collection.search_records/2` → `Records.Search.search_records/3` → `build_search/3`) already supports passing a query string and treats `{:query, ""}` as a no-op.
|
||||
|
||||
**Alternatives considered and rejected:**
|
||||
|
||||
- **New dedicated endpoint (`GET /api/v1/collection/search`)**: Adds a route and controller action for no benefit. The existing endpoint already returns searchable data — adding a query parameter is simpler and REST-idiomatic.
|
||||
- **Separate search action in the controller**: Would duplicate the pagination and render logic already in `index/2`. The only difference is the `q` value — a parameter is cleaner.
|
||||
- **Validation/sanitization layer**: The `Records.Search` module already handles FTS5 escaping via `fts_query_escape/1` (splitting on whitespace, escaping quotes, wrapping in double-quote syntax with `*` suffix). No additional sanitization is needed.
|
||||
- **New LiveView action/template**: Not applicable. This is API-only, consumed by the Presto client.
|
||||
|
||||
### 3. Completeness and sequencing
|
||||
|
||||
All steps are independent and can be done in a single pass:
|
||||
|
||||
1. **Extract `q` param in `CollectionController.index/2`** — Add `query = normalize_query(params["q"])` before the existing `limit`/`offset` extraction, and pass `query` to `search_records_count/1` and `search_records/2` instead of `""`. Add a private `normalize_query/1` helper that returns `""` for `nil` or empty string input.
|
||||
|
||||
2. **Add tests to `collection_controller_test.exs`** — Six test cases in the `describe "GET /api/v1/collection"` block:
|
||||
- Existing test unchanged (no `q` param → returns all records)
|
||||
- `q` with matching text → returns filtered results
|
||||
- Empty `q` string → returns all records (same as absent)
|
||||
- `q` with no matches → returns `total: 0`, empty `records`
|
||||
- `q` with pagination (`limit`/`offset`) → respects both search and pagination
|
||||
- Response shape validation → confirms the schema is unchanged
|
||||
|
||||
3. **Run the full test suite** — `mix test test/music_library_web/controllers/collection_controller_test.exs`
|
||||
|
||||
### 4. Verifiability
|
||||
|
||||
| Step | Verification |
|
||||
| ------------------- | ----------------------------------------------------------------------------------------------------------------- |
|
||||
| Controller change | `mix test test/music_library_web/controllers/collection_controller_test.exs` — all tests pass |
|
||||
| No regression | `mix test` full suite — all tests pass |
|
||||
| Manual API check | `curl -H "Authorization: Bearer <token>" "localhost:4002/api/v1/collection?q=beatles"` returns filtered results |
|
||||
| Empty query | `curl ... "localhost:4002/api/v1/collection?q="` returns all records |
|
||||
| Pagination + search | `curl ... "localhost:4002/api/v1/collection?q=beatles&limit=1&offset=0"` returns correct count with single result |
|
||||
|
||||
### 5. Architecture impact analysis
|
||||
|
||||
| Touchpoint | Impact |
|
||||
| --------------------------------------------------- | ------------------------------------------------------------------------------------- |
|
||||
| `MusicLibraryWeb.CollectionController` (controller) | **Changed** — `index/2` reads `q` param and a new `normalize_query/1` helper is added |
|
||||
| `MusicLibrary.Collection` (context) | **Unchanged** — already accepts query string |
|
||||
| `MusicLibrary.Records.Search` (search module) | **Unchanged** — already handles empty queries |
|
||||
| `MusicLibraryWeb.CollectionJSON` (JSON view) | **Unchanged** — response format is identical |
|
||||
| Router (`router.ex`) | **Unchanged** — query params don't affect routing |
|
||||
| Schemas (`SearchIndex`, `Record`) | **Unchanged** — no schema changes |
|
||||
| PubSub | **Unchanged** — no broadcast events involved |
|
||||
| Supervision tree | **Unchanged** — no new processes |
|
||||
| External APIs | **Unchanged** — no external calls |
|
||||
| Presto client | **Unchanged** — the client already sends requests; the `q` param is opt-in |
|
||||
|
||||
### 6. Performance profile
|
||||
|
||||
- **Query complexity**: Unchanged from the existing FTS5 path. The query already runs `records_search_index MATCH <escaped_query>` with the same LIMIT/OFFSET pattern. An empty `q` produces the same query plan as today (no FTS clause → full scan with LIMIT/OFFSET on `purchased_at IS NOT NULL`).
|
||||
- **N+1 risk**: None. `SearchIndex` is a pre-joined materialization that includes all needed fields (artists, genres as JSON). The `essential_fields()` macro selects all columns in a single query.
|
||||
- **Memory**: Unchanged. Pagination caps results at 20 records by default, same as today.
|
||||
- **Latency**: FTS5 queries on the `records_search_index` table are sub-millisecond for typical collection sizes. The index is maintained automatically by SQLite triggers.
|
||||
|
||||
### 7. Benchmarking requirements
|
||||
|
||||
No benchmarks needed. This change does not introduce new query patterns — it reuses the existing FTS5 search path. The performance characteristics are identical whether `q` is empty or populated (both paths already exist in `build_search/3`).
|
||||
|
||||
### 8. Cost profile
|
||||
|
||||
No cost impact. No external API calls, no additional storage, no compute changes. The FTS5 search runs entirely within the local SQLite database.
|
||||
|
||||
### 9. Production infrastructure steps
|
||||
|
||||
**No production changes required.** The route is unchanged (query parameters don't affect routing), no new environment variables, no migrations, no service provisioning. The change is a standard code deploy.
|
||||
|
||||
### 10. Documentation updates
|
||||
|
||||
No documentation updates needed. The API endpoint already exists in `router.ex`. The `q` parameter is additive and optional — existing API consumers are unaffected. If project API docs exist externally, note that `GET /api/v1/collection` now accepts `?q=<search term>` as an optional parameter, but this is self-documenting via the REST pattern.
|
||||
|
||||
<!-- SECTION:PLAN:END -->
|
||||
|
||||
## Implementation Notes
|
||||
|
||||
<!-- SECTION:NOTES:BEGIN -->
|
||||
|
||||
Implementation complete. Changes:
|
||||
|
||||
1. **Controller** (`lib/music_library_web/controllers/collection_controller.ex`):
|
||||
- Added `query = normalize_query(params["q"])` in `index/2`
|
||||
- Pass `query` to `search_records_count/1` and `search_records/2` instead of hardcoded `""`
|
||||
- Added private `normalize_query/1` helper: nil → "", "" → "", otherwise passthrough
|
||||
|
||||
2. **Tests** (`test/music_library_web/controllers/collection_controller_test.exs`):
|
||||
- 5 new tests added:
|
||||
- `q` param matches → returns filtered results
|
||||
- Empty `q` string → returns all records
|
||||
- `q` with no matches → returns `total: 0`, empty `records`
|
||||
- `q` with pagination → respects limit/offset on filtered results
|
||||
- Response shape validation → confirms schema is unchanged
|
||||
|
||||
3. **Full test suite**: 968 passed (0 failures), no regressions
|
||||
<!-- SECTION:NOTES:END -->
|
||||
|
||||
## Final Summary
|
||||
|
||||
<!-- SECTION:FINAL_SUMMARY:BEGIN -->
|
||||
|
||||
## Summary
|
||||
|
||||
Extended `GET /api/v1/collection` to accept an optional `q` query parameter for FTS5 full-text search.
|
||||
|
||||
### Changes
|
||||
|
||||
**Controller** (`lib/music_library_web/controllers/collection_controller.ex`):
|
||||
|
||||
- `index/2` now reads `params["q"]` and normalizes nil/empty → `""` via a new private `normalize_query/1` helper
|
||||
- Passes the query string to the existing `Collection.search_records_count/1` and `Collection.search_records/2` pipeline (previously hardcoded `""`)
|
||||
|
||||
**Tests** (`test/music_library_web/controllers/collection_controller_test.exs`):
|
||||
|
||||
- 5 new tests: matching query, empty query string, no-match query, pagination with search, and response shape validation
|
||||
|
||||
### Key points
|
||||
|
||||
- No new search logic — reuses existing `Records.Search.build_search/3` which already handled empty queries as a no-op
|
||||
- Response format unchanged — `CollectionJSON.index/1` returns the same shape
|
||||
- No route/schema/migration changes required
|
||||
- Full suite: 968 passed, 0 regressions
|
||||
<!-- SECTION:FINAL_SUMMARY:END -->
|
||||
+175
@@ -0,0 +1,175 @@
|
||||
---
|
||||
id: ML-176.2
|
||||
title: "Presto: add splash screen, search views, and on-screen keyboard"
|
||||
status: Done
|
||||
assignee: []
|
||||
created_date: "2026-05-10 13:35"
|
||||
updated_date: "2026-05-11 06:47"
|
||||
labels:
|
||||
- presto
|
||||
milestone: m-0
|
||||
dependencies:
|
||||
- ML-176.1
|
||||
references:
|
||||
- presto/AGENTS.md
|
||||
- presto/README.md
|
||||
- >-
|
||||
doc-17 -
|
||||
Implementation-Plan-ML-176.2-—-Presto-splash-screen-search-views-and-on-screen-keyboard
|
||||
modified_files:
|
||||
- presto/records_on_the_day.py
|
||||
parent_task_id: ML-176
|
||||
priority: medium
|
||||
ordinal: 2000
|
||||
---
|
||||
|
||||
## Description
|
||||
|
||||
<!-- SECTION:DESCRIPTION:BEGIN -->
|
||||
|
||||
Refactor `presto/records_on_the_day.py` into a unified app with a splash screen, search capability, and the existing calendar. All work is in the single MicroPython file.
|
||||
|
||||
## New states
|
||||
|
||||
- `STATE_HOME` (new, default on boot/wake)
|
||||
- `STATE_SEARCH_INPUT` (new — search box + on-screen keyboard)
|
||||
- `STATE_SEARCH_RESULTS` (new — results list, reuses day-view rendering)
|
||||
- `STATE_MONTH` / `STATE_DAY` / `STATE_RECORD` (existing, unchanged logic)
|
||||
|
||||
## Splash screen (STATE_HOME)
|
||||
|
||||
- Two large touch targets: "Search Collection" and "Today's Records"
|
||||
- "Today's Records" → transitions to existing `STATE_MONTH` / today's records flow
|
||||
- "Search Collection" → transitions to `STATE_SEARCH_INPUT`
|
||||
- On boot and display wake: always show `STATE_HOME`
|
||||
|
||||
## On-screen keyboard (part of STATE_SEARCH_INPUT)
|
||||
|
||||
- 3-row QWERTY layout: qwertyuiop / asdfghjkl / zxcvbnm
|
||||
- Bottom row: [123 toggle] [space] [⌫ backspace] [OK]
|
||||
- Numbers keyboard: 1234567890 / symbols row / [ABC toggle] [space] [⌫] [OK]
|
||||
- Space and ⌫ are shared between both layouts
|
||||
- Lowercase only, no shift/caps
|
||||
- OK button disabled (dimmed) when query is empty
|
||||
- Each key is a rounded rectangle, touch-mapped to append/delete chars
|
||||
- Visual feedback on key press (brief highlight)
|
||||
- Query buffer displayed in a text field area above the keyboard, max ~50 chars
|
||||
|
||||
## Search triggering
|
||||
|
||||
- User types query, presses OK
|
||||
- OK calls `fetch_search_results(query)` → API
|
||||
- Transitions to `STATE_SEARCH_RESULTS`
|
||||
|
||||
## Search results (STATE_SEARCH_RESULTS)
|
||||
|
||||
- Reuses existing `_draw_record_list()`, scroll, and row rendering from day view
|
||||
- Header shows "Search: <query>" with Back button
|
||||
- Back returns to `STATE_SEARCH_INPUT` (preserving query so user can refine)
|
||||
- Tapping a record → `STATE_RECORD` (detail view, unchanged)
|
||||
- Back from detail → returns to search results
|
||||
- Empty results: "No records found" full-screen message
|
||||
- Error: "Could not reach server" with retry-on-tap (same as day view)
|
||||
- 20 results per page, no infinite scroll
|
||||
|
||||
## API client
|
||||
|
||||
- New `fetch_search_results(query)` function
|
||||
- Calls `GET /api/v1/collection?q=<url-encoded-query>&limit=20`
|
||||
- Parses `{total, records, ...}` response
|
||||
- Returns `(records_list, error_flag)` tuple (same shape as `fetch_records`)
|
||||
|
||||
## Navigation state
|
||||
|
||||
- Track previous state to know where "Back" in detail view should go
|
||||
- Detail Back → search results when entered from search
|
||||
- Detail Back → day view when entered from day
|
||||
- Month view and search views both have "Back to Home"
|
||||
|
||||
## Constraints
|
||||
|
||||
- Must reuse existing record row rendering (`_draw_record_row`), detail view (`draw_record_detail`), thumbnail fetching, WiFi, NTP, display sleep logic
|
||||
- Scroll performance rules from AGENTS.md still apply
|
||||
- Memory: no large buffers, keep GC pressure low
|
||||
<!-- SECTION:DESCRIPTION:END -->
|
||||
|
||||
## Acceptance Criteria
|
||||
|
||||
<!-- AC:BEGIN -->
|
||||
|
||||
- [ ] #1 On boot, splash screen shows with 'Search Collection' and 'Today's Records' buttons
|
||||
- [ ] #2 Tapping 'Today's Records' opens the existing month calendar (unchanged behavior)
|
||||
- [ ] #3 Tapping 'Search Collection' opens search input view with on-screen QWERTY keyboard
|
||||
- [ ] #4 Keyboard: tapping letter keys builds query string shown in input field
|
||||
- [ ] #5 Keyboard: backspace removes last character
|
||||
- [ ] #6 Keyboard: 123/ABC toggle switches between QWERTY and numbers keyboard layouts
|
||||
- [ ] #7 Keyboard: OK button is visually disabled (dimmed) when query is empty
|
||||
- [ ] #8 Keyboard: OK button triggers API search when query is non-empty
|
||||
- [ ] #9 Search results display using same record row layout as day view (cover, title, artist, format, year)
|
||||
- [ ] #10 Search results: Back button returns to search input view with query preserved
|
||||
- [ ] #11 Search results: tapping a record opens detail view
|
||||
- [ ] #12 Detail view Back returns to search results when entered from search
|
||||
- [ ] #13 Detail view Back returns to day view when entered from day
|
||||
- [ ] #14 Empty results show 'No records found' full-screen message
|
||||
- [ ] #15 API error shows 'Could not reach server' with retry-on-tap
|
||||
- [ ] #16 Display sleep/wake always returns to splash screen
|
||||
- [ ] #17 Scroll in search results works identically to day view scroll
|
||||
- [ ] #18 Keyboard renders correctly at 720×720: all keys visible and tappable
|
||||
- [ ] #19 Back-to-Home navigation available from month view and search views
|
||||
<!-- AC:END -->
|
||||
|
||||
## Implementation Notes
|
||||
|
||||
<!-- SECTION:NOTES:BEGIN -->
|
||||
|
||||
## Implementation complete (~617 new lines, 2450 total)
|
||||
|
||||
### State constants + globals
|
||||
|
||||
- Renumbered STATE_MONTH=0, STATE_DAY=1, STATE_RECORD=2. Added STATE_HOME=3, STATE_SEARCH_INPUT=4, STATE_SEARCH_RESULTS=5. Removed STATE_STARTUP.
|
||||
- New globals: `search_query`, `search_results`, `search_results_error`, `search_scroll_offset`, `_search_content_height`, `keyboard_mode`, `previous_state`.
|
||||
|
||||
### Splash screen (STATE_HOME)
|
||||
|
||||
- `draw_home_screen()` with "Search Collection" and "Today's Records" buttons. Tapping "Today's Records" → month view with today highlighted. Tapping "Search Collection" → search input.
|
||||
|
||||
### On-screen keyboard (STATE_SEARCH_INPUT)
|
||||
|
||||
- QWERTY layout (3 rows) + numbers/symbols. Bottom row: [123/ABC] [Space] [Backspace] [OK]. OK dimmed when query empty. `_keyboard_hit_test()` maps touch to action. `handle_search_input_touch()` dispatches all actions. Visual feedback via `_flash_key()`.
|
||||
|
||||
### Search results (STATE_SEARCH_RESULTS)
|
||||
|
||||
- Reuses `_draw_record_row()`. Own scroll state (`search_scroll_offset`, `_search_content_height`). Header: "Search: <query>" with truncation. Empty: "No records found". Error: "Could not reach server" with retry-on-tap.
|
||||
|
||||
### API client
|
||||
|
||||
- `fetch_search_results(query)` → `GET /api/v1/collection?q=<encoded>&limit=20`. Returns `(records_list, error_flag)`.
|
||||
|
||||
### Generic display pipeline
|
||||
|
||||
- `prepare_records_for_display(recs=None)` and `preload_record_thumbnails(recs=None)` accept optional list. Height cache tracked per source.
|
||||
|
||||
### Navigation tracking
|
||||
|
||||
- `previous_state` tracks detail origin (STATE_DAY / STATE_SEARCH_RESULTS). Back → correct origin. `draw_record_detail()` reads from `search_results` or `records`.
|
||||
|
||||
### Back-to-Home
|
||||
|
||||
- Month header gains "H" Home button. `wake_display()` always sets `state = STATE_HOME`. Search input header Back → Home.
|
||||
|
||||
### Boot flow
|
||||
|
||||
- Starts at `draw_home_screen()`, no auto-fetch. User taps a button.
|
||||
|
||||
### Main loop
|
||||
|
||||
- New dispatches for STATE_HOME, STATE_SEARCH_INPUT, STATE_SEARCH_RESULTS (drag scroll mirrors day view).
|
||||
|
||||
### Touch helpers
|
||||
|
||||
- `touch_to_record_index()` accepts optional `recs` and `offset` params.
|
||||
|
||||
### Verification
|
||||
|
||||
- Syntax check passes (`python3 py_compile`). No `STATE_STARTUP` references remain.
|
||||
<!-- SECTION:NOTES:END -->
|
||||
+562
@@ -0,0 +1,562 @@
|
||||
---
|
||||
id: ML-178
|
||||
title: Add scrobble button to Presto record detail view with "Done" state
|
||||
status: Done
|
||||
assignee: []
|
||||
created_date: "2026-05-10 18:57"
|
||||
updated_date: "2026-05-11 06:47"
|
||||
labels:
|
||||
- presto
|
||||
dependencies:
|
||||
- ML-177
|
||||
references:
|
||||
- presto/main.py
|
||||
- presto/AGENTS.md
|
||||
- presto/README.md
|
||||
priority: medium
|
||||
ordinal: 5000
|
||||
---
|
||||
|
||||
## Description
|
||||
|
||||
<!-- SECTION:DESCRIPTION:BEGIN -->
|
||||
|
||||
Add a scrobble button to the record detail view (`draw_record_detail()` / `STATE_RECORD`) in the Presto MicroPython app. The button sends a POST request to `/api/v1/collection/:record_id/scrobble` and shows "Done" on success.
|
||||
|
||||
## Context
|
||||
|
||||
The Presto app (`presto/main.py`) browses the music collection via the REST API. It has three views: month calendar, day list, and record detail. The record detail view (`STATE_RECORD` → `draw_record_detail()`) currently shows: header with back button, title/artists, large cover image, genres, metadata (type|format|year), and purchase date.
|
||||
|
||||
This task adds a scrobble button **at the bottom** of the record detail view, after all existing content. The button is only shown when the record has a `selected_release_id` (fetched from the API response — this field is added by backend task ML-177).
|
||||
|
||||
## Design decisions
|
||||
|
||||
- The button is a rectangular button with centered text, similar to the existing "Back" button in the header
|
||||
- Button text states: "Scrobble" (initial), "..." (loading/request in progress), "Done" (success), revert to "Scrobble" on error
|
||||
- The button is only drawn when `rec.get("selected_release_id")` is truthy
|
||||
- Uses `urequests.post()` to call the API with the stored `API_TOKEN` in the `Authorization` header
|
||||
- On HTTP error or network failure, the button reverts to "Scrobble" (no toast — the Presto has no toast system, but could show an error briefly)
|
||||
- Scrobble state is per-record and resets when navigating away
|
||||
|
||||
## Layout constants needed
|
||||
|
||||
- `SCROBBLE_BUTTON_W` — button width (suggest centered, e.g., 200px)
|
||||
- `SCROBBLE_BUTTON_H` — button height
|
||||
- `SCROBBLE_BUTTON_Y` — computed from the bottom of the detail content
|
||||
- Colors: `_pen_scrobble_bg`, `_pen_scrobble_text`, `_pen_scrobble_done_bg`, `_pen_scrobble_done_text`
|
||||
|
||||
## Touch handling
|
||||
|
||||
In the `STATE_RECORD` touch dispatch, check if the tap falls within the scrobble button bounds. If so:
|
||||
|
||||
1. Start the HTTP request (blocking — use a brief timeout)
|
||||
2. Update the button state
|
||||
3. Redraw
|
||||
|
||||
## Out of scope
|
||||
|
||||
- No changes to the backend API (covered by ML-177)
|
||||
- No toast/error display system
|
||||
- No debounce or double-tap prevention (can be added later)
|
||||
<!-- SECTION:DESCRIPTION:END -->
|
||||
|
||||
## Acceptance Criteria
|
||||
|
||||
<!-- AC:BEGIN -->
|
||||
|
||||
- [x] #1 The scrobble button appears at the bottom of the record detail view only when the record has a non-null selected_release_id
|
||||
- [x] #2 The scrobble button is not shown for records without a selected_release_id
|
||||
- [x] #3 Tapping the button sends a POST to /api/v1/collection/:record_id/scrobble with the Bearer token from secrets
|
||||
- [x] #4 While the HTTP request is in progress, the button shows a loading indicator (e.g., "..." text)
|
||||
- [x] #5 On 200 response, the button displays "Done"
|
||||
- [x] #6 On non-200 response or network error, the button reverts to "Scrobble"
|
||||
- [x] #7 The Presto app can still be deployed with `mise run presto`
|
||||
- [x] #8 The `selected_release_id` field from the on_this_day API response is used to gate button visibility
|
||||
- [x] #9 Scrolling in the detail view still works correctly (the button scrolls with the content)
|
||||
<!-- AC:END -->
|
||||
|
||||
## Implementation Plan
|
||||
|
||||
<!-- SECTION:PLAN:BEGIN -->
|
||||
|
||||
## Objective alignment
|
||||
|
||||
Add a scrobble button to the Presto record detail view (`draw_record_detail()` / `STATE_RECORD`) that sends `POST /api/v1/collection/:record_id/scrobble` and displays a visual "Done" confirmation on success. The button gates on the `selected_release_id` field (added by ML-177), so only scrobble-eligible records show the button.
|
||||
|
||||
This maps directly to the problem: the Presto device can browse records but has no way to trigger a Last.fm scrobble. The REST endpoint (ML-177) provides the server-side action; this task wires it to a touch target on the device.
|
||||
|
||||
## Simplicity and alternatives considered
|
||||
|
||||
**Chosen approach:** A single rectangular button appended to the bottom of the scrollable detail content, with a blocking HTTP request on tap and three visual states (idle / loading / done). The "Done" state persists until the user navigates away (no auto-revert), keeping the implementation simple and avoiding additional blocking sleeps.
|
||||
|
||||
**Alternatives evaluated and rejected:**
|
||||
|
||||
1. **Fixed-position button (always visible at screen bottom):** Simpler to implement (no scroll height recalculation, no hit-test complexity). Rejected because (a) it consumes ~50px of the 720px display permanently, (b) the Presto has no concept of a "sticky footer" — the existing scrolling + clip infrastructure would need rework for a fixed overlay, and (c) a button that scrolls with content is more discoverable and feels like part of the record.
|
||||
|
||||
2. **Non-blocking / async HTTP request:** Ideal UX but MicroPython on Presto does not support `_thread` or `asyncio` in a usable way. The blocking approach with a brief timeout is the pragmatic trade-off. The "..." loading state provides visual feedback during the blocking call. **Note:** MicroPython's `urequests` has limited timeout support. If WiFi drops mid-request, the default timeout can be 30+ seconds, freezing the display. This is an acceptable trade-off for a user-initiated action on a device that rarely loses connectivity, but should be tested on-device with a simulated WiFi disconnect to understand the real-world behavior.
|
||||
|
||||
3. **Toast / error display system:** Would improve error feedback but is explicitly out of scope for this task. The plan uses a minimal error-revert (button returns to "Scrobble" on failure) which is sufficient.
|
||||
|
||||
4. **Auto-revert "Done" to "Scrobble" after a delay:** Would reset the button to allow re-scrobbling. Rejected because (a) it adds a `time.sleep(1.5)` on top of the already-blocking HTTP request, (b) the scrobble state already resets on navigation (Step 3), and (c) keeping "Done" visible provides clearer feedback that the action succeeded. Users who want to re-scrobble can navigate away and back.
|
||||
|
||||
## Completeness and sequencing
|
||||
|
||||
The following steps are ordered by dependency. Each step must be completed and verified before the next.
|
||||
|
||||
### Step 1: Add layout constants
|
||||
|
||||
**File:** `presto/main.py`, in the "Detail view" constants block (near `DETAIL_TEXT_W`)
|
||||
|
||||
Add:
|
||||
|
||||
```python
|
||||
# Scrobble button (detail view)
|
||||
SCROBBLE_BUTTON_W = 200
|
||||
SCROBBLE_BUTTON_H = 40
|
||||
SCROBBLE_BUTTON_GAP = 12 # vertical gap before button
|
||||
```
|
||||
|
||||
Also add color constants in the "Colors" block (near `STATUS_TEXT`):
|
||||
|
||||
```python
|
||||
SCROBBLE_BG = (55, 65, 81) # Same slate as HOME_BTN_BG
|
||||
SCROBBLE_TEXT = (228, 228, 231) # Same as TITLE_COLOR
|
||||
SCROBBLE_DONE_BG = (34, 197, 94) # Green success
|
||||
SCROBBLE_DONE_TEXT = (255, 255, 255)
|
||||
```
|
||||
|
||||
**Verification:** `python3 -c "import py_compile; py_compile.compile('presto/main.py', doraise=True)"` passes without error.
|
||||
|
||||
### Step 2: Add new pens
|
||||
|
||||
**File:** `presto/main.py`
|
||||
|
||||
Add module-level pen variables near existing `_pen_home_btn`:
|
||||
|
||||
```python
|
||||
_pen_scrobble_bg = None
|
||||
_pen_scrobble_text = None
|
||||
_pen_scrobble_done_bg = None
|
||||
_pen_scrobble_done_text = None
|
||||
```
|
||||
|
||||
Initialize in `_init_pens()`:
|
||||
|
||||
```python
|
||||
_pen_scrobble_bg = display.create_pen(*SCROBBLE_BG)
|
||||
_pen_scrobble_text = display.create_pen(*SCROBBLE_TEXT)
|
||||
_pen_scrobble_done_bg = display.create_pen(*SCROBBLE_DONE_BG)
|
||||
_pen_scrobble_done_text = display.create_pen(*SCROBBLE_DONE_TEXT)
|
||||
```
|
||||
|
||||
Also add these pens to the `global` declarations in `_init_pens()`.
|
||||
|
||||
**Verification:** Syntax check passes. No pen-related `NameError` when running on device.
|
||||
|
||||
### Step 3: Add global scrobble state
|
||||
|
||||
**File:** `presto/main.py`, in the "GLOBAL STATE" block (near `detail_scroll_offset`)
|
||||
|
||||
Add:
|
||||
|
||||
```python
|
||||
# Scrobble button state (per-record-detail-visit; resets on navigation)
|
||||
_scrobble_state = "idle" # "idle" | "loading" | "done"
|
||||
```
|
||||
|
||||
Add `_scrobble_state` to the `global` declarations in `main()`.
|
||||
|
||||
Reset `_scrobble_state = "idle"` in all entry points to `STATE_RECORD` and all exit points:
|
||||
|
||||
**Entry point 1 — STATE_DAY → STATE_RECORD (in `main()`, inside the `if not dragged:` block after the day-view drag loop, alongside `detail_scroll_offset = 0`):**
|
||||
|
||||
```python
|
||||
detail_scroll_offset = 0
|
||||
_scrobble_state = "idle"
|
||||
```
|
||||
|
||||
**Entry point 2 — STATE_SEARCH_RESULTS → STATE_RECORD (in `handle_search_results_touch()`, alongside `detail_scroll_offset = 0`):**
|
||||
|
||||
```python
|
||||
detail_scroll_offset = 0
|
||||
_scrobble_state = "idle"
|
||||
```
|
||||
|
||||
**Exit point — STATE_RECORD back-button handler (in `main()`, alongside `detail_scroll_offset = 0`):**
|
||||
|
||||
```python
|
||||
detail_scroll_offset = 0
|
||||
_scrobble_state = "idle"
|
||||
```
|
||||
|
||||
**Verification:** Syntax check passes. Code review: verify that every code path that enters `STATE_RECORD` sets `_scrobble_state = "idle"`, and every code path that leaves `STATE_RECORD` also sets it. There are exactly 2 entry paths and 1 exit path as enumerated above.
|
||||
|
||||
### Step 4: Modify `_measure_detail_content()` to include button height
|
||||
|
||||
**File:** `presto/main.py`, function `_measure_detail_content()`
|
||||
|
||||
At the end of the function, before `_detail_content_height = h`, add:
|
||||
|
||||
```python
|
||||
# Scrobble button (at bottom, only if scrobble-eligible)
|
||||
if rec.get("selected_release_id"):
|
||||
h += SCROBBLE_BUTTON_GAP
|
||||
h += SCROBBLE_BUTTON_H
|
||||
```
|
||||
|
||||
**Verification:** On-device test — navigate to a record with `selected_release_id` set (after ML-177 is deployed). The scroll viewport should accommodate the extra button height. The `^` / `v` scroll indicators should appear/disappear correctly. Records without `selected_release_id` should have the same scroll behavior as before. For offline testing before ML-177 is deployed, temporarily hard-code `selected_release_id` to a known UUID string on a test record dict.
|
||||
|
||||
### Step 5: Add `_draw_scrobble_button()` helper
|
||||
|
||||
**File:** `presto/main.py`, in the "DRAWING: RECORD DETAIL" section
|
||||
|
||||
New function:
|
||||
|
||||
```python
|
||||
def _draw_scrobble_button(rec, y):
|
||||
"""Draw the scrobble button at (x, y). Button is horizontally centered.
|
||||
Visual state depends on global _scrobble_state."""
|
||||
global _scrobble_state
|
||||
|
||||
bx = (WIDTH - SCROBBLE_BUTTON_W) // 2
|
||||
|
||||
if _scrobble_state == "done":
|
||||
bg_pen = _pen_scrobble_done_bg
|
||||
text_pen = _pen_scrobble_done_text
|
||||
label = "Done"
|
||||
elif _scrobble_state == "loading":
|
||||
bg_pen = _pen_scrobble_bg
|
||||
text_pen = _pen_scrobble_text
|
||||
label = "..."
|
||||
else:
|
||||
bg_pen = _pen_scrobble_bg
|
||||
text_pen = _pen_scrobble_text
|
||||
label = "Scrobble"
|
||||
|
||||
display.set_pen(bg_pen)
|
||||
display.rectangle(bx, y, SCROBBLE_BUTTON_W, SCROBBLE_BUTTON_H)
|
||||
|
||||
display.set_pen(text_pen)
|
||||
display.set_font("bitmap8")
|
||||
_draw_centered_text(label, bx, y, SCROBBLE_BUTTON_W, SCROBBLE_BUTTON_H, 8)
|
||||
```
|
||||
|
||||
**Verification:** Syntax check passes. Helper is callable but not yet wired in.
|
||||
|
||||
### Step 6: Modify `draw_record_detail()` to draw the button
|
||||
|
||||
**File:** `presto/main.py`, function `draw_record_detail()`
|
||||
|
||||
**Important:** The local variable `y` in `draw_record_detail()` does **not** track the bottom of content — `_draw_detail_info_below_cover()`'s return value is discarded. Compute the button Y position from the pre-measured `_detail_content_height` instead.
|
||||
|
||||
After the existing `_draw_detail_info_below_cover(rec, y)` call and before the scroll indicators, add:
|
||||
|
||||
```python
|
||||
# Scrobble button (at bottom of content, only if eligible)
|
||||
if rec.get("selected_release_id"):
|
||||
# Compute button Y from measured content height (y doesn't track
|
||||
# the bottom because _draw_detail_info_below_cover's return is
|
||||
# discarded — use _detail_content_height which already includes
|
||||
# the button height and gap from Step 4).
|
||||
button_y = (DETAIL_COVER_Y - offset
|
||||
+ _detail_content_height - SCROBBLE_BUTTON_H)
|
||||
# Only draw if the button is at least partially visible
|
||||
if (button_y + SCROBBLE_BUTTON_H > DAY_HEADER_Y + DAY_HEADER_H
|
||||
and button_y < HEIGHT):
|
||||
_draw_scrobble_button(rec, button_y)
|
||||
```
|
||||
|
||||
**Verification:** On-device — navigate to a record with `selected_release_id`. The "Scrobble" button appears at the bottom of the detail content, centered horizontally, AFTER all metadata and purchase date text. Scroll down to reveal it if the content is taller than the screen. Records without `selected_release_id` show no button. The button Y position must match the hit-test formula in Step 7.
|
||||
|
||||
### Step 7: Add scrobble hit-test and HTTP request handler
|
||||
|
||||
**File:** `presto/main.py`, in the "TOUCH HANDLING" section
|
||||
|
||||
New function — hit test:
|
||||
|
||||
```python
|
||||
def _scrobble_button_hit_test(x, y, rec):
|
||||
"""Return True if the touch (x, y) falls within the scrobble button bounds.
|
||||
Accounts for current detail_scroll_offset.
|
||||
Ignores touches in the fixed header area (handled separately)."""
|
||||
if not rec.get("selected_release_id"):
|
||||
return False
|
||||
|
||||
# Reject touches in the fixed header zone (handled by back-button logic)
|
||||
if y <= DAY_HEADER_Y + DAY_HEADER_H:
|
||||
return False
|
||||
|
||||
# Compute button Y position — must match the formula used in
|
||||
# draw_record_detail() (Step 6). _detail_content_height already
|
||||
# includes SCROBBLE_BUTTON_GAP + SCROBBLE_BUTTON_H from Step 4.
|
||||
button_y = (DETAIL_COVER_Y - detail_scroll_offset
|
||||
+ _detail_content_height - SCROBBLE_BUTTON_H)
|
||||
bx = (WIDTH - SCROBBLE_BUTTON_W) // 2
|
||||
|
||||
return (bx <= x <= bx + SCROBBLE_BUTTON_W and
|
||||
button_y <= y <= button_y + SCROBBLE_BUTTON_H)
|
||||
```
|
||||
|
||||
New function — HTTP handler:
|
||||
|
||||
```python
|
||||
def handle_scrobble_touch(rec):
|
||||
"""Execute the scrobble HTTP request and update button state.
|
||||
Blocks during the request (urequests is synchronous on MicroPython).
|
||||
On success, sets state to "done" (persists until navigation).
|
||||
On failure, reverts to "idle"."""
|
||||
global _scrobble_state
|
||||
|
||||
rec_id = rec.get("id")
|
||||
if not rec_id:
|
||||
return
|
||||
|
||||
# Set loading state and redraw immediately so the user sees "..."
|
||||
_scrobble_state = "loading"
|
||||
draw_record_detail()
|
||||
|
||||
# Make the POST request. urequests has limited timeout support on
|
||||
# MicroPython; if WiFi drops mid-request, the default timeout may
|
||||
# be 30+ seconds. Test this scenario on-device.
|
||||
url = API_BASE + "/api/v1/collection/" + str(rec_id) + "/scrobble"
|
||||
try:
|
||||
resp = urequests.post(url, headers=_auth_header())
|
||||
if resp.status_code == 200:
|
||||
_scrobble_state = "done"
|
||||
else:
|
||||
_scrobble_state = "idle"
|
||||
resp.close()
|
||||
except Exception:
|
||||
_scrobble_state = "idle"
|
||||
|
||||
gc.collect()
|
||||
draw_record_detail()
|
||||
```
|
||||
|
||||
**Verification:** Syntax check. Logic review — the function handles success (200 → "done"), API errors (non-200 → "idle"), and network exceptions (→ "idle"). The "done" state persists until the user navigates away (resets in Step 3). The hit-test formula (`_detail_content_height - SCROBBLE_BUTTON_H`) must match the drawing formula in Step 6.
|
||||
|
||||
### Step 8: Modify `STATE_RECORD` touch handling in the main loop
|
||||
|
||||
**File:** `presto/main.py`, `main()` function, inside `elif state == STATE_RECORD:`
|
||||
|
||||
After the existing `if dragged and pending_delta:` block (which handles scroll-finalize), add a `not dragged` branch:
|
||||
|
||||
```python
|
||||
if not dragged:
|
||||
# Determine which record is being viewed
|
||||
if previous_state == STATE_SEARCH_RESULTS:
|
||||
rec = search_results[selected_record_idx]
|
||||
else:
|
||||
rec = records[selected_record_idx]
|
||||
|
||||
# Check scrobble button tap
|
||||
if _scrobble_button_hit_test(x, y, rec):
|
||||
handle_scrobble_touch(rec)
|
||||
```
|
||||
|
||||
**Verification:** On-device — tap the scrobble button. Button text changes to "..." then "Done" (on success) or reverts to "Scrobble" (on failure). Note: while the HTTP request is in progress (loading state), the event loop is blocked by the synchronous `urequests.post()` call, so no new touches are processed until the request completes. This is expected behavior for MicroPython's single-threaded model. Scrolling still works correctly; a drag does not trigger a scrobble because `dragged` is `True` for drags.
|
||||
|
||||
### Step 9: Update `presto/README.md`
|
||||
|
||||
**File:** `presto/README.md`
|
||||
|
||||
Two changes:
|
||||
|
||||
**(a)** In the "Record detail view" usage section, add a bullet:
|
||||
|
||||
```
|
||||
- If the record can be scrobbled, a **"Scrobble" button** appears at the bottom — tap it to scrobble the release to Last.fm; "Done" confirms success
|
||||
```
|
||||
|
||||
**(b)** In the API Response format example, add `selected_release_id`:
|
||||
|
||||
```json
|
||||
"selected_release_id": "abc-123-uuid",
|
||||
```
|
||||
|
||||
(immediately after `"id"` or near the end of the record object)
|
||||
|
||||
**Verification:** Read the rendered README to confirm both additions read naturally and the API example remains valid JSON.
|
||||
|
||||
### Step 10: Update `presto/AGENTS.md`
|
||||
|
||||
**File:** `presto/AGENTS.md`
|
||||
|
||||
In the "API Contract" section, add `selected_release_id` to the "Fields used by the app" list. The current list is:
|
||||
|
||||
```
|
||||
Fields used by the app: `title`, `artists`, `format`, `release_date`, `genres`, `record_type`, `purchased_at`, `micro_cover_url`, `mini_cover_url`, `thumb_url`.
|
||||
```
|
||||
|
||||
Change to:
|
||||
|
||||
```
|
||||
Fields used by the app: `title`, `artists`, `format`, `release_date`, `genres`, `record_type`, `purchased_at`, `micro_cover_url`, `mini_cover_url`, `thumb_url`, `selected_release_id`.
|
||||
```
|
||||
|
||||
Also add the new endpoint to the API Contract section:
|
||||
|
||||
```
|
||||
POST /api/v1/collection/:record_id/scrobble
|
||||
Authorization: Bearer <API_TOKEN>
|
||||
```
|
||||
|
||||
**Verification:** Read the rendered AGENTS.md to confirm the field list is updated and the new endpoint is documented.
|
||||
|
||||
## Architecture impact analysis
|
||||
|
||||
| Touchpoint | Impact |
|
||||
| ------------------------------------------------------------- | --------------------------------------------------------------------------------------------------- |
|
||||
| `presto/main.py` — layout constants block | +3 constants (`SCROBBLE_BUTTON_W`, `_H`, `_GAP`) |
|
||||
| `presto/main.py` — color constants block | +4 color tuples (`SCROBBLE_BG`, `_TEXT`, `_DONE_BG`, `_DONE_TEXT`) |
|
||||
| `presto/main.py` — pen variables and `_init_pens()` | +4 pens initialized at startup; +4 globals in `_init_pens()` |
|
||||
| `presto/main.py` — global state block | +1 variable (`_scrobble_state`); +1 global in `main()` |
|
||||
| `presto/main.py` — `_measure_detail_content()` | +3 lines for button height when eligible |
|
||||
| `presto/main.py` — `draw_record_detail()` | +6 lines to draw button after existing content (using `_detail_content_height`-based Y calculation) |
|
||||
| `presto/main.py` — new function `_draw_scrobble_button()` | ~25 lines |
|
||||
| `presto/main.py` — new function `_scrobble_button_hit_test()` | ~15 lines (includes header-zone guard) |
|
||||
| `presto/main.py` — new function `handle_scrobble_touch()` | ~25 lines (no auto-revert sleep) |
|
||||
| `presto/main.py` — `main()` STATE_RECORD handler | +10 lines for tap dispatch |
|
||||
| `presto/main.py` — `main()` state transitions | +3 reset points for `_scrobble_state` (2 entry, 1 exit) |
|
||||
| `presto/main.py` — `handle_search_results_touch()` | +1 reset point for `_scrobble_state` |
|
||||
| `presto/README.md` | +2 lines (usage + API response) |
|
||||
| `presto/AGENTS.md` | +2 changes (field list + endpoint docs) |
|
||||
| Backend API | No changes (ML-177 covers this) |
|
||||
| Web UI (LiveView) | No changes |
|
||||
| Database / schemas | No changes |
|
||||
| PubSub / supervision tree | No changes |
|
||||
|
||||
## Performance profile
|
||||
|
||||
- **Button drawing:** O(1) — one `display.rectangle()` + one `_draw_centered_text()` call. Each redraw of the detail view adds ~2ms at most.
|
||||
- **Content height measurement:** O(1) addition — one condition check + two integer additions. No loops, no text measurement.
|
||||
- **HTTP request:** Blocking `urequests.post()`, expected 1–5 seconds on a functional WiFi connection. During this time the display is frozen (no touch processing), which is acceptable for a user-initiated action with "..." visual feedback. **Caveat:** MicroPython's `urequests` has limited timeout support. If WiFi drops mid-request, the default timeout can be 30+ seconds. This should be tested on-device with a simulated WiFi disconnect to confirm the real-world behavior.
|
||||
- **Scroll performance:** Zero impact. The button is only drawn during full redraws, never during drag-scroll redraw loops (which use clips and skip non-content). No new allocations in the scroll hot path.
|
||||
- **Memory:** ~4 pen objects (already amortized at startup), 1 global string/state variable. No new per-record allocations. No image caching. Negligible GC pressure.
|
||||
- **N+1 risk:** None. One POST per explicit user tap. No prefetching or batching.
|
||||
|
||||
## Benchmarking requirements
|
||||
|
||||
No benchmarks needed. This is a UI-only change with a single user-initiated HTTP request. The performance characteristics are straightforward and deterministic.
|
||||
|
||||
## Cost profile
|
||||
|
||||
- **API calls:** One `POST /api/v1/collection/:id/scrobble` per user scrobble action. The endpoint internally calls Last.fm (free API) and MusicBrainz (free, rate-limited but not metered). No paid third-party services consumed by this feature.
|
||||
- **Compute:** Negligible — the scrobble endpoint is a thin wrapper around existing `ScrobbleActivity.scrobble_release/3`.
|
||||
- **Storage:** No new database rows or files.
|
||||
- **Overall cost:** $0 marginal cost.
|
||||
|
||||
## Production infrastructure steps
|
||||
|
||||
### Production Changes
|
||||
|
||||
No infrastructure changes required. The backend endpoint is deployed as part of ML-177. This task deploys only the Presto client code.
|
||||
|
||||
**Rollout:**
|
||||
|
||||
```bash
|
||||
mise run presto
|
||||
```
|
||||
|
||||
This copies `presto/main.py` to the device and resets it.
|
||||
|
||||
**Rollback:**
|
||||
Redeploy the previous `main.py` (e.g., from git):
|
||||
|
||||
```bash
|
||||
git show HEAD~1:presto/main.py > /tmp/main_rollback.py
|
||||
mpremote fs cp /tmp/main_rollback.py :main.py
|
||||
mpremote reset
|
||||
```
|
||||
|
||||
**No other changes needed:**
|
||||
|
||||
- No environment variables
|
||||
- No database migrations
|
||||
- No DNS or firewall rules
|
||||
- No service restarts on the server
|
||||
|
||||
## Documentation updates
|
||||
|
||||
| File | Change |
|
||||
| ----------------------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
|
||||
| `presto/README.md` | Add scrobble button to "Record detail view" usage bullet points. Add `selected_release_id` to API response example JSON. |
|
||||
| `presto/AGENTS.md` | Add `selected_release_id` to the "Fields used by the app" list under API Contract section. Add `POST /api/v1/collection/:record_id/scrobble` endpoint documentation. |
|
||||
| `docs/architecture.md` | No changes (Presto is a client, not part of the supervision tree or database layout). |
|
||||
| `docs/project-conventions.md` | No changes (no new conventions introduced). |
|
||||
| `docs/production-infrastructure.md` | No changes (no infrastructure impact). |
|
||||
|
||||
Implementation complete. All code changes made; verified with python3 syntax check. On-device testing requires ML-177 (backend endpoint + selected_release_id field) to be deployed first.
|
||||
|
||||
<!-- SECTION:PLAN:END -->
|
||||
|
||||
## Implementation Notes
|
||||
|
||||
<!-- SECTION:NOTES:BEGIN -->
|
||||
|
||||
Implemented all 10 steps per the plan:
|
||||
|
||||
1. ✅ Added SCROBBLE_BUTTON_W/H/GAP constants and SCROBBLE_BG/TEXT/DONE_BG/DONE_TEXT color tuples
|
||||
|
||||
2. ✅ Added 4 new pens (\_pen_scrobble_bg, \_pen_scrobble_text, \_pen_scrobble_done_bg, \_pen_scrobble_done_text) initialized in \_init_pens()
|
||||
|
||||
3. ✅ Added \_scrobble_state global with resets at all 2 entry points and 1 exit point to STATE_RECORD
|
||||
|
||||
4. ✅ Modified \_measure_detail_content() to include SCROBBLE_BUTTON_GAP + SCROBBLE_BUTTON_H when selected_release_id is present
|
||||
|
||||
5. ✅ Added \_draw_scrobble_button() helper with 3 visual states (idle/loading/done)
|
||||
|
||||
6. ✅ Modified draw_record_detail() to draw button using \_detail_content_height-based Y calculation, with viewport visibility check
|
||||
|
||||
7. ✅ Added \_scrobble_button_hit_test() and handle_scrobble_touch() functions
|
||||
|
||||
8. ✅ Modified STATE_RECORD handler in main() with not dragged branch for scrobble tap dispatch
|
||||
|
||||
9. ✅ Updated README.md: added scrobble button usage bullet + selected_release_id to API response example
|
||||
|
||||
10. ✅ Updated AGENTS.md: added selected_release_id to fields list + POST /api/v1/collection/:record_id/scrobble endpoint docs
|
||||
|
||||
Syntax check passes cleanly (py_compile). Ready for on-device testing once ML-177 backend is deployed.
|
||||
|
||||
<!-- SECTION:NOTES:END -->
|
||||
|
||||
## Final Summary
|
||||
|
||||
<!-- SECTION:FINAL_SUMMARY:BEGIN -->
|
||||
|
||||
## What changed
|
||||
|
||||
Added a scrobble button to the Presto record detail view (`draw_record_detail()` / `STATE_RECORD`) that sends `POST /api/v1/collection/:record_id/scrobble` with the Bearer token and shows visual feedback.
|
||||
|
||||
### presto/main.py
|
||||
|
||||
- **Layout/color constants**: `SCROBBLE_BUTTON_W` (200), `SCROBBLE_BUTTON_H` (40), `SCROBBLE_BUTTON_GAP` (12); colors reuse existing palette (slate/green/white)
|
||||
- **4 new pens**: `_pen_scrobble_bg`, `_pen_scrobble_text`, `_pen_scrobble_done_bg`, `_pen_scrobble_done_text`, initialized in `_init_pens()`
|
||||
- **Global state**: `_scrobble_state` ("idle" / "loading" / "done"), reset on all 3 entry/exit paths to `STATE_RECORD`
|
||||
- **`_measure_detail_content()`**: Includes button height + gap when `selected_release_id` is present, ensuring correct scroll bounds
|
||||
- **`draw_record_detail()`**: Draws button at `_detail_content_height - SCROBBLE_BUTTON_H` from `DETAIL_COVER_Y`, viewport-clipped
|
||||
- **`_draw_scrobble_button()`**: Renders 3 visual states (Scrobble / ... / Done) centered horizontally
|
||||
- **`_scrobble_button_hit_test()`**: Hit-test matching the drawing formula, rejects header-zone touches
|
||||
- **`handle_scrobble_touch()`**: Blocking `urequests.post()`, sets state → redraws before/after request, catches all exceptions
|
||||
- **Main loop `STATE_RECORD`**: `not dragged` branch dispatches to scrobble on tap
|
||||
|
||||
### presto/README.md
|
||||
|
||||
- Added scrobble button usage bullet to Record detail view section
|
||||
- Added `selected_release_id` to API response JSON example
|
||||
|
||||
### presto/AGENTS.md
|
||||
|
||||
- Added `selected_release_id` to "Fields used by the app" list
|
||||
- Documented `POST /api/v1/collection/:record_id/scrobble` endpoint
|
||||
|
||||
## Why
|
||||
|
||||
The Presto can browse records but had no way to trigger a Last.fm scrobble. The backend endpoint (ML-177) provides the server-side action; this wires it to a touch target on the device, gated on `selected_release_id` so only scrobble-eligible records show the button.
|
||||
|
||||
## Risks / follow-ups
|
||||
|
||||
- Blocking HTTP request freezes display during scrobble (~1-5s normal, up to 30s on WiFi drop). Acceptable for user-initiated action on single-threaded MicroPython.
|
||||
- No debounce/double-tap prevention — could be added later if needed.
|
||||
- No error toast — button reverts to "Scrobble" on failure (minimal feedback).
|
||||
<!-- SECTION:FINAL_SUMMARY:END -->
|
||||
@@ -0,0 +1,72 @@
|
||||
---
|
||||
id: ML-179
|
||||
title: Refactor Presto main.py state handling
|
||||
status: Done
|
||||
assignee: []
|
||||
created_date: "2026-05-10 20:11"
|
||||
updated_date: "2026-05-11 06:47"
|
||||
labels: []
|
||||
dependencies: []
|
||||
documentation:
|
||||
- presto/AGENTS.md
|
||||
- presto/README.md
|
||||
- docs/project-conventions.md
|
||||
modified_files:
|
||||
- presto/main.py
|
||||
priority: medium
|
||||
ordinal: 6000
|
||||
---
|
||||
|
||||
## Description
|
||||
|
||||
<!-- SECTION:DESCRIPTION:BEGIN -->
|
||||
|
||||
Refactor presto/main.py in a single-file, behavior-preserving pass to reduce scattered global state, centralize navigation/reset rules, and remove duplicated scroll gesture logic. Keep deployment as one main.py file for the Pimoroni Presto app.
|
||||
|
||||
<!-- SECTION:DESCRIPTION:END -->
|
||||
|
||||
## Acceptance Criteria
|
||||
|
||||
<!-- AC:BEGIN -->
|
||||
|
||||
- [x] #1 presto/main.py keeps hardware objects, constants, layout values, color pens, and pure helpers at module level while moving mutable view state into simple MicroPython-compatible holder classes.
|
||||
- [x] #2 Stateful functions accept an AppState instance and access state through grouped fields instead of mutable module globals, with behavior preserved across home, search, month, day, detail, display sleep, and scrobble flows.
|
||||
- [x] #3 Navigation helper functions centralize screen transitions and reset the correct per-view scroll, loading, error, keyboard, detail, and scrobble state.
|
||||
- [x] #4 Record-list preparation returns content height for the caller to assign to day or search state while preserving existing per-record display cache keys.
|
||||
- [x] #5 Shared vertical drag handling replaces duplicated day, search, and detail drag loops without fetching images or measuring layout in the drag hot path.
|
||||
- [x] #6 Syntax verification passes from presto/ with python3 py_compile and final notes state that physical-device behavior was not verified unless tested on hardware.
|
||||
<!-- AC:END -->
|
||||
|
||||
## Implementation Plan
|
||||
|
||||
<!-- SECTION:PLAN:BEGIN -->
|
||||
|
||||
1. Add simple AppState, DayListState, SearchState, DetailState, and TouchState classes near the current global state section and initialize grouped mutable state through AppState.
|
||||
2. Convert stateful startup, display sleep, API, draw, hit-test, and touch handler functions to accept app and read/write app.<group>.<field> instead of module-level mutable globals.
|
||||
3. Add navigation helpers for home, today/month, day, detail, back-from-detail, search input, and search results transitions; use them for all screen changes so scroll/error/loading/detail/scrobble resets are centralized.
|
||||
4. Replace prepare_records_for_display/1 with prepare_record_list(recs), returning content height while preserving record dict cache keys and thumbnail preloading behavior.
|
||||
5. Extract the duplicated vertical drag loop into a shared helper parameterized by offset getters/setters, max offset, redraw callback, and placeholder-during-drag behavior.
|
||||
6. Run the Presto syntax check from presto/ and update the task acceptance criteria/final notes based on verification.
|
||||
<!-- SECTION:PLAN:END -->
|
||||
|
||||
## Implementation Notes
|
||||
|
||||
<!-- SECTION:NOTES:BEGIN -->
|
||||
|
||||
Implemented the refactor in presto/main.py as a single-file change. Mutable app state now lives under AppState and grouped state holders. Navigation helpers own the screen transitions and reset rules. Record-list preparation now returns content height for the caller. The repeated day/search/detail scroll loops were replaced by handle_vertical_drag, and detail cover drawing avoids thumbnail fetches while drag redraws are active. Verification run: python3 py_compile from presto/ passed; git diff --check passed. Physical Presto hardware behavior was not verified.
|
||||
|
||||
Follow-up tightening before final response: cached wrapped row lines and detail line metrics so drag redraws can reuse prepared layout data. Re-ran python3 py_compile from presto/ and git diff --check; both passed.
|
||||
|
||||
<!-- SECTION:NOTES:END -->
|
||||
|
||||
## Final Summary
|
||||
|
||||
<!-- SECTION:FINAL_SUMMARY:BEGIN -->
|
||||
|
||||
Refactored presto/main.py to group mutable runtime state under AppState while leaving hardware handles, constants, layout values, pens, and pure helpers at module scope. Added navigation helpers for home, today/month, day loading, record detail, detail back navigation, search input, and search results so view-entry reset behavior is centralized.
|
||||
|
||||
Converted stateful drawing, API, display sleep, hit-test, and touch handler paths to accept the AppState instance. Replaced prepare_records_for_display with prepare_record_list returning content height for day/search callers, preserving the existing record display cache keys and adding prepared wrapped-line caches for drag redraw performance. Replaced duplicated day, search, and detail drag loops with handle_vertical_drag and kept image fetching out of drag redraws by using placeholders where needed.
|
||||
|
||||
Verification: python3 py_compile passed from presto/ with cfile=/tmp/main.pyc, and git diff --check passed. Physical-device behavior was not verified on the Presto hardware.
|
||||
|
||||
<!-- SECTION:FINAL_SUMMARY:END -->
|
||||
Reference in New Issue
Block a user