From 3a7b6ac7a378d6be1256bd84abf2a773a06ae2dc Mon Sep 17 00:00:00 2001 From: Markus Graf Date: Sun, 2 Aug 2026 21:50:00 +0200 Subject: [PATCH] refactor: styles own their ringing tone directly (Pattern C) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Drop the injected AlarmSound Protocol and the wecker.Sound class. Each style now owns its tone by using pygame directly; the runner owns only the audio engine lifecycle (mixer init/quit), button sampling, and cleanup. This is the seam for future styles that handle their own tone — a 'talk' style would just import pygame and play speech, with no shared interface to extend. - styles/base.py: AlarmStyle.__init__(set_led, music_file) + start() + update() - styles/blink.py: start() loads+plays music via pygame.mixer.music - styles/simple.py: owns its beep — synthesises a square-wave buffer in module (stdlib array+math) and plays it via pygame.mixer.Sound - wecker.py: setup() brings up the mixer only; run_alarm constructs the style and guards start() with a clean log+exit on failure Tests: a shared tests/conftest.py stubs RPi.GPIO/pygame in sys.modules before any SUT import (order-independent, removes duplicated inline mocking); the wecker mock_pygame fixture patches one fresh pygame mock into wecker + both style modules so assertions see the same calls. README: tone-ownership and plugin-contract updated. --- README.md | 4 +-- styles/base.py | 40 ++++++--------------- styles/blink.py | 20 +++++------ styles/simple.py | 64 ++++++++++++++++++++++++--------- tests/conftest.py | 14 ++++++++ tests/test_single_instance.py | 13 +++---- tests/test_styles_blink.py | 25 +++++++------ tests/test_styles_simple.py | 60 +++++++++++++++++-------------- tests/test_wecker.py | 51 +++++++++++---------------- wecker.py | 66 ++++++++--------------------------- 10 files changed, 167 insertions(+), 190 deletions(-) create mode 100644 tests/conftest.py diff --git a/README.md b/README.md index e7dd31a..4410ca3 100644 --- a/README.md +++ b/README.md @@ -150,9 +150,9 @@ Each alarm has a **style** that decides how you turn it off. Set it per alarm vi | `simple` | A repeating, ordinary-alarm-clock **beep** (no music file needed). | Press the button once. The LED stays solid on so the button is findable in the dark. **Default for new alarms.** | | `blink` | A music file on an endless loop (from `--music-file` / `MUSIC_FILE` / the default track). | The memory-and-attention puzzle described in [How the Puzzle Works](#how-the-puzzle-works). | -**The style owns its ringing tone.** The runner hands each style an audio capability (`play_music(path)` and `play_beep()`); the style decides which to use and when. This 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, sound, 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, music_file)` + `start()` + `update(now, is_pressed) -> bool`) and register one line in `styles/__init__.py`. **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")`. diff --git a/styles/base.py b/styles/base.py index 920f7b8..4e47419 100644 --- a/styles/base.py +++ b/styles/base.py @@ -1,44 +1,26 @@ from abc import ABC, abstractmethod -from typing import Callable, Protocol - - -class AlarmSound(Protocol): - """Audio capability injected into a style so it can own its ringing tone. - - Styles stay free of pygame/GPIO imports (the API process imports them only - for name validation); the runner supplies a concrete implementation. - """ - - def play_music(self, path: str) -> None: - """Load and play a music file on an endless loop.""" - ... - - def play_beep(self) -> None: - """Play one short beep tone.""" - ... +from typing import Callable class AlarmStyle(ABC): """A pluggable alarm behaviour. - A style owns *its own ringing tone* (via the injected ``sound``) and - decides when the alarm stops and what the LED does. It never touches - hardware directly: ``set_led`` and ``sound`` are injected by the runner, so - styles stay import-safe in the API process (no GPIO/pygame). + A style owns *its own ringing tone*: it uses the audio engine (pygame) + directly and decides when the alarm stops and what the LED does. It never + touches hardware directly: ``set_led`` is injected by the runner, and the + runner owns the audio engine lifecycle (mixer init/quit). Styles stay + import-safe in the API process — ``import pygame`` does not initialise + audio, so the API can import styles just to validate names. - Lifecycle: construct once with ``set_led``/``sound``/``music_file``; call - ``start()`` to begin ringing (kick off the tone and initial LED); then call - ``update`` every tick. Return ``False`` from ``update`` to stop the alarm. + Lifecycle: construct once with ``set_led``/``music_file``; call ``start()`` + to begin ringing (kick off the tone and initial LED); then call ``update`` + every tick. Return ``False`` from ``update`` to stop the alarm. """ def __init__( - self, - set_led: Callable[[bool], None], - sound: AlarmSound, - music_file: str | None = None, + self, set_led: Callable[[bool], None], music_file: str | None = None ): self.set_led = set_led - self.sound = sound self.music_file = music_file def start(self) -> None: diff --git a/styles/blink.py b/styles/blink.py index be507ee..b617511 100644 --- a/styles/blink.py +++ b/styles/blink.py @@ -1,6 +1,8 @@ import logging import random +import pygame + from styles.base import AlarmStyle # State Machine states for the puzzle flow. @@ -21,8 +23,8 @@ class BlinkStyle(AlarmStyle): exactly that many times. A wrong count starts a fresh sequence. """ - def __init__(self, set_led, sound, music_file=None): - super().__init__(set_led, sound, music_file) + def __init__(self, set_led, music_file=None): + super().__init__(set_led, music_file) self.state = STATE_RINGING self.target_blinks = 0 self.user_presses = 0 @@ -35,7 +37,9 @@ class BlinkStyle(AlarmStyle): def start(self): # The puzzle rings with music on an endless loop; the LED stays off # until the button is pressed to start the blink sequence. - self.sound.play_music(self.music_file) + pygame.mixer.music.load(self.music_file) + pygame.mixer.music.set_volume(1.0) + pygame.mixer.music.play(-1) self.set_led(False) def update(self, now, is_pressed): @@ -130,14 +134,8 @@ class BlinkStyle(AlarmStyle): if __name__ == "__main__": # Self-check: a correct blink sequence stops the alarm. - class _StubSound: - def play_music(self, path): - pass - - def play_beep(self): - pass - - style = BlinkStyle(lambda on: None, _StubSound(), "x.mp3") + # (Does not call start(), which needs the audio engine.) + style = BlinkStyle(lambda on: None, "x.mp3") assert style.update(100.0, True) is True assert style.state == STATE_WAIT_BEFORE_BLINK diff --git a/styles/simple.py b/styles/simple.py index 0abc2c2..fa9f11c 100644 --- a/styles/simple.py +++ b/styles/simple.py @@ -1,3 +1,8 @@ +import array +import math + +import pygame + from styles.base import AlarmStyle # Time between beep retriggers; a short beep followed by this gap gives the @@ -5,48 +10,73 @@ from styles.base import AlarmStyle BEEP_INTERVAL = 0.5 +def _make_beep_buffer( + freq: int = 1000, duration: float = 0.15, rate: int = 44100 +) -> bytes: + """Synthesize a short square-wave beep as signed-16-bit stereo PCM. + + Matches the mixer format used by wecker.setup() (rate, -16, 2 channels) so + it feeds straight into ``pygame.mixer.Sound``. No audio file, no extra dep. + """ + n = int(rate * duration) + buf = array.array("h") + for i in range(n): + v = 32767 if math.sin(2 * math.pi * freq * (i / rate)) >= 0 else -32767 + buf.append(v) + buf.append(v) # duplicate to stereo + return buf.tobytes() + + +_BEEP_BUFFER = _make_beep_buffer() + + class SimpleStyle(AlarmStyle): """Press the button once to stop. Rings with a repeating beep. - This style plays no music file; it owns its tone via ``sound.play_beep``. + This style plays no music file; it owns its tone via a synthesised square + wave played through ``pygame.mixer.Sound``. """ - def __init__(self, set_led, sound, music_file=None): - super().__init__(set_led, sound, music_file) + def __init__(self, set_led, music_file=None): + super().__init__(set_led, music_file) self._next_beep = 0.0 + self._beep = None def start(self): # LED solid on so the button is findable in the dark. self.set_led(True) + self._beep = pygame.mixer.Sound(_BEEP_BUFFER) def update(self, now, is_pressed): self.set_led(True) if now >= self._next_beep: - self.sound.play_beep() + self._beep.play() self._next_beep = now + BEEP_INTERVAL return not is_pressed # first press -> False -> stop if __name__ == "__main__": - # Self-check: not pressed keeps ringing and beeps; a press stops it. - class _StubSound: + # Self-check: the timer beeps on the interval and a press stops it. + # Bypasses start() (which needs the audio engine) by stubbing the beep. + + class _Beep: def __init__(self): - self.beeps = 0 + self.plays = 0 - def play_music(self, path): - raise AssertionError("simple style must never play music") + def play(self): + self.plays += 1 - def play_beep(self): - self.beeps += 1 + style = SimpleStyle(lambda on: None) + style._beep = _Beep() - sound = _StubSound() - style = SimpleStyle(lambda on: None, sound) - style.start() assert style.update(0.0, False) is True - assert sound.beeps == 1 # first tick beeps + assert style._beep.plays == 1 # first tick beeps + assert style.update(0.4, False) is True # before interval -> no beep - assert sound.beeps == 1 + assert style._beep.plays == 1 + assert style.update(0.5, False) is True # at interval -> beep - assert sound.beeps == 2 + assert style._beep.plays == 2 + assert style.update(0.6, True) is False # press -> stop print("simple self-check ok") diff --git a/tests/conftest.py b/tests/conftest.py new file mode 100644 index 0000000..4b8d9ad --- /dev/null +++ b/tests/conftest.py @@ -0,0 +1,14 @@ +"""Shared test setup: stub hardware/audio modules before any SUT import. + +wecker and the alarm styles import RPi.GPIO (hardware) and pygame (audio +engine). The tests run off the Pi and without a real audio device, so these +are replaced with MagicMocks in sys.modules before wecker/styles are imported. +Loaded by pytest before any test module in this directory is collected. +""" + +import sys +from unittest.mock import MagicMock + +sys.modules["RPi"] = MagicMock() +sys.modules["RPi.GPIO"] = MagicMock() +sys.modules["pygame"] = MagicMock() diff --git a/tests/test_single_instance.py b/tests/test_single_instance.py index 1543a16..2c6b6b0 100644 --- a/tests/test_single_instance.py +++ b/tests/test_single_instance.py @@ -1,14 +1,9 @@ import pytest -import sys -from unittest.mock import MagicMock, patch +from unittest.mock import patch -# Mock hardware modules before importing wecker (tests run off the Pi). -sys.modules["RPi"] = MagicMock() -sys.modules["RPi.GPIO"] = MagicMock() -sys.modules["pygame"] = MagicMock() - -import common # noqa: E402 -import wecker # noqa: E402 +# Hardware/audio mocks live in tests/conftest.py (shared across all test files). +import common +import wecker @pytest.fixture diff --git a/tests/test_styles_blink.py b/tests/test_styles_blink.py index a861723..c11d7b5 100644 --- a/tests/test_styles_blink.py +++ b/tests/test_styles_blink.py @@ -1,4 +1,4 @@ -from unittest.mock import MagicMock +from unittest.mock import MagicMock, patch from styles.blink import ( STATE_BLINKING, @@ -11,20 +11,20 @@ from styles.blink import ( def _make_blink(music_file="music.mp3"): - """Return (style, sound_mock) for a BlinkStyle ready to start.""" - sound = MagicMock() - style = BlinkStyle(lambda on: None, sound, music_file) - return style, sound + """Return (style, set_led calls) for a BlinkStyle ready to update.""" + calls = [] + style = BlinkStyle(lambda on: calls.append(on), music_file) + return style, calls def test_start_plays_music_and_led_off(): - calls = [] - sound = MagicMock() - style = BlinkStyle(lambda on: calls.append(on), sound, "track.mp3") - style.start() - sound.play_music.assert_called_once_with("track.mp3") + style, calls = _make_blink("track.mp3") + with patch("styles.blink.pygame", MagicMock()) as pg: + style.start() + pg.mixer.music.load.assert_called_with("track.mp3") + pg.mixer.music.set_volume.assert_called_with(1.0) + pg.mixer.music.play.assert_called_with(-1) assert calls == [False] # LED off while music plays until a press starts the puzzle - assert style.state == 0 # STATE_RINGING until a press def test_press_starts_puzzle(): @@ -74,8 +74,7 @@ def test_blink_incorrect_retries(): def test_blinking_non_blocking(): - calls = [] - style, _ = _make_blink() + style, calls = _make_blink() style.set_led = calls.append style.state = STATE_BLINKING style.target_blinks = 2 diff --git a/tests/test_styles_simple.py b/tests/test_styles_simple.py index 3bf1d8e..a721656 100644 --- a/tests/test_styles_simple.py +++ b/tests/test_styles_simple.py @@ -1,49 +1,55 @@ -from unittest.mock import MagicMock +from unittest.mock import MagicMock, patch from styles.simple import SimpleStyle def _make_style(): - sound = MagicMock() - style = SimpleStyle(lambda on: None, sound) - return style, sound + calls = [] + return SimpleStyle(lambda on: calls.append(on)), calls def test_simple_keeps_ringing_when_not_pressed(): style, _ = _make_style() - assert style.update(0.0, False) is True + with patch("styles.simple.pygame", MagicMock()): + style.start() + assert style.update(0.0, False) is True def test_simple_stops_on_first_press(): style, _ = _make_style() - assert style.update(0.0, True) is False # press -> stop + with patch("styles.simple.pygame", MagicMock()): + style.start() + assert style.update(0.0, True) is False # press -> stop def test_simple_led_solid_on(): - calls = [] - sound = MagicMock() - style = SimpleStyle(lambda on: calls.append(on), sound) - style.start() - assert calls == [True] - style.update(0.0, False) - assert calls[-1] is True # stays on every tick + style, calls = _make_style() + with patch("styles.simple.pygame", MagicMock()): + style.start() + style.update(0.0, False) + assert calls == [True, True] # start sets LED on; update keeps it on def test_simple_beeps_on_interval(): - style, sound = _make_style() - style.start() - style.update(100.0, False) # first tick -> beep - assert sound.play_beep.called - sound.play_beep.reset_mock() - style.update(100.4, False) # before interval -> no beep - assert not sound.play_beep.called - style.update(100.5, False) # at interval -> beep - assert sound.play_beep.called + style, _ = _make_style() + with patch("styles.simple.pygame", MagicMock()) as pg: + style.start() + style.update(100.0, False) # first tick -> beep + assert pg.mixer.Sound.return_value.play.called + pg.mixer.Sound.return_value.play.reset_mock() + + style.update(100.4, False) # before interval -> no beep + assert not pg.mixer.Sound.return_value.play.called + + style.update(100.5, False) # at interval -> beep + assert pg.mixer.Sound.return_value.play.called def test_simple_never_plays_music(): - style, sound = _make_style() - style.start() - style.update(0.0, False) - style.update(0.0, True) # press -> stop - assert not sound.play_music.called + style, _ = _make_style() + with patch("styles.simple.pygame", MagicMock()) as pg: + style.start() + style.update(0.0, False) + style.update(0.0, True) # press -> stop + assert not pg.mixer.music.play.called + assert not pg.mixer.music.load.called diff --git a/tests/test_wecker.py b/tests/test_wecker.py index 9fa16b5..5cb20d4 100644 --- a/tests/test_wecker.py +++ b/tests/test_wecker.py @@ -1,13 +1,9 @@ import pytest -import sys from unittest.mock import MagicMock, patch -# Mock RPi.GPIO and pygame before importing wecker -sys.modules["RPi"] = MagicMock() -sys.modules["RPi.GPIO"] = MagicMock() -sys.modules["pygame"] = MagicMock() - -import wecker # noqa: E402 +import styles.blink +import styles.simple +import wecker @pytest.fixture @@ -20,7 +16,18 @@ def mock_gpio(): @pytest.fixture def mock_pygame(): - with patch.object(wecker, "pygame") as mock: + """One fresh pygame mock shared by wecker and both styles. + + Under Pattern C the styles use pygame directly, so a single mock must be + patched into wecker (setup) and the style modules (start) for assertions + to see the same calls. + """ + mock = MagicMock() + with ( + patch.object(wecker, "pygame", mock), + patch.object(styles.blink, "pygame", mock), + patch.object(styles.simple, "pygame", mock), + ): yield mock @@ -50,7 +57,7 @@ def test_run_alarm_unknown_style_raises(mock_gpio, mock_pygame): def test_run_alarm_uses_selected_style(mock_gpio, mock_pygame): - """run_alarm looks up the style, injects a Sound, and calls start().""" + """run_alarm looks up the style, injects set_led + music_file, calls start().""" fake = MagicMock() fake.update.return_value = True # keep ringing fake_style_cls = MagicMock(return_value=fake) @@ -61,7 +68,7 @@ def test_run_alarm_uses_selected_style(mock_gpio, mock_pygame): get_style.assert_called_once_with("whatever") args = fake_style_cls.call_args.args assert args[0] is wecker.set_led - assert isinstance(args[1], wecker.Sound) + assert isinstance(args[1], str) # resolved music file fake.start.assert_called_once() fake.update.assert_called() @@ -73,7 +80,7 @@ def test_run_alarm_default_music_file(mock_gpio, mock_pygame, monkeypatch): fake_style_cls = MagicMock(return_value=fake) with patch("wecker.get_style", return_value=fake_style_cls): wecker.run_alarm(test_mode=True) - assert fake_style_cls.call_args.args[2] == wecker.DEFAULT_MUSIC_FILE + assert fake_style_cls.call_args.args[1] == wecker.DEFAULT_MUSIC_FILE def test_run_alarm_env_music_file(mock_gpio, mock_pygame, monkeypatch): @@ -83,7 +90,7 @@ def test_run_alarm_env_music_file(mock_gpio, mock_pygame, monkeypatch): fake_style_cls = MagicMock(return_value=fake) with patch("wecker.get_style", return_value=fake_style_cls): wecker.run_alarm(test_mode=True) - assert fake_style_cls.call_args.args[2] == "/env/track.mp3" + assert fake_style_cls.call_args.args[1] == "/env/track.mp3" def test_run_alarm_music_file_arg_overrides_env(mock_gpio, mock_pygame, monkeypatch): @@ -93,11 +100,10 @@ def test_run_alarm_music_file_arg_overrides_env(mock_gpio, mock_pygame, monkeypa fake_style_cls = MagicMock(return_value=fake) with patch("wecker.get_style", return_value=fake_style_cls): wecker.run_alarm(music_file="/arg/track.mp3", test_mode=True) - assert fake_style_cls.call_args.args[2] == "/arg/track.mp3" + assert fake_style_cls.call_args.args[1] == "/arg/track.mp3" -@patch("wecker.time.time") -def test_run_alarm_blink_plays_music(mock_time, mock_gpio, mock_pygame): +def test_run_alarm_blink_plays_music(mock_gpio, mock_pygame): """The default (blink) style plays music on an endless loop.""" wecker.run_alarm(test_mode=True) assert mock_pygame.mixer.music.play.called @@ -109,18 +115,3 @@ def test_run_alarm_simple_beeps_and_plays_no_music(mock_gpio, mock_pygame): wecker.run_alarm(style_name="simple", test_mode=True) assert mock_pygame.mixer.Sound.return_value.play.called # beep played assert not mock_pygame.mixer.music.play.called # no music - - -def test_sound_play_music_loads_and_loops(mock_pygame): - s = wecker.Sound() - s.play_music("/track.mp3") - mock_pygame.mixer.music.load.assert_called_with("/track.mp3") - mock_pygame.mixer.music.set_volume.assert_called_with(1.0) - mock_pygame.mixer.music.play.assert_called_with(-1) - - -def test_sound_play_beep_plays_buffer(mock_pygame): - s = wecker.Sound() - s.play_beep() - mock_pygame.mixer.Sound.assert_called_once_with(wecker._BEEP_BUFFER) - mock_pygame.mixer.Sound.return_value.play.assert_called_once() diff --git a/wecker.py b/wecker.py index f141788..3b0aa27 100644 --- a/wecker.py +++ b/wecker.py @@ -1,5 +1,3 @@ -import array -import math import os import sys import time @@ -73,10 +71,10 @@ DEFAULT_MUSIC_FILE = "Laid Back - Sunshine Reggae.mp3" def setup(): - """Initialize GPIO and audio hardware. Call once before running. + """Initialize GPIO and the audio engine. Call once before running. - Audio playback itself is owned by each style via the injected ``Sound`` - capability, so this only brings up the mixer and pins. + Each style owns its own ringing tone via pygame directly; this only brings + up the mixer and the GPIO pins. """ GPIO.setmode(GPIO.BCM) GPIO.setup(LED_PIN, GPIO.OUT) @@ -87,48 +85,6 @@ def setup(): pygame.mixer.init() -def _make_beep_buffer( - freq: int = 1000, duration: float = 0.15, rate: int = 44100 -) -> bytes: - """Synthesize a short square-wave beep as signed-16-bit stereo PCM. - - Matches the mixer format from ``pre_init`` (rate, -16, 2 channels) so it - feeds straight into ``pygame.mixer.Sound``. No audio file, no extra dep. - """ - n = int(rate * duration) - buf = array.array("h") - for i in range(n): - v = 32767 if math.sin(2 * math.pi * freq * (i / rate)) >= 0 else -32767 - buf.append(v) - buf.append(v) # duplicate to stereo - return buf.tobytes() - - -_BEEP_BUFFER = _make_beep_buffer() - - -class Sound: - """Audio capability injected into styles so each can own its ringing tone.""" - - def __init__(self): - self._beep = pygame.mixer.Sound(_BEEP_BUFFER) - - def play_music(self, path: str): - """Load and play a music file on an endless loop.""" - try: - pygame.mixer.music.load(path) - pygame.mixer.music.set_volume(1.0) - pygame.mixer.music.play(-1) - except Exception as e: - logging.error(f"Error loading audio file: {e}") - if "pytest" not in sys.modules: - sys.exit(1) - - def play_beep(self): - """Play one short beep tone.""" - self._beep.play() - - def set_led(on): """Turns the LED on or off (LOW = ON, HIGH = OFF)""" if "GPIO" in globals() and hasattr(GPIO, "output"): @@ -145,18 +101,24 @@ def run_alarm( ): """Run the alarm clock with the given style until the style signals stop. - The runner owns only the shared button sampling and cleanup; each style - owns its own ringing tone (via the injected ``Sound``) and LED behaviour. + The runner owns only the audio engine lifecycle (mixer init/quit), button + sampling, and cleanup; each style owns its own ringing tone (via pygame + directly) and LED behaviour. """ style_cls = get_style(style_name) # validate before touching hardware if music_file is None: music_file = os.environ.get("MUSIC_FILE", DEFAULT_MUSIC_FILE) setup() - sound = Sound() - style = style_cls(set_led, sound, music_file) + style = style_cls(set_led, music_file) logging.info("Alarm clock started.") - style.start() # style kicks off its own tone + initial LED + try: + style.start() # style kicks off its own tone + initial LED + except Exception: + logging.exception("Error starting alarm") + if "pytest" not in sys.modules: + sys.exit(1) + raise try: while True: