From b1820759adaa077ee111144a18f3894fdec93590 Mon Sep 17 00:00:00 2001 From: ded-furby <190979964+ded-furby@users.noreply.github.com> Date: Mon, 1 Jun 2026 18:04:35 +1000 Subject: [PATCH 1/5] Fix custom joke sound playback --- tests/test_sound.py | 20 ++++++++++ trushell/chronoterm/sound.py | 77 ++++++++++++++++++++++++++++-------- trushell/pyfunny.py | 16 ++++---- 3 files changed, 89 insertions(+), 24 deletions(-) diff --git a/tests/test_sound.py b/tests/test_sound.py index 066d50f..413b618 100644 --- a/tests/test_sound.py +++ b/tests/test_sound.py @@ -1,10 +1,12 @@ import subprocess from trushell.chronoterm import sound +from trushell import pyfunny def test_play_alarm_uses_quiet_subprocess(monkeypatch): calls = [] + monkeypatch.setattr(sound.sys, "platform", "linux") def fake_which(name: str) -> str | None: return "/usr/bin/" + name if name == "paplay" else None @@ -26,3 +28,21 @@ def fake_run(cmd, stdout, stderr, check): assert calls[0]["stderr"] == subprocess.DEVNULL assert calls[0]["check"] is False assert calls[0]["cmd"][0] == "paplay" + + +def test_play_sound_uses_requested_sound_file(monkeypatch, tmp_path): + sound_file = tmp_path / "custom-sound.mp3" + sound_file.write_text("not real audio") + calls = [] + + monkeypatch.setattr(pyfunny, "_sound_path", lambda filename: sound_file) + + def fake_play_file(path): + calls.append(path) + + monkeypatch.setattr(sound, "play_audio_file", fake_play_file, raising=False) + monkeypatch.setattr(pyfunny, "play_audio_file", fake_play_file, raising=False) + + pyfunny._play_sound("custom-sound.mp3") + + assert calls == [sound_file] diff --git a/trushell/chronoterm/sound.py b/trushell/chronoterm/sound.py index 80c6dec..d992e79 100644 --- a/trushell/chronoterm/sound.py +++ b/trushell/chronoterm/sound.py @@ -1,8 +1,64 @@ from __future__ import annotations -import os -import sys + import shutil import subprocess +import sys +from pathlib import Path + + +def _run_quietly(cmd: list[str]) -> bool: + result = subprocess.run( + cmd, + stdout=subprocess.DEVNULL, + stderr=subprocess.DEVNULL, + check=False, + ) + return result.returncode == 0 + + +def _resolve_windows_sound_path(path: Path) -> Path | None: + if path.suffix.lower() == ".wav": + return path + + wav_path = path.with_suffix(".wav") + if wav_path.exists(): + return wav_path + + return None + + +def play_audio_file(path: str | Path) -> None: + """Play a specific audio asset when a platform player is available.""" + sound_path = Path(path) + + if sys.platform.startswith("win"): + playable_path = _resolve_windows_sound_path(sound_path) + if playable_path is None: + raise RuntimeError(f"Windows playback requires a .wav fallback for {sound_path.name}") + + import winsound + + winsound.PlaySound(str(playable_path), winsound.SND_FILENAME) + return + + if sys.platform == "darwin": + if not shutil.which("afplay"): + raise RuntimeError("afplay is unavailable") + if _run_quietly(["afplay", str(sound_path)]): + return + raise RuntimeError(f"afplay failed for {sound_path}") + + for player in ( + ["paplay", str(sound_path)], + ["aplay", str(sound_path)], + ["ffplay", "-nodisp", "-autoexit", str(sound_path)], + ["mpg123", "-q", str(sound_path)], + ["mpg321", "-q", str(sound_path)], + ): + if shutil.which(player[0]) and _run_quietly(player): + return + + raise RuntimeError(f"No supported Linux audio player could play {sound_path}") def play_alarm() -> None: @@ -14,12 +70,7 @@ def play_alarm() -> None: winsound.Beep(900, 400) elif sys.platform == "darwin": - subprocess.run( - ["afplay", "/System/Library/Sounds/Glass.aiff"], - stdout=subprocess.DEVNULL, - stderr=subprocess.DEVNULL, - check=False, - ) + _run_quietly(["afplay", "/System/Library/Sounds/Glass.aiff"]) else: # Linux/Unix for cmd in [ @@ -28,13 +79,7 @@ def play_alarm() -> None: ["canberra-gtk-play", "--id=alarm-clock-elapsed"], ]: if shutil.which(cmd[0]): - result = subprocess.run( - cmd, - stdout=subprocess.DEVNULL, - stderr=subprocess.DEVNULL, - check=False, - ) - if result.returncode == 0: + if _run_quietly(cmd): return sys.stdout.write("\007" * 3) @@ -42,4 +87,4 @@ def play_alarm() -> None: except Exception: sys.stdout.write("\007") - sys.stdout.flush() \ No newline at end of file + sys.stdout.flush() diff --git a/trushell/pyfunny.py b/trushell/pyfunny.py index 32c0c1d..62c0607 100644 --- a/trushell/pyfunny.py +++ b/trushell/pyfunny.py @@ -7,7 +7,7 @@ import typer from .chronoterm.state import StateStore -from .chronoterm.sound import play_alarm +from .chronoterm.sound import play_alarm, play_audio_file DEFAULT_JOKE_CHARACTER = "cow" DEFAULT_JOKE_SOUND = "cow-sound.mp3" @@ -24,14 +24,14 @@ def _play_sound(filename: str) -> None: typer.secho(f"Sound file missing: {sound_path}", fg=typer.colors.YELLOW) return - # Note: Your play_alarm() currently plays a system beep/tone. - # If you want it to play specific MP3s, you'd need to update play_alarm - # to accept a file path. For now, this just triggers the alarm sound - # as a notification that a joke is coming. try: - play_alarm() + play_audio_file(sound_path) except Exception: - typer.secho("Unable to play sound. Continuing without audio.", fg=typer.colors.YELLOW) + typer.secho("Unable to play selected sound. Falling back to alarm.", fg=typer.colors.YELLOW) + try: + play_alarm() + except Exception: + typer.secho("Unable to play sound. Continuing without audio.", fg=typer.colors.YELLOW) def _joke_preferences() -> tuple[str, str]: @@ -56,4 +56,4 @@ def joke() -> str: def joke_trex() -> str: joke_text = pyjokes.get_joke() _play_sound("trex-sound.mp3") - return cowsay.trex(joke_text) \ No newline at end of file + return cowsay.trex(joke_text) From 1195e7628ebacf828f3b15d63641466e7c826ce3 Mon Sep 17 00:00:00 2001 From: ded-furby <190979964+ded-furby@users.noreply.github.com> Date: Mon, 1 Jun 2026 21:34:15 +1000 Subject: [PATCH 2/5] Avoid alarm fallback after custom playback attempts --- tests/test_sound.py | 35 ++++++++++++++++++++++++++++++ trushell/chronoterm/sound.py | 41 ++++++++++++++++++++++++------------ trushell/pyfunny.py | 27 +++++++++++++++++++----- 3 files changed, 84 insertions(+), 19 deletions(-) diff --git a/tests/test_sound.py b/tests/test_sound.py index 413b618..ec03c3b 100644 --- a/tests/test_sound.py +++ b/tests/test_sound.py @@ -39,6 +39,7 @@ def test_play_sound_uses_requested_sound_file(monkeypatch, tmp_path): def fake_play_file(path): calls.append(path) + return True monkeypatch.setattr(sound, "play_audio_file", fake_play_file, raising=False) monkeypatch.setattr(pyfunny, "play_audio_file", fake_play_file, raising=False) @@ -46,3 +47,37 @@ def fake_play_file(path): pyfunny._play_sound("custom-sound.mp3") assert calls == [sound_file] + + +def test_play_sound_falls_back_when_custom_player_unavailable( + monkeypatch, tmp_path +): + sound_file = tmp_path / "custom-sound.mp3" + sound_file.write_text("not real audio") + alarm_calls = [] + + monkeypatch.setattr(pyfunny, "_sound_path", lambda filename: sound_file) + monkeypatch.setattr( + pyfunny, + "play_audio_file", + lambda path: (_ for _ in ()).throw(RuntimeError("no supported player")), + ) + monkeypatch.setattr(pyfunny, "play_alarm", lambda: alarm_calls.append("alarm")) + + pyfunny._play_sound("custom-sound.mp3") + + assert alarm_calls == ["alarm"] + + +def test_play_sound_skips_alarm_after_custom_playback_attempt(monkeypatch, tmp_path): + sound_file = tmp_path / "custom-sound.mp3" + sound_file.write_text("not real audio") + alarm_calls = [] + + monkeypatch.setattr(pyfunny, "_sound_path", lambda filename: sound_file) + monkeypatch.setattr(pyfunny, "play_audio_file", lambda path: False) + monkeypatch.setattr(pyfunny, "play_alarm", lambda: alarm_calls.append("alarm")) + + pyfunny._play_sound("custom-sound.mp3") + + assert alarm_calls == [] diff --git a/trushell/chronoterm/sound.py b/trushell/chronoterm/sound.py index d992e79..f334ce2 100644 --- a/trushell/chronoterm/sound.py +++ b/trushell/chronoterm/sound.py @@ -27,27 +27,31 @@ def _resolve_windows_sound_path(path: Path) -> Path | None: return None -def play_audio_file(path: str | Path) -> None: - """Play a specific audio asset when a platform player is available.""" +def play_audio_file(path: str | Path) -> bool: + """Play a specific audio asset when a platform player is available. + + Returns False when a player was attempted but did not confirm success. + """ sound_path = Path(path) if sys.platform.startswith("win"): playable_path = _resolve_windows_sound_path(sound_path) if playable_path is None: - raise RuntimeError(f"Windows playback requires a .wav fallback for {sound_path.name}") + raise RuntimeError( + f"Windows playback requires a .wav fallback for {sound_path.name}" + ) import winsound winsound.PlaySound(str(playable_path), winsound.SND_FILENAME) - return + return True if sys.platform == "darwin": if not shutil.which("afplay"): raise RuntimeError("afplay is unavailable") - if _run_quietly(["afplay", str(sound_path)]): - return - raise RuntimeError(f"afplay failed for {sound_path}") + return _run_quietly(["afplay", str(sound_path)]) + attempted_player = False for player in ( ["paplay", str(sound_path)], ["aplay", str(sound_path)], @@ -55,8 +59,13 @@ def play_audio_file(path: str | Path) -> None: ["mpg123", "-q", str(sound_path)], ["mpg321", "-q", str(sound_path)], ): - if shutil.which(player[0]) and _run_quietly(player): - return + if shutil.which(player[0]): + attempted_player = True + if _run_quietly(player): + return True + + if attempted_player: + return False raise RuntimeError(f"No supported Linux audio player could play {sound_path}") @@ -66,25 +75,29 @@ def play_alarm() -> None: try: if sys.platform.startswith("win"): import winsound + winsound.Beep(1200, 400) winsound.Beep(900, 400) - + elif sys.platform == "darwin": _run_quietly(["afplay", "/System/Library/Sounds/Glass.aiff"]) - + else: # Linux/Unix for cmd in [ - ["paplay", "/usr/share/sounds/freedesktop/stereo/alarm-clock-elapsed.oga"], + [ + "paplay", + "/usr/share/sounds/freedesktop/stereo/alarm-clock-elapsed.oga", + ], ["aplay", "/usr/share/sounds/alsa/Front_Center.wav"], ["canberra-gtk-play", "--id=alarm-clock-elapsed"], ]: if shutil.which(cmd[0]): if _run_quietly(cmd): return - + sys.stdout.write("\007" * 3) sys.stdout.flush() - + except Exception: sys.stdout.write("\007") sys.stdout.flush() diff --git a/trushell/pyfunny.py b/trushell/pyfunny.py index 62c0607..7daac38 100644 --- a/trushell/pyfunny.py +++ b/trushell/pyfunny.py @@ -19,24 +19,41 @@ def _sound_path(filename: str) -> Path: def _play_sound(filename: str) -> None: sound_path = _sound_path(filename) - + if not sound_path.exists(): typer.secho(f"Sound file missing: {sound_path}", fg=typer.colors.YELLOW) return try: - play_audio_file(sound_path) + played_selected_sound = play_audio_file(sound_path) except Exception: - typer.secho("Unable to play selected sound. Falling back to alarm.", fg=typer.colors.YELLOW) + typer.secho( + "Unable to play selected sound. Falling back to alarm.", + fg=typer.colors.YELLOW, + ) try: play_alarm() except Exception: - typer.secho("Unable to play sound. Continuing without audio.", fg=typer.colors.YELLOW) + typer.secho( + "Unable to play sound. Continuing without audio.", + fg=typer.colors.YELLOW, + ) + return + + if not played_selected_sound: + typer.secho( + "Selected sound playback was attempted but failed. " + "Skipping fallback to avoid overlapping audio.", + fg=typer.colors.YELLOW, + ) def _joke_preferences() -> tuple[str, str]: state = StateStore().load() - return state.joke_character or DEFAULT_JOKE_CHARACTER, state.joke_sound or DEFAULT_JOKE_SOUND + return ( + state.joke_character or DEFAULT_JOKE_CHARACTER, + state.joke_sound or DEFAULT_JOKE_SOUND, + ) def _render_joke(character_name: str, text: str) -> str: From ddb30f612a601ee2e8fabce3fd3e7e3e4e9261c1 Mon Sep 17 00:00:00 2001 From: ded-furby <190979964+ded-furby@users.noreply.github.com> Date: Mon, 1 Jun 2026 21:45:07 +1000 Subject: [PATCH 3/5] Avoid alarm fallback after partial custom playback --- tests/test_sound.py | 28 ++++++++++++++++++++++++---- trushell/chronoterm/sound.py | 12 +++++++++--- trushell/pyfunny.py | 15 +++++++++++++-- 3 files changed, 46 insertions(+), 9 deletions(-) diff --git a/tests/test_sound.py b/tests/test_sound.py index ec03c3b..5805a81 100644 --- a/tests/test_sound.py +++ b/tests/test_sound.py @@ -49,9 +49,7 @@ def fake_play_file(path): assert calls == [sound_file] -def test_play_sound_falls_back_when_custom_player_unavailable( - monkeypatch, tmp_path -): +def test_play_sound_falls_back_when_custom_player_unavailable(monkeypatch, tmp_path): sound_file = tmp_path / "custom-sound.mp3" sound_file.write_text("not real audio") alarm_calls = [] @@ -60,7 +58,9 @@ def test_play_sound_falls_back_when_custom_player_unavailable( monkeypatch.setattr( pyfunny, "play_audio_file", - lambda path: (_ for _ in ()).throw(RuntimeError("no supported player")), + lambda path: (_ for _ in ()).throw( + sound.AudioPlaybackUnavailable("no supported player") + ), ) monkeypatch.setattr(pyfunny, "play_alarm", lambda: alarm_calls.append("alarm")) @@ -81,3 +81,23 @@ def test_play_sound_skips_alarm_after_custom_playback_attempt(monkeypatch, tmp_p pyfunny._play_sound("custom-sound.mp3") assert alarm_calls == [] + + +def test_play_sound_skips_alarm_after_unexpected_playback_exception( + monkeypatch, tmp_path +): + sound_file = tmp_path / "custom-sound.mp3" + sound_file.write_text("not real audio") + alarm_calls = [] + + monkeypatch.setattr(pyfunny, "_sound_path", lambda filename: sound_file) + + def fake_play_file(path): + raise RuntimeError("player failed after starting playback") + + monkeypatch.setattr(pyfunny, "play_audio_file", fake_play_file) + monkeypatch.setattr(pyfunny, "play_alarm", lambda: alarm_calls.append("alarm")) + + pyfunny._play_sound("custom-sound.mp3") + + assert alarm_calls == [] diff --git a/trushell/chronoterm/sound.py b/trushell/chronoterm/sound.py index f334ce2..ae8a0ea 100644 --- a/trushell/chronoterm/sound.py +++ b/trushell/chronoterm/sound.py @@ -6,6 +6,10 @@ from pathlib import Path +class AudioPlaybackUnavailable(RuntimeError): + """Raised when the host has no supported way to play a selected asset.""" + + def _run_quietly(cmd: list[str]) -> bool: result = subprocess.run( cmd, @@ -37,7 +41,7 @@ def play_audio_file(path: str | Path) -> bool: if sys.platform.startswith("win"): playable_path = _resolve_windows_sound_path(sound_path) if playable_path is None: - raise RuntimeError( + raise AudioPlaybackUnavailable( f"Windows playback requires a .wav fallback for {sound_path.name}" ) @@ -48,7 +52,7 @@ def play_audio_file(path: str | Path) -> bool: if sys.platform == "darwin": if not shutil.which("afplay"): - raise RuntimeError("afplay is unavailable") + raise AudioPlaybackUnavailable("afplay is unavailable") return _run_quietly(["afplay", str(sound_path)]) attempted_player = False @@ -67,7 +71,9 @@ def play_audio_file(path: str | Path) -> bool: if attempted_player: return False - raise RuntimeError(f"No supported Linux audio player could play {sound_path}") + raise AudioPlaybackUnavailable( + f"No supported Linux audio player could play {sound_path}" + ) def play_alarm() -> None: diff --git a/trushell/pyfunny.py b/trushell/pyfunny.py index 7daac38..b9748e2 100644 --- a/trushell/pyfunny.py +++ b/trushell/pyfunny.py @@ -7,7 +7,11 @@ import typer from .chronoterm.state import StateStore -from .chronoterm.sound import play_alarm, play_audio_file +from .chronoterm.sound import ( + AudioPlaybackUnavailable, + play_alarm, + play_audio_file, +) DEFAULT_JOKE_CHARACTER = "cow" DEFAULT_JOKE_SOUND = "cow-sound.mp3" @@ -26,7 +30,7 @@ def _play_sound(filename: str) -> None: try: played_selected_sound = play_audio_file(sound_path) - except Exception: + except AudioPlaybackUnavailable: typer.secho( "Unable to play selected sound. Falling back to alarm.", fg=typer.colors.YELLOW, @@ -39,6 +43,13 @@ def _play_sound(filename: str) -> None: fg=typer.colors.YELLOW, ) return + except Exception: + typer.secho( + "Selected sound playback failed unexpectedly. " + "Skipping fallback to avoid overlapping audio.", + fg=typer.colors.YELLOW, + ) + return if not played_selected_sound: typer.secho( From 449679b525c40bd031992195807bbfd9932f49f6 Mon Sep 17 00:00:00 2001 From: ded-furby <190979964+ded-furby@users.noreply.github.com> Date: Tue, 2 Jun 2026 11:05:08 +1000 Subject: [PATCH 4/5] Fix sound path subprocess compatibility --- tests/test_sound.py | 13 +++++++++++++ trushell/chronoterm/sound.py | 13 +++++++------ 2 files changed, 20 insertions(+), 6 deletions(-) diff --git a/tests/test_sound.py b/tests/test_sound.py index 5805a81..8cf5bd3 100644 --- a/tests/test_sound.py +++ b/tests/test_sound.py @@ -49,6 +49,19 @@ def fake_play_file(path): assert calls == [sound_file] +def test_play_audio_file_passes_string_path_to_linux_player(monkeypatch, tmp_path): + sound_file = tmp_path / "custom-sound.mp3" + sound_file.write_text("not real audio") + calls = [] + + monkeypatch.setattr(sound.sys, "platform", "linux") + monkeypatch.setattr(sound.shutil, "which", lambda name: "/usr/bin/" + name) + monkeypatch.setattr(sound, "_run_quietly", lambda cmd: calls.append(cmd) or True) + + assert sound.play_audio_file(sound_file) is True + assert calls == [["paplay", str(sound_file)]] + + def test_play_sound_falls_back_when_custom_player_unavailable(monkeypatch, tmp_path): sound_file = tmp_path / "custom-sound.mp3" sound_file.write_text("not real audio") diff --git a/trushell/chronoterm/sound.py b/trushell/chronoterm/sound.py index ae8a0ea..26592c1 100644 --- a/trushell/chronoterm/sound.py +++ b/trushell/chronoterm/sound.py @@ -37,6 +37,7 @@ def play_audio_file(path: str | Path) -> bool: Returns False when a player was attempted but did not confirm success. """ sound_path = Path(path) + sound_path_str = str(sound_path) if sys.platform.startswith("win"): playable_path = _resolve_windows_sound_path(sound_path) @@ -53,15 +54,15 @@ def play_audio_file(path: str | Path) -> bool: if sys.platform == "darwin": if not shutil.which("afplay"): raise AudioPlaybackUnavailable("afplay is unavailable") - return _run_quietly(["afplay", str(sound_path)]) + return _run_quietly(["afplay", sound_path_str]) attempted_player = False for player in ( - ["paplay", str(sound_path)], - ["aplay", str(sound_path)], - ["ffplay", "-nodisp", "-autoexit", str(sound_path)], - ["mpg123", "-q", str(sound_path)], - ["mpg321", "-q", str(sound_path)], + ["paplay", sound_path_str], + ["aplay", sound_path_str], + ["ffplay", "-nodisp", "-autoexit", sound_path_str], + ["mpg123", "-q", sound_path_str], + ["mpg321", "-q", sound_path_str], ): if shutil.which(player[0]): attempted_player = True From 302bc2b83480a3f6566a91e4de868fb1e277c5d1 Mon Sep 17 00:00:00 2001 From: ded-furby <190979964+ded-furby@users.noreply.github.com> Date: Tue, 2 Jun 2026 11:28:03 +1000 Subject: [PATCH 5/5] docs: note supported custom sound formats --- README.md | 3 +++ tests/test_sound.py | 20 ++++++++++++++++---- 2 files changed, 19 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index 8221363..886ae28 100644 --- a/README.md +++ b/README.md @@ -142,6 +142,9 @@ pytest tests/ The package metadata is in `pyproject.toml`; the runtime version exported by `trushell.__version__` should be kept in sync with it. +For custom joke sounds, prefer `.mp3` and `.wav` assets for the broadest +cross-platform playback support across the available audio backends. + ## License Apache-2.0. See [LICENSE](LICENSE) for details. diff --git a/tests/test_sound.py b/tests/test_sound.py index 8cf5bd3..d8acdf4 100644 --- a/tests/test_sound.py +++ b/tests/test_sound.py @@ -48,15 +48,27 @@ def fake_play_file(path): assert calls == [sound_file] - -def test_play_audio_file_passes_string_path_to_linux_player(monkeypatch, tmp_path): +def test_play_audio_file_uses_string_path_for_linux_players(monkeypatch, tmp_path): sound_file = tmp_path / "custom-sound.mp3" sound_file.write_text("not real audio") calls = [] monkeypatch.setattr(sound.sys, "platform", "linux") - monkeypatch.setattr(sound.shutil, "which", lambda name: "/usr/bin/" + name) - monkeypatch.setattr(sound, "_run_quietly", lambda cmd: calls.append(cmd) or True) + monkeypatch.setattr( + sound.shutil, + "which", + lambda name: "/usr/bin/paplay" if name == "paplay" else None, + ) + + def fake_run(cmd, stdout, stderr, check): + calls.append(cmd) + + class FakeResult: + returncode = 0 + + return FakeResult() + + monkeypatch.setattr(sound.subprocess, "run", fake_run) assert sound.play_audio_file(sound_file) is True assert calls == [["paplay", str(sound_file)]]