diff --git a/docs/alarm-styles-plan.md b/docs/alarm-styles-plan.md new file mode 100644 index 0000000..c7ad38f --- /dev/null +++ b/docs/alarm-styles-plan.md @@ -0,0 +1,165 @@ +# Plan: Pluggable Alarm Styles + +Branch: `feature/alarm-styles` +Status: **plan only — no implementation yet.** Review and approve before coding. + +## Goal + +1. Support **multiple alarm styles**, selectable **per alarm**. +2. Make styles **pluggable** (a "plugin-like" system) so a new style is a drop-in. +3. Ship **two** styles: + - `simple` — press the button once → alarm stops. *(the "default" style; not implemented yet)* + - `blink` — the current count-the-blinks puzzle. *(already implemented, lives in `AlarmClock`)* +4. Style travels with each alarm (cron is still the single source of truth). + +## Current state (what changes, what stays) + +| Concern | Today | After | +|---|---|---| +| Scheduler | OS `cron`; crontab entries are alarms | **unchanged** | +| Source of truth | crontab (`wecker-alarm:` comments) | **unchanged** | +| Per-alarm runnable | command `... wecker.py [--music-file X]` | command `... wecker.py --style [--music-file X]` | +| Ring process | `wecker.py` runs `AlarmClock` state machine | `wecker.py` looks up style in a registry and runs it | +| Styles | one, hardcoded in `wecker.py` | a `styles/` package; `AlarmClock` → `BlinkStyle` | +| GraphQL `Alarm` type | `id`, `cronExpression`, `isEnabled` | + `style: str` | +| `setAlarm` | `cronExpression`, `isEnabled`, `id?` | + `style: str = "simple"` | +| `startRinging` | spawns `wecker.py` | spawns `wecker.py --style ` | +| PID file handoff | `wecker.pid` | **unchanged** | + +No database, no in-process scheduler, no separate alarm store. The crontab stays the store; this feature only adds *which style each entry runs*. + +## Design decisions (and the lazy choice for each) + +### 1. Where the style lives per alarm → **in the cron command** +`--style ` rides in the command, exactly like `--music-file` already does. The command *is* "how to run this alarm", so the style belongs there. +- **Rejected:** storing style in the comment (`wecker-alarm::blink`). Works, but couples identity (id) with behaviour (style) and changes the comment format. The command is the right place. +- **Parsing back:** `get_alarms` already reads `job.command`; it scans the tokens for `--style `. Since `_default_command()` is the only writer, parsing is reliable. + +### 2. The plugin system → **`styles/` package + explicit registry** (stdlib only) +``` +styles/ + __init__.py # STYLES = {"blink": BlinkStyle, "simple": SimpleStyle}; get_style(name) + base.py # AlarmStyle: __init__(set_led), update(now, is_pressed) -> bool + blink.py # BlinkStyle (refactored out of wecker.py AlarmClock) + simple.py # SimpleStyle (new) +``` +Adding a style = write a module, add one line to `styles/__init__.py`. This is the classic minimal Python plugin pattern. +- **Rejected:** `importlib.metadata` entry points (true external plugins). Overkill for a Pi alarm clock with two styles; the comment format isn't even a packaging distribution. **Upgrade path if needed:** auto-discover modules in `styles/` via `pkgutil.iter_modules` so a new file needs no registry edit. Not now. + +### 3. The style interface → **one callable in, one bool out** +```python +class AlarmStyle: + def __init__(self, set_led: Callable[[bool], None]): ... + def update(self, now: float, is_pressed: bool) -> bool: ... # False = stop +``` +That is the whole contract. `run_alarm()` owns everything shared: +- starts the music loop *before* the loop, +- samples the button each tick, +- calls `style.update(now, is_pressed)`, +- on `False` (or `test_mode`) stops the music and cleans up. + +Styles only decide **when to stop** and **what the LED does**. They never import hardware directly — `set_led` is injected, so: +- no circular import (`styles/` doesn't touch `RPi.GPIO`/`pygame`), +- the API process can `import styles` purely to validate style names, with no hardware loaded. +- **Rejected:** a full `Hardware`/`Outputs` abstraction layer. One passed callable is enough; if a future style needs volume/pwm, promote `set_led` to a small object then. YAGNI. +- Music stop moves out of the style (today `AlarmClock` calls `pygame.mixer.music.stop()` itself) into `run_alarm`'s cleanup. Behaviour-equivalent; the `finally` already does `pygame.mixer.quit()`. + +### 4. Defaults & backward compatibility +Two distinct "default" situations — kept separate on purpose: + +| Situation | Result | Why | +|---|---|---| +| **Legacy** cron entry with **no** `--style` (existing alarms after upgrade) | `blink` | Preserve current behaviour; an upgrade must not silently change every alarm. `argparse` default for `--style` = `"blink"`. | +| **New API call** `setAlarm(...)` with `style` omitted | `simple` | The user's stated "default style". Makes the new simple behaviour the obvious choice going forward. | +| `startRinging` with `style` omitted | `simple` | Consistent with `setAlarm`. | + +> **Decision point (please confirm):** I recommend `simple` as the API default for *new* alarms while keeping `blink` as the fallback for *legacy* cron commands. Alternative: default new alarms to `blink` too, so "no style" always means blink everywhere. Say the word and I'll flip it. + +### 5. Validation +- **`wecker.py`**: unknown `--style` → log error and exit non-zero. Never ring silently on a typo. +- **`setAlarm`**: `style` not in `STYLES` → raise → GraphQL error (fail at the boundary, like invalid cron expressions today). +- **`startRinging`**: same. + +## TDD / file-by-file work plan + +Per AGENTS.md: tests first, ruff green, no untested commits. + +### `styles/base.py` (new) +- `AlarmStyle` ABC: `__init__(self, set_led)`, abstract `update(now, is_pressed) -> bool`. +- `__main__` self-check: instantiate a fake style, assert `update` contract. + +### `styles/blink.py` (new) — refactor of `wecker.AlarmClock` +- Move the six states (`STATE_RINGING`…`STATE_WAIT_BEFORE_RETRY`), `BLINK_INTERVAL`, and the non-blocking blink logic verbatim into `BlinkStyle(set_led).update`. +- `run_alarm` handles music; `BlinkStyle` no longer calls `pygame.mixer.music.stop()` (returns `False` instead). + +### `styles/simple.py` (new) +```python +class SimpleStyle(AlarmStyle): + def update(self, now, is_pressed): + self.set_led(True) # LED solid on so the button is findable in the dark + return not is_pressed # first press → False → stop +``` +> **Decision point:** LED behaviour for `simple` — solid-on while ringing (proposed), or mirror the button press? Minor; propose solid-on. + +### `styles/__init__.py` (new) +```python +from styles.base import AlarmStyle +from styles.blink import BlinkStyle +from styles.simple import SimpleStyle + +STYLES = {"blink": BlinkStyle, "simple": SimpleStyle} + +def get_style(name: str): + try: + return STYLES[name] + except KeyError: + raise ValueError(f"Unknown alarm style: {name!r}. Known: {sorted(STYLES)}") +``` + +### `wecker.py` (modified) +- argparse: add `--style` (default `"blink"` for legacy compat). +- `setup()` unchanged (GPIO + mixer + load music). +- `run_alarm(style_name)`: `style = get_style(style_name)(set_led)`; loop calls `style.update`; on exit stop music. +- Remove `AlarmClock` and the `STATE_*` constants (moved to `styles/blink.py`). +- `__main__`: `run_alarm(style_name=args.style, music_file=args.music_file)`. + +### `api/crontab_manager.py` (modified) +- `get_alarms()` dict gains a `"style"` key, parsed from `job.command` (`--style ` → value; absent → `"blink"`). + +### `api/schema.py` (modified) +- `Alarm` type: + `style: str`; `_alarm_from_dict` maps it. +- `_default_command(style)`: append `--style {shlex.quote(style)}` to the command. +- `setAlarm(...)`: + `style: str = "simple"`; validate via `styles.get_style` (or check `STYLES`); pass to manager. +- `startRinging(style: str = "simple")`: validate; pass `--style {style}` to the `Popen` argv. +- `_start_wecker_process(style)`: add `--style` to the arg list. + +### Tests +- `tests/test_styles_blink.py`: port `test_wecker.py`'s state-machine tests to `BlinkStyle(mock_set_led)`. +- `tests/test_styles_simple.py`: press → `update` returns `False`; no press → `True`. +- `tests/test_styles_registry.py`: `get_style("blink"/"simple")` resolve; unknown raises `ValueError`. +- `tests/test_wecker.py`: rewrite for new entry point — `--style` arg, unknown style errors, `run_alarm` dispatches to style. Drop the moved state-machine tests. +- `tests/test_crontab.py`: `set_alarm` writes `--style` into command; `get_alarms` returns parsed `style`; legacy command (no `--style`) → `"blink"`. +- `tests/test_api.py`: `setAlarm` accepts + returns `style`; rejects unknown style; `getAlarms` includes `style`; `startRinging` passes `--style` to `Popen`. +- Every new module gets a `demo()`/`__main__` self-check per ponytail (blink logic, simple logic, registry). + +### `README.md` (modified, per AGENTS.md) +- New **"Alarm Styles"** section: describe `simple` vs `blink`. +- Update GraphQL examples: `setAlarm(..., style: "simple")`, `getAlarms { ... style }`, `startRinging(style: "blink")`. +- Note backward compat: existing alarms keep blinking until re-saved. + +## Migration / rollout +1. Deploy code. Existing cron entries (no `--style`) still run `blink` — no behaviour change. +2. Existing alarms keep blinking. To switch one to simple: `setAlarm(id=..., cronExpression=..., style: "simple")` (re-writes the command). +3. New alarms default to `simple`. + +## Out of scope (deliberately) +- External/plugin packages (entry points) — add if a third style comes from outside this repo. +- Per-style configuration knobs (e.g. blink count range) — hardcode sensible defaults now; add a config arg when a style actually needs tuning. +- A `Hardware` abstraction layer — one injected `set_led` callable is enough. +- Auto-discovery of style modules — explicit registry; switch to `pkgutil.iter_modules` only if the count grows. + +## Suggested commit sequence +1. `feat: add styles package with AlarmStyle base, registry, and blink+simple` (with tests) +2. `refactor: move AlarmClock state machine into styles.blink` +3. `feat: pass --style through wecker.py, crontab, and GraphQL API` (with tests) +4. `docs: document alarm styles in README`