Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Add restore volume service #2773

Merged
merged 8 commits into from
Dec 28, 2024

Conversation

danielbrunt57
Copy link
Collaborator

This adds a new service: Alexa Media Player: Restore Previous Volume (alexa_media.restore_volume)

@coveralls
Copy link

coveralls commented Dec 26, 2024

Pull Request Test Coverage Report for Build 12500877169

Details

  • 6 of 24 (25.0%) changed or added relevant lines in 2 files are covered.
  • 2 unchanged lines in 1 file lost coverage.
  • Overall coverage increased (+0.09%) to 9.42%

Changes Missing Coverage Covered Lines Changed/Added Lines %
custom_components/alexa_media/services.py 5 23 21.74%
Files with Coverage Reduction New Missed Lines %
custom_components/alexa_media/services.py 2 30.56%
Totals Coverage Status
Change from base Build 12450004706: 0.09%
Covered Lines: 325
Relevant Lines: 3450

💛 - Coveralls

@alandtse alandtse merged commit eca4ef6 into alandtse:dev Dec 28, 2024
4 checks passed
@alandtse
Copy link
Owner

Please update the wiki.

@dpgh947
Copy link

dpgh947 commented Jan 19, 2025

I didn't think this warranted opening an issue (yet), but wanted to explain a possible problem I found with the new volume save/restore function. I use node red for all my automation stuff, and already had a notify flow that allows for loud TTS if requested. It uses a current state node to save the current volume level, then sets the loud volume, issues the notify, then sets the volume back. In a tinkering mood I thought I would update this to use the new restore volume action. I tested that a set volume call populated the previous volume attribute, and then calling the restore action did indeed restore it. But when put in to my flow, I found that the previous volume attribute was not being updated. On a hunch I thought this might be a timing issue caused by the volume set being followed immediately by the notify, and put a delay in between. 1 second made no difference, but 2 seconds made it work. So there is a timing issue somewhere in the mix - a set volume call from node red followed immediately by a TTS notify call prevents the previous volume attribute update somehow, but a 2 second delay in between makes it work. Any thoughts?

@danielbrunt57
Copy link
Collaborator Author

The previous volume level is only designed to immediately update when the volume change is made within AMP. When it's done externally, it may never update or if it does, it could be quite delayed.

@dpgh947
Copy link

dpgh947 commented Jan 20, 2025

I'm calling media_player.volume_set

@danielbrunt57
Copy link
Collaborator Author

danielbrunt57 commented Jan 21, 2025

I'm calling media_player.volume_set

a set volume call from node red followed immediately by a TTS notify call prevents the previous volume attribute update somehow, but a 2 second delay in between makes it work.

You are contradicting yourself.

@danielbrunt57
Copy link
Collaborator Author

I'll gladly revert the changes if people are not happy with it and someone else can tackle it.

@alandtse
Copy link
Owner

@danielbrunt57 thanks for all your help. Do you need a break? You're sounding like burn out imho. I'm only seeing some of the chatter, but remember it's ok to step away.

@danielbrunt57
Copy link
Collaborator Author

danielbrunt57 commented Jan 21, 2025

@alandtse Yes I do! My townhouse sold last week and I now have until the end of February to pack up and vacate! I've already de-automated most of my motion sensors, lights, switches and door locks. It sure is a pain having to remember to turn on a light when entering a room but it's even tougher to remember to turn it off afterwards!!

@dpgh947
Copy link

dpgh947 commented Jan 21, 2025

You are contradicting yourself.

I'm really not. I'm calling that action from node red. I could put that in italics if it helps.

image

Look, if I call that action as in that screenshot, and then look at the entity, the previous volume attribute is updated. If I call that action and then the next node immediately calls a TTS notify, the attribute does not get updated. If I put a 2 second delay in between, it works.

As I said, I didn't think it warranted an issue because I doubt it's a "bug" in your code. But there is clearly a timing issue somewhere in the interface between HA and node red and Amazon, which might cause other people to raise issues as more start to use this.

I just thought you would be interested. I wish I hadn't bothered now, I'm happy to carry on using the code I had which saves the volume itself and reverts it afterwards, and doesn't need any delays.

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.

4 participants