Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -43,4 +43,11 @@ Progress auto-saves on every completed set and on Ctrl-C. Re-run the same workou

## Voice

Uses macOS `say` for coaching cues. Silently skipped on other platforms.
Uses macOS `say` or Linux `espeak`/`spd-say` for coaching cues. Silently skipped if no TTS is available.

On Linux, install a TTS engine:

```
sudo apt install espeak # Debian/Ubuntu
sudo dnf install espeak-ng # Fedora
```
44 changes: 40 additions & 4 deletions coach.py
Original file line number Diff line number Diff line change
Expand Up @@ -144,34 +144,70 @@ def exercises_match(a: list[Exercise], b: list[Exercise]) -> bool:


# ---------------------------------------------------------------------------
# Voice coaching (non-blocking macOS `say`)
# Voice coaching (macOS `say`, Linux `espeak`/`spd-say`)
# ---------------------------------------------------------------------------

_say_proc: subprocess.Popen | None = None


def _tts_cmd(text: str) -> list[str]:
"""Return the TTS command + args for the current platform."""
if sys.platform == "darwin":
return ["say", text]
# Linux: prefer espeak, fall back to spd-say
for cmd in ("espeak", "spd-say"):
if _which(cmd):
return [cmd, text]
Comment on lines +158 to +160
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Pass -w to spd-say in the synchronous path

When spd-say is the only Linux backend, say_sync() no longer blocks until the prompt finishes. The spd-say(1) docs say -w, --wait will "Wait till the message is spoken or discarded" (https://manpages.debian.org/testing/speech-dispatcher/spd-say.1.en.html), but _tts_cmd() currently returns only ['spd-say', text]. In timed_hold(), that makes the 3-2-1 countdown start while "Get in position" is still being spoken, so timed sets begin early on those machines.

Useful? React with 👍 / 👎.

return []


def _which(cmd: str) -> bool:
"""Check whether *cmd* is on PATH (cached)."""
if cmd not in _which_cache:
_which_cache[cmd] = (
subprocess.call(
["which", cmd],
stdout=subprocess.DEVNULL,
stderr=subprocess.DEVNULL,
Comment on lines +168 to +171
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Avoid shelling out to which on unsupported platforms

This helper now invokes which before say()/say_sync() reach their FileNotFoundError guard. On Windows, which itself is not present, so _tts_cmd() raises on the first voice cue and the app crashes instead of silently skipping speech as it did before this change. Using shutil.which() or catching lookup failures inside _which() would preserve the advertised no-TTS fallback.

Useful? React with 👍 / 👎.

)
== 0
)
return _which_cache[cmd]


_which_cache: dict[str, bool] = {}
Comment on lines +164 to +178
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

critical

There are two issues in this block:

  1. _which_cache is used in _which before it is defined, which will cause a NameError at runtime.
  2. The implementation of _which using subprocess.call(['which', ...]) is not robust. If the which command is not available on the system, it will raise an unhandled FileNotFoundError.

I suggest moving the definition of _which_cache before _which and using the more robust and standard shutil.which to find executables. This will fix the bug and make the code cleaner.

Note: You'll need to add import shutil at the top of the file for this suggestion to work.

_which_cache: dict[str, bool] = {}


def _which(cmd: str) -> bool:
    """Check whether *cmd* is on PATH (cached)."""
    if cmd not in _which_cache:
        _which_cache[cmd] = shutil.which(cmd) is not None
    return _which_cache[cmd]



def say(text: str) -> None:
global _say_proc
cmd = _tts_cmd(text)
if not cmd:
return
try:
if _say_proc and _say_proc.poll() is None:
_say_proc.terminate()
_say_proc = subprocess.Popen(
["say", text],
cmd,
stdout=subprocess.DEVNULL,
stderr=subprocess.DEVNULL,
)
except FileNotFoundError:
pass # not on macOS
pass


def say_sync(text: str, wait: float = 0) -> None:
"""Say something and optionally wait after it finishes."""
global _say_proc
cmd = _tts_cmd(text)
if not cmd:
if wait > 0:
time.sleep(wait)
return
Comment on lines +202 to +205
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

This logic causes say_sync to wait for wait seconds even if no TTS command is available and nothing is spoken. The docstring says 'optionally wait after it finishes', which implies that if speech doesn't happen, the wait should also be skipped. This could lead to unexpected delays if say_sync is called with a wait value on a system without TTS. This behavior is also inconsistent with the previous implementation which would not wait if the say command was not found.

    if not cmd:
        return

try:
if _say_proc and _say_proc.poll() is None:
_say_proc.terminate()
_say_proc = subprocess.Popen(
["say", text],
cmd,
stdout=subprocess.DEVNULL,
stderr=subprocess.DEVNULL,
)
Expand Down
22 changes: 17 additions & 5 deletions plank.sh
Original file line number Diff line number Diff line change
@@ -1,13 +1,25 @@
#!/bin/bash

# Cross-platform TTS: macOS `say`, Linux `espeak` or `spd-say`
speak() {
if command -v say &>/dev/null; then
say "$1"
elif command -v espeak &>/dev/null; then
espeak "$1"
elif command -v spd-say &>/dev/null; then
spd-say "$1"
Comment on lines +9 to +10
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Make plank.sh wait for spd-say before sleeping

The new spd-say branch changes this script's timing because spd-say returns as soon as it submits the utterance unless -w/--wait is passed; spd-say(1) documents that flag as "Wait till the message is spoken or discarded" (https://manpages.debian.org/testing/speech-dispatcher/spd-say.1.en.html). On Linux systems that have spd-say but not espeak, the following sleep 3, sleep 40, and sleep 60 start before the cue finishes, so spoken prompts overlap the countdown and the hold/rest intervals become shorter than advertised.

Useful? React with 👍 / 👎.

fi
}
Comment on lines +4 to +12
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

This implementation has two areas for improvement:

  1. It only passes the first argument ($1) to the TTS command. Using "$@" would make it more robust by handling all arguments.
  2. It checks for the existence of say, espeak, and spd-say on every call. It would be more efficient to perform this check only once when the script starts.

Here is a suggested refactoring that addresses both points by finding the appropriate TTS command once and defining a speak function that uses it.

Suggested change
speak() {
if command -v say &>/dev/null; then
say "$1"
elif command -v espeak &>/dev/null; then
espeak "$1"
elif command -v spd-say &>/dev/null; then
spd-say "$1"
fi
}
_tts_command=""
if command -v say &>/dev/null; then
_tts_command="say"
elif command -v espeak &>/dev/null; then
_tts_command="espeak"
elif command -v spd-say &>/dev/null; then
_tts_command="spd-say"
fi
speak() {
if [ -n "$_tts_command" ]; then
"$_tts_command" "$@"
fi
}


for set in 1 2 3; do
say "Set $set. Get in position."
speak "Set $set. Get in position."
sleep 3
say "Go"
speak "Go"
sleep 40
say "Done. Set $set complete."
speak "Done. Set $set complete."
if [ $set -lt 3 ]; then
say "Rest 60 seconds"
speak "Rest 60 seconds"
sleep 60
fi
done
say "RKC Planks done. 3 sets of 40 seconds."
speak "RKC Planks done. 3 sets of 40 seconds."