Validate player control config references before using them#3347
Validate player control config references before using them#3347teancom wants to merge 2 commits intomusic-assistant:devfrom
Conversation
|
One thing I didn't do in this PR is automatically clean up the old stale entries. I'm a little worried about anything along those lines possibly removing configuration for things that just haven't been initialized yet. If we want, I can look into that in a follow-up PR or add it to this one if we want to do it all at once. Or we can leave it as is and let people clean up their configs manually. |
music_assistant/models/player.py
Outdated
| ): | ||
| return conf_str | ||
| # configured ID doesn't resolve to any known player or control | ||
| self.logger.warning( |
There was a problem hiding this comment.
There is a high chance that this will now be logged every time at startup due to the fact that players and controls get added asynchronously - this might be confusing to the user. I would use debug log level here.
On the other hand, if the value is explicitly set by a user, it might be strange to just fallback to something else. We may want to consider only doing a fallback if the raw value is actually None, which means its not set by the user.
self.config.get_value --> contains calculated value
self.mass.config.get_raw_player_config_value --> contains the actually stored value
If the raw value is None, it means it has not been explicitly set
There was a problem hiding this comment.
That leads to the original issue (pointing to non-existent controls). But there may be a way to reconcile at the end. I'm gonna go have a hard think about it.
The volume_control, mute_control, and power_control properties trusted config values without verifying the referenced player or control still exists. After the universal player consolidation, stale config entries (e.g. old Chromecast player IDs) caused volume commands to silently fail. Now validates that config IDs resolve to known constants or existing players/controls before using them, falling through to auto-detection otherwise. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Instead of immediately discarding unresolvable control configs, return them during startup so async players (e.g. Sendspin) have time to register. After 30 seconds, _reconcile_stale_control_configs() clears any configs that still don't resolve, allowing auto-detection to take over. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
2 participants
Myself and at least one other person have seen that, post-universal player consolidation, our WiiM devices always show 'zero' as the volume, and any changes aren't reflected. I believe that's not anything WiiM-specific, but that we previously had DNLA/Chromecast/whatever entries that aren't used anymore, but are being referenced on start. This fixes that, at least for me locally (I've been testing it all day).
The volume_control, mute_control, and power_control properties trusted config values without verifying the referenced player or control still exists. After the universal player consolidation, stale config entries (e.g. old Chromecast player IDs) caused volume commands to silently fail.
Now validates that config IDs resolve to known constants or existing players/controls before using them, falling through to auto-detection otherwise.