Skip to content

Conversation

@albaintor
Copy link
Contributor

@albaintor albaintor commented Apr 8, 2025

Actually the volume set is only done on the AppleTV side. But when you set your AppleTV to cast audio to multiple devices (homepod or any Airplay speaker), the set volume won't affect the other devices.

I added a new endpoint to specify the endpoint id while setting volume, in order to be able to reproduce the global volume slider from the notification center (it lets change all volumes at the same time).

This is related to this ticket : #2672

…ume updates of output devices. Volume is stored in the OutputDevice structure that has been enriched. This event completes the set_device_volume method that lets set volume to a given output device id
@albaintor
Copy link
Contributor Author

EDIT : I also added a new event to be notified when the volume of output devices changes

@postlund
Copy link
Owner

postlund commented Apr 9, 2025

Thank you, I will review this later tonight. But you have to revert the changes related to protobuf.

@albaintor
Copy link
Contributor Author

Hi,

it's done and tests passed

Copy link
Owner

@postlund postlund left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good overall, but a few comments to make conform with how the API generally work in pyatv. I would also like to see some kind of test (unit test in facade for instance) since this is new functionality.

@albaintor albaintor requested a review from postlund April 11, 2025 08:58
@albaintor
Copy link
Contributor Author

Hi again, I have added a test for device volume update and tested ok.

There is one thing that I changed partially : you requested OutputDevice class in the interfaces.py to be immutable.
I didn't manage to do this, I just migrated the class to a dataclass but this does not address the request.

To be honest I don't know how to do this in an easy way : the volume property needs to be updated.

@albaintor
Copy link
Contributor Author

Hi @postlund did you have the time to look after the recent changes ? thank you

@albaintor
Copy link
Contributor Author

Hi @postlund any news here ? thank you

@postlund
Copy link
Owner

@albaintor Can you take a look at this because I believe you have changed line endings or something. All files more or less as marked as changed. I would also like you to remove the "tvOS 18.4" commit as that has been merged already.

@albaintor
Copy link
Contributor Author

Hi, I don't know what happened with CRLF... this is fixed. I also removed the tvOS 18.4 patch.
There is still the thing I didn't manage to handle, mentioned in this post #2673 (comment)

About using immutable data whereas I need to send updated data (updated volume)

@albaintor
Copy link
Contributor Author

Hi @postlund , did you have the time to look after my PR ? thank you

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.

2 participants