Skip to content

Stop the current song before starting a new one - #16

Merged
bharvey88 merged 1 commit into
betafrom
fix/interrupt-song-on-new-press
Aug 26, 2026
Merged

bharvey88 merged 1 commit into
betafrom
fix/interrupt-song-on-new-press

Conversation

@bharvey88

Copy link
Copy Markdown
Contributor

Version: 26.8.26.1

What does this implement/fix?

Pressing a second song button while a song is playing started the new LED effect but left the old song playing. The new song never started.

Rtttl::play() returns early when a song is already running:

void Rtttl::play(std::string rtttl) {
  if (this->state_ != State::STATE_STOPPED && this->state_ != State::STATE_STOPPING) {
    ...
    ESP_LOGW(TAG, "Already playing: %s", name.c_str());
    return;
  }

The light.turn_on actions in play_song_* run before that and are not conditional, so the lights always switched and the audio never did.

Each of the seven play_song_* scripts now calls rtttl.stop first. On the output path Rtttl::stop() sets the level to 0 and moves straight to STATE_STOPPED in the same call, so the following rtttl.play() sees a stopped component and starts normally.

rtttl.stop() does not fire on_finished_playback (only finish_() does), so interrupting a song does not run the all_lights_off + ShouldSleep handler and cannot put the ornament to sleep mid-song.

play_buzzer is left alone. It has the same limitation, but it deliberately does not set song_active, and making it interrupt a song would leave that flag stale and let the API action trigger a sleep. Worth a separate look.

Types of changes

  • Bugfix (fixed change that fixes an issue)
  • New feature (thanks!)
  • Breaking change (repair/feature that breaks existing functionality)
  • Dependency Update - Does not publish
  • Other - Does not publish
  • Website of github readme file update - Does not publish
  • Github workflows - Does not publish

Checklist / Checklijst:

  • The code change has been tested and works locally
  • The code change has not yet been tested

If user-visible functionality or configuration variables are added/modified:

  • Added/updated documentation for the web page

esphome config passes on both H-3.yaml and H-3D.yaml, and the merged config renders all seven rtttl.stop actions. Not yet tested on hardware.

🤖 Generated with Claude Code

Rtttl::play() returns early when a song is already running, so pressing a
second button started the new LED effect while the old song kept playing.
Each play_song_* script now stops playback before setting up the new song.

Version 26.8.26.1

🤖 Generated with [Claude Code](https://claude.com/claude-code)
@bharvey88 bharvey88 added the bugfix Something isn't working label Aug 26, 2026
@coderabbitai

coderabbitai Bot commented Aug 26, 2026

Copy link
Copy Markdown

Important

  • 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: f28a2ad9-a76d-4bd9-9312-ce164e094687


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@bharvey88
bharvey88 merged commit 7db46c3 into beta Aug 26, 2026
10 checks passed
@bharvey88
bharvey88 deleted the fix/interrupt-song-on-new-press branch August 28, 2026 00:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bugfix Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant