refactor: keep style-specific config off the base class via **kwargs
The base AlarmStyle contract carried music_file, forcing SimpleStyle to accept a parameter it never uses. Move style-specific config to the subclass: the base __init__ takes only set_led (**kwargs swallows the rest); each style declares the keyword args it actually uses (BlinkStyle: music_file; SimpleStyle: none). The runner passes config as keyword args and each style keeps only what it needs, so the base contract never grows as styles are added — a future 'talk' style adds talk_file to its own signature, not to the base. README plugin contract updated.
This commit is contained in:
@@ -152,7 +152,7 @@ Each alarm has a **style** that decides how you turn it off. Set it per alarm vi
|
|||||||
|
|
||||||
**The style owns its ringing tone.** The runner brings up the audio engine (the pygame mixer) and hands each style the button + LED; the style then produces its own tone directly — `blink` plays a music file, `simple` synthesises a beep, a future `talk` style would play speech. Styles use pygame directly (it's the audio engine, not hardware); the runner still owns mixer init and cleanup. This direct ownership is the seam for future styles that handle their own tone.
|
**The style owns its ringing tone.** The runner brings up the audio engine (the pygame mixer) and hands each style the button + LED; the style then produces its own tone directly — `blink` plays a music file, `simple` synthesises a beep, a future `talk` style would play speech. Styles use pygame directly (it's the audio engine, not hardware); the runner still owns mixer init and cleanup. This direct ownership is the seam for future styles that handle their own tone.
|
||||||
|
|
||||||
Adding a style is a plugin-style drop-in: add a class under `styles/` implementing the `AlarmStyle` contract (`__init__(set_led, music_file)` + `start()` + `update(now, is_pressed) -> bool`) and register one line in `styles/__init__.py`.
|
Adding a style is a plugin-style drop-in: add a class under `styles/` implementing the `AlarmStyle` contract (`__init__(set_led, **kwargs)` + `start()` + `update(now, is_pressed) -> bool`) and register one line in `styles/__init__.py`. Only `set_led` is universal; declare any style-specific config as your own keyword args (e.g. `BlinkStyle` takes `music_file`, `SimpleStyle` takes none) — the runner passes config as keyword args and each style keeps only what it uses.
|
||||||
|
|
||||||
**Backward compatibility:** existing cron entries created before this feature have no `--style` flag and keep running the `blink` puzzle, so an upgrade never silently changes an alarm. To switch an existing alarm to `simple`, re-save it with `setAlarm(id: ..., cronExpression: ..., style: "simple")`.
|
**Backward compatibility:** existing cron entries created before this feature have no `--style` flag and keep running the `blink` puzzle, so an upgrade never silently changes an alarm. To switch an existing alarm to `simple`, re-save it with `setAlarm(id: ..., cronExpression: ..., style: "simple")`.
|
||||||
|
|
||||||
|
|||||||
+10
-7
@@ -12,16 +12,19 @@ class AlarmStyle(ABC):
|
|||||||
import-safe in the API process — ``import pygame`` does not initialise
|
import-safe in the API process — ``import pygame`` does not initialise
|
||||||
audio, so the API can import styles just to validate names.
|
audio, so the API can import styles just to validate names.
|
||||||
|
|
||||||
Lifecycle: construct once with ``set_led``/``music_file``; call ``start()``
|
Lifecycle: construct once with ``set_led`` (plus any style-specific
|
||||||
to begin ringing (kick off the tone and initial LED); then call ``update``
|
keyword config the runner passes); call ``start()`` to begin ringing (kick
|
||||||
every tick. Return ``False`` from ``update`` to stop the alarm.
|
off the tone and initial LED); then call ``update`` every tick. Return
|
||||||
|
``False`` from ``update`` to stop the alarm.
|
||||||
|
|
||||||
|
Only ``set_led`` is universal. Style-specific config (e.g. ``music_file``)
|
||||||
|
belongs on the subclass, not here, so the base contract never grows as
|
||||||
|
styles are added; the runner passes config as keyword arguments and each
|
||||||
|
style declares only the ones it uses (extra kwargs are swallowed).
|
||||||
"""
|
"""
|
||||||
|
|
||||||
def __init__(
|
def __init__(self, set_led: Callable[[bool], None], **kwargs):
|
||||||
self, set_led: Callable[[bool], None], music_file: str | None = None
|
|
||||||
):
|
|
||||||
self.set_led = set_led
|
self.set_led = set_led
|
||||||
self.music_file = music_file
|
|
||||||
|
|
||||||
def start(self) -> None:
|
def start(self) -> None:
|
||||||
"""Begin ringing. Override to start the style's tone / initial LED."""
|
"""Begin ringing. Override to start the style's tone / initial LED."""
|
||||||
|
|||||||
+3
-2
@@ -23,8 +23,9 @@ class BlinkStyle(AlarmStyle):
|
|||||||
exactly that many times. A wrong count starts a fresh sequence.
|
exactly that many times. A wrong count starts a fresh sequence.
|
||||||
"""
|
"""
|
||||||
|
|
||||||
def __init__(self, set_led, music_file=None):
|
def __init__(self, set_led, music_file=None, **kwargs):
|
||||||
super().__init__(set_led, music_file)
|
super().__init__(set_led, **kwargs)
|
||||||
|
self.music_file = music_file
|
||||||
self.state = STATE_RINGING
|
self.state = STATE_RINGING
|
||||||
self.target_blinks = 0
|
self.target_blinks = 0
|
||||||
self.user_presses = 0
|
self.user_presses = 0
|
||||||
|
|||||||
+2
-2
@@ -37,8 +37,8 @@ class SimpleStyle(AlarmStyle):
|
|||||||
wave played through ``pygame.mixer.Sound``.
|
wave played through ``pygame.mixer.Sound``.
|
||||||
"""
|
"""
|
||||||
|
|
||||||
def __init__(self, set_led, music_file=None):
|
def __init__(self, set_led, **kwargs):
|
||||||
super().__init__(set_led, music_file)
|
super().__init__(set_led, **kwargs)
|
||||||
self._next_beep = 0.0
|
self._next_beep = 0.0
|
||||||
self._beep = None
|
self._beep = None
|
||||||
|
|
||||||
|
|||||||
@@ -8,6 +8,16 @@ def _make_style():
|
|||||||
return SimpleStyle(lambda on: calls.append(on)), calls
|
return SimpleStyle(lambda on: calls.append(on)), calls
|
||||||
|
|
||||||
|
|
||||||
|
def test_simple_ignores_unrelated_config():
|
||||||
|
# SimpleStyle must swallow style-specific kwargs it doesn't use (e.g.
|
||||||
|
# music_file belongs to blink), so the runner can pass config uniformly.
|
||||||
|
style, _ = _make_style()
|
||||||
|
style = SimpleStyle(lambda on: None, music_file="ignored.mp3")
|
||||||
|
with patch("styles.simple.pygame", MagicMock()):
|
||||||
|
style.start()
|
||||||
|
assert style.update(0.0, False) is True
|
||||||
|
|
||||||
|
|
||||||
def test_simple_keeps_ringing_when_not_pressed():
|
def test_simple_keeps_ringing_when_not_pressed():
|
||||||
style, _ = _make_style()
|
style, _ = _make_style()
|
||||||
with patch("styles.simple.pygame", MagicMock()):
|
with patch("styles.simple.pygame", MagicMock()):
|
||||||
|
|||||||
@@ -66,9 +66,8 @@ def test_run_alarm_uses_selected_style(mock_gpio, mock_pygame):
|
|||||||
wecker.run_alarm(style_name="whatever", test_mode=True)
|
wecker.run_alarm(style_name="whatever", test_mode=True)
|
||||||
|
|
||||||
get_style.assert_called_once_with("whatever")
|
get_style.assert_called_once_with("whatever")
|
||||||
args = fake_style_cls.call_args.args
|
assert fake_style_cls.call_args.args[0] is wecker.set_led
|
||||||
assert args[0] is wecker.set_led
|
assert isinstance(fake_style_cls.call_args.kwargs["music_file"], str)
|
||||||
assert isinstance(args[1], str) # resolved music file
|
|
||||||
fake.start.assert_called_once()
|
fake.start.assert_called_once()
|
||||||
fake.update.assert_called()
|
fake.update.assert_called()
|
||||||
|
|
||||||
@@ -80,7 +79,7 @@ def test_run_alarm_default_music_file(mock_gpio, mock_pygame, monkeypatch):
|
|||||||
fake_style_cls = MagicMock(return_value=fake)
|
fake_style_cls = MagicMock(return_value=fake)
|
||||||
with patch("wecker.get_style", return_value=fake_style_cls):
|
with patch("wecker.get_style", return_value=fake_style_cls):
|
||||||
wecker.run_alarm(test_mode=True)
|
wecker.run_alarm(test_mode=True)
|
||||||
assert fake_style_cls.call_args.args[1] == wecker.DEFAULT_MUSIC_FILE
|
assert fake_style_cls.call_args.kwargs["music_file"] == wecker.DEFAULT_MUSIC_FILE
|
||||||
|
|
||||||
|
|
||||||
def test_run_alarm_env_music_file(mock_gpio, mock_pygame, monkeypatch):
|
def test_run_alarm_env_music_file(mock_gpio, mock_pygame, monkeypatch):
|
||||||
@@ -90,7 +89,7 @@ def test_run_alarm_env_music_file(mock_gpio, mock_pygame, monkeypatch):
|
|||||||
fake_style_cls = MagicMock(return_value=fake)
|
fake_style_cls = MagicMock(return_value=fake)
|
||||||
with patch("wecker.get_style", return_value=fake_style_cls):
|
with patch("wecker.get_style", return_value=fake_style_cls):
|
||||||
wecker.run_alarm(test_mode=True)
|
wecker.run_alarm(test_mode=True)
|
||||||
assert fake_style_cls.call_args.args[1] == "/env/track.mp3"
|
assert fake_style_cls.call_args.kwargs["music_file"] == "/env/track.mp3"
|
||||||
|
|
||||||
|
|
||||||
def test_run_alarm_music_file_arg_overrides_env(mock_gpio, mock_pygame, monkeypatch):
|
def test_run_alarm_music_file_arg_overrides_env(mock_gpio, mock_pygame, monkeypatch):
|
||||||
@@ -100,7 +99,7 @@ def test_run_alarm_music_file_arg_overrides_env(mock_gpio, mock_pygame, monkeypa
|
|||||||
fake_style_cls = MagicMock(return_value=fake)
|
fake_style_cls = MagicMock(return_value=fake)
|
||||||
with patch("wecker.get_style", return_value=fake_style_cls):
|
with patch("wecker.get_style", return_value=fake_style_cls):
|
||||||
wecker.run_alarm(music_file="/arg/track.mp3", test_mode=True)
|
wecker.run_alarm(music_file="/arg/track.mp3", test_mode=True)
|
||||||
assert fake_style_cls.call_args.args[1] == "/arg/track.mp3"
|
assert fake_style_cls.call_args.kwargs["music_file"] == "/arg/track.mp3"
|
||||||
|
|
||||||
|
|
||||||
def test_run_alarm_blink_plays_music(mock_gpio, mock_pygame):
|
def test_run_alarm_blink_plays_music(mock_gpio, mock_pygame):
|
||||||
|
|||||||
@@ -110,7 +110,7 @@ def run_alarm(
|
|||||||
music_file = os.environ.get("MUSIC_FILE", DEFAULT_MUSIC_FILE)
|
music_file = os.environ.get("MUSIC_FILE", DEFAULT_MUSIC_FILE)
|
||||||
|
|
||||||
setup()
|
setup()
|
||||||
style = style_cls(set_led, music_file)
|
style = style_cls(set_led, music_file=music_file)
|
||||||
logging.info("Alarm clock started.")
|
logging.info("Alarm clock started.")
|
||||||
try:
|
try:
|
||||||
style.start() # style kicks off its own tone + initial LED
|
style.start() # style kicks off its own tone + initial LED
|
||||||
|
|||||||
Reference in New Issue
Block a user