fix: Clear error message for music streaming #2736
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This clearly conveys to the users that the inability to stream music is not a problem with the integration or home assistant but instead something that Amazon doesn't allow. It also add media_type argument to the async_play_tts_cloud_say function to differentiate the error message for when it is a simple TTS message being sent or music
sayam93
changed the title
Pull Request Test Coverage Report for Build 12245896986Details
💛 - Coveralls |
alandtse
reviewed
Dec 9, 2024
This saves music streaming error message as a string instead of duplicating
for more information, see https://pre-commit.ci
Added the error message as constant to const.py file for better structuring
Imports the music streaming error string from const.py instead
for more information, see https://pre-commit.ci
alandtse
reviewed
Dec 9, 2024
Added more error messages as constants to const.py file for better structuring
for more information, see https://pre-commit.ci
Imports the error strings from const.py instead
for more information, see https://pre-commit.ci
alandtse
added a commit
that referenced
this pull request
Dec 10, 2024
fix: clarify error message for music streaming (#2736)
|
This PR breaks tts.speak: |
You're right. The above format doesn't work. Was it working for you previously? The only way this could break it is if the media_type for messages sent using tts action is set up as music by Home Assistant. I used the above format for my use case (both with type: announce and without) and that still works. |
|
I downgraded to 5.0.2 to test this. It did work before 5.0.3. I think HA sends TTS text in audio form as media_type music when the TTS speak or TTS cloud speak action is used in HA which is why this is now simply throwing an error. |
I've created a PR to fix this while still providing the correct error for music streaming #2742. |
|
May I suggest a change to the wording that was newly implemented? Perhaps it would be better to say: "Sorry, the media you're trying to play isn't supported by this method. This limitation is set by Amazon, and not by Alexa-Media-Player, Music-Assistant, nor Home-Assistant." With the dashes to have a more natural sounding message. |
|
Hey @sayam93, I am trying to play SiriusXM as well as I tried Spotify in Music Assistant and it's not working. I am getting that "sorry folks" message.. but both of these ARE supported by MA. It was working before but not now. Here are my logs: |
The new PR will hopefully be merged into a release soon. Anything that was working before 5.0.3 will work again after it is done. The only thing that will change is the message for music as it was initially supposed to. |
|
Hey @sayam93, unfortunately after updating to the latest push you made, SiriusXM and Spotify still don't work with MA for me. What's weird is, I can see it playing (and hearing it of course), yet I can't start it within MA. I believe it's because of the architecture of how MA was designed. I can start playing either one of them using mini-media-player which utilizes the skill for them respectively. Is there any though to mimic mini-media-player in doing the same? Techinically speaking, the error message we get saying that it's a limitation by Amazon, though 100% correct when trying to stream direct from local sources, isn't 100% correct, to say that it can be done, just not directly from a local source. aka, using a corresponding skill to play it with MA just being a middleman or skin to display info/and controls. I hope this makes sense what I'm trying to convey. Here are 2 photos. One showing the music stopped in MA and when I tap on the play button, I get that error message. The other photo shows MA displaying me actually playing the music - but because I used mini-media-player to initiate it. I have also included the yaml for the card portion to play the stations (both siriusxm and spotify are in the code - I use SpotifyPlus), as well as the script that is used by mini-media-player to start SiriusXM. Can the info I have supplied here, he considered or used in conjunction with MA?
|
The Echo entity created by Music Assistant does not have the ability to control an already playing stream is what you mean, right? Mini Media Player works because with the above script, you are technically issuing it a command as you would normally do with your voice. This has nothing to do with this Custom Integration. If you roll back to an an older version (anything <5.0.3), you'll simply get "To send TTS, please set the public URL in integration configuration". It's probably related to how Music Assistant server controls the Home Assistant entities created by it's integration to control the actual media_player entity. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This clearly conveys to the users that the inability to stream music is not a problem with the integration or home assistant but instead something that Amazon doesn't allow. It also add media_type argument to the async_play_tts_cloud_say function to differentiate the error message for when it is a simple TTS message being sent or music