Skip to content

feat(home-assistant): play and pause music by room, not by speaker name - #1794

Merged
johnae merged 1 commit into
mainfrom
ha-music-by-room
Sep 26, 2026
Merged

johnae merged 1 commit into
mainfrom
ha-music-by-room

Conversation

@johnae

@johnae johnae commented Sep 25, 2026

Copy link
Copy Markdown
Owner

Music failed from the kitchen Voice PE, but the same requests worked from the phone.

The Voice PE tells the model which room it is in. With that context, the model
also put the artist in HassMediaSearchAndPlay's name field, which selects a
speaker by its name, so Home Assistant searched for a speaker called "Dolly
Style" and failed. Stopping had the same problem through HassMediaPause: a
room was passed as a speaker name, a tool that doesn't exist was called, or
nothing happened at all.

Two script tools now take only a room:

  • Spela musik: what to play, its type (artist, album or track), the artist,
    and the room. It finds the Music Assistant player in that room and calls
    music_assistant.play_media. "köket" and "vardagsrummet" match Kök and
    Vardagsrum.
  • Pausa musik: pauses the player in the room, or every playing player when
    no room is given.

The Eris prompt now points the model at these two scripts and tells it to
ignore background speech that is not meant for it. The Voice PE's microphone
had picked up children and conversation after "Menade du …?". The docs contain
the updated prompt and list the new exposures.

Tested with the Voice PE's device context, speakers muted:

  • 8/8 play requests correct (kitchen by default, a named room, the phone
    asking for a room)
  • 8/8 stop phrasings correct
  • background chatter answered with "Okej." and nothing played

🤖 Generated with Claude Code

https://claude.ai/code/session_01XfYTVcQVjCevVeoJofu8FF

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XfYTVcQVjCevVeoJofu8FF
@claude

claude Bot commented Sep 25, 2026 •

Copy link
Copy Markdown
Contributor

Claude finished @johnae's task —— View job


Review

  • Read repo guidelines (DEVELOPMENT.md)
  • Review profiles/home-assistant.nix
  • Review docs/home-assistant-runtime.md
  • Post review feedback

The diagnosis is right and the shape of the fix is good: taking a room instead of a free-text name removes the whole class of "speaker called Dolly Style" failures, the shared templates are factored into let bindings rather than copy-pasted, and the error path that returns the list of rooms with speakers is a genuinely nice touch for a model-facing tool. Comments explain the why, in keeping with the rest of the file. Notes below, roughly in order of how much they'd bite.


1. Prefix matching on room names is ambiguous and order-dependent

profiles/home-assistant.nix:20-22

{% for a in areas() if not ns.p and r.startswith(area_name(a) | lower) %}
  {% set ns.p = area_entities(a) | select('in', ma) | first | default("") %}
{% endfor %}

Two problems stack here:

  • Prefixes collide. If you ever add "Sovrum 2" next to "Sovrum", then rum = "sovrum 2" matches both, and which one wins depends on the order areas() happens to return. Same for any "Kök" / "Köksö" pair.
  • A matching room without a speaker falls through. The if not ns.p guard doesn't stop at the matched area, it stops at the first area that matched and yielded a player. So if "Sovrum" matches first but has no Music Assistant player, the loop keeps going and can land on a different area whose name is also a prefix — music starts in the wrong room instead of returning Ingen högtalare i ….
  • Conversely, anything that isn't a plain prefix misses: "i köket", "kökets högtalare", "vardagsrummet därinne".

Home Assistant already has the mechanism for this: area aliases. Set köket/vardagsrummet as aliases on the areas and area_id(rum) resolves name or alias directly, which also helps the built-in intents. Something like: try area_id(rum) first, and only fall back to the prefix loop if it comes back undefined — and when a room does match, commit to it rather than continuing the loop. That also lets the comment at line 19 shrink to "aliases carry the definite forms".

2. Which speaker in the room is arbitrary

profiles/home-assistant.nix:21 — … | first picks whatever area_entities() returns first. For a room with one player that's fine; the day a room has a stereo pair, a TV cast target, or a second MA player, the choice silently depends on registry ordering, and it can change across restarts. Worth either sorting deterministically, or preferring a specific entity, or at minimum documenting that one player per room is an assumption.

