docs: plan for pluggable alarm styles
This commit is contained in:
@@ -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:<uuid>` comments) | **unchanged** |
|
||||
| Per-alarm runnable | command `... wecker.py [--music-file X]` | command `... wecker.py --style <name> [--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 <name>` |
|
||||
| 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 <name>` 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:<uuid>: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 <name>`. 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 <name>` → 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`
|
||||
Reference in New Issue
Block a user