3. pausa_musik can report None as a room

profiles/home-assistant.nix:281

{{ {'pausad': (spelare | trim).split(',') | reject('eq', '') | map('area_name') | list} }}

In the no-room branch this pauses every playing MA player, including any that aren't assigned to an area — area_name returns None for those, so the model gets {'pausad': [None]} and will try to say it. roomsWithSpeakers (line 27) already guards this with reject('none'); the same filter belongs here. Same latent issue for area_name(spelare) at line 220, though there it's unreachable since the player was found via an area.

4. media_pause vs. what "stäng av musiken" means

profiles/home-assistant.nix:276 uses media_player.media_pause, but the tool description (line 233) advertises "pausa, stoppa, stänga av eller sluta spela". Radio streams and some providers don't support pause — supported_features won't have PAUSE, and the call either errors or no-ops, which is exactly the "nothing happened at all" symptom this PR is fixing. Consider media_player.media_stop (or branching on supported_features) so the stop phrasings actually stop.

Related: the no-room branch only selects is_state('playing'). A player in buffering when the command arrives is skipped.

5. spelare | trim — pick one convention

pausa_musik trims four times (lines 271, 277, 281) while spela_musik doesn't (lines 198, 211, 220). HA strips rendered template results, so neither is needed and the trims are noise — but the asymmetry reads like one of the two is relying on behaviour the other doesn't. Worth making them the same.

6. The templated data dict is clever but undocumented

profiles/home-assistant.nix:212-218 renders a Python-repr dict and relies on HA's native-type rendering to literal_eval it back. It does work, and it correctly quotes names containing apostrophes, but it's the one construct in this file with no comment explaining why it isn't the ordinary static-keys form. A line saying "artist must be absent, not empty, so the whole dict is built in the template" would save the next reader (you, in six months) the archaeology.

7. Hidden coupling on a variable named rum

musicPlayerInRoom (line 15) and noSpeakerReply (line 29) silently require a rum in scope. It works because both call sites define the field, but nothing in the binding says so, and pausa_musik has to wrap the whole thing in {% if rum is defined %} (line 243) to compensate. Making them Nix functions — musicPlayerInRoom = room: ''…'' — makes the dependency explicit and lets pausa_musik pass its own expression instead of nesting a multi-line template inside another template.

8. The prompt is the only thing stopping the built-in intents

"Never use HassMediaSearchAndPlay" / "Never use HassMediaPause" (docs/home-assistant-runtime.md:50,55) only holds as far as the model obeys. Those intents exist because media player entities are exposed to Assist. The docs already explain why the native Sonos players are unexposed (line 82-83); it'd be worth stating explicitly whether the Music Assistant players stay exposed and why, since that's what decides whether the model can still reach the intent you just told it not to use. If they don't need to be exposed, unexposing them is a much stronger fix than a prompt line.

9. Small docs nit

docs/home-assistant-runtime.md:69 runs to ~110 characters while the rest of the file wraps around 76. Re-wrapping keeps the diff readable.


Performance: negligible. Each call walks every area once and calls integration_entities two or three times. One cosmetic point: rum_med_hogtalare (lines 191, 249) is computed on every invocation but only read in the error branch — it could move inside the then.

Security: nothing concerning. namn and rum reach templates as Jinja variables, never concatenated into template source, so there's no template injection even though both are model-controlled. The one thing the model fully controls is media_id, which Music Assistant will resolve as a URI if it looks like one — low risk on a LAN voice assistant, worth knowing it exists.

Test coverage: none is really possible here — these templates only execute inside a running HA — and the repo has no HA config check to extend. The manual matrix in the PR description (8/8 play, 8/8 stop, background chatter) is good evidence but lives only in the PR. Since the cases that will actually break are the ones listed above, it'd be worth extending the manual pass with: a room that has no speaker, pause with nothing playing, pause with two rooms playing, and a room name said as "i köket".

I couldn't run world lint / world check — Nix isn't available in this environment, so the Nix formatting and evaluation are unverified.
• branch ha-music-by-room

@johnae
johnae merged commit 83ce21b into main Sep 26, 2026
3 checks passed
@johnae
johnae deleted the ha-music-by-room branch September 26, 2026 10:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant