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

Added support for CasaTunes Multiroom Audio #22536

Closed
wants to merge 6 commits into from
Closed

Added support for CasaTunes Multiroom Audio #22536

wants to merge 6 commits into from

Conversation

resoai
Copy link

@resoai resoai commented Mar 29, 2019

Description:

This component adds support for CasaTunes multiroom audio servers. Zones are added as media players which can select source, volume and be muted. Sources are added as media players that feed audio to the zones and can support play/pause/stop/backward/forward commands as well as album cover and track position.

Example entry for configuration.yaml:

media_player:
  - platform: casatunes
    name: CasaTunes
    host: 192.168.1.31

Checklist:

  • The code change is tested and works locally.
  • Local tests pass with tox.
  • There is no commented out code in this PR.

If user exposed functionality or configuration variables are added/changed:

  • Documentation added/updated (will be added in due course)

If the code communicates with devices, web services, or third-party tools:

  • New dependencies have been added to the REQUIREMENTS.
  • New dependencies are only imported inside functions that use them.

Copy link
Member

@andrewsayre andrewsayre left a comment

Choose a reason for hiding this comment

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

This is off to a great start!

  1. Add a __init__.py in the folder with just single line doc string.
  2. Run script/gen_requirements_all.py to update the requirements files (this will pickup the lib after you add init).
  3. Update .coveragerc since there are no tests.

General question: What is the relationship between player and zone? Is it 1:1? They both seem to be half of a player. I'm wondering if a zone and player should be represented as a single entity...

homeassistant/components/casatunes/media_player.py Outdated Show resolved Hide resolved
homeassistant/components/casatunes/media_player.py Outdated Show resolved Hide resolved
homeassistant/components/casatunes/media_player.py Outdated Show resolved Hide resolved
homeassistant/components/casatunes/media_player.py Outdated Show resolved Hide resolved
homeassistant/components/casatunes/media_player.py Outdated Show resolved Hide resolved
homeassistant/components/casatunes/media_player.py Outdated Show resolved Hide resolved

async def async_media_seek(self, position):
"""Send seek command."""
await self._player.seek(position)
Copy link
Member

Choose a reason for hiding this comment

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

Does this trigger an update so media position reflects the correct placement?

Copy link
Author

@resoai resoai Apr 1, 2019

Choose a reason for hiding this comment

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

Unfortunately no, but I think I found a workaround. Still need to do a bit of testing.

Copy link
Member

Choose a reason for hiding this comment

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

Given this integration uses polling, you could:

  1. Update state here if the new data would be available in fresh poll: self.async_schedule_update_ha_state(True). True indicates to call async_update before updating the state machine.
  2. Operate optimistically by setting the new position and last update and then call self.async_schedule_update_ha_state()

You may need something similar in the other commands if you're finding the state isn't updating until the next poll.

Copy link
Author

Choose a reason for hiding this comment

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

Thanks for the hint! I think, I'll have to spend a bit more time with it as the implementation of CasaTunes "seeking" is not very reliable.

homeassistant/components/casatunes/media_player.py Outdated Show resolved Hide resolved
@andrewsayre andrewsayre self-assigned this Mar 29, 2019
@resoai
Copy link
Author

resoai commented Mar 30, 2019

This is off to a great start!

  1. Add a __init__.py in the folder with just single line doc string.
  2. Run script/gen_requirements_all.py to update the requirements files (this will pickup the lib after you add init).
  3. Update .coveragerc since there are no tests.

General question: What is the relationship between player and zone? Is it 1:1? They both seem to be half of a player. I'm wondering if a zone and player should be represented as a single entity...

Thanks! I'll try to fix it.

No, Zones are basically "Rooms" and players are sources which can be selected to play in those zones. They are both media players with different capabilities: Zones can select the source (player), adjust volume and be muted. Players can pause/play/back/forward and display album art as well as track position (if this is not the radio). I hope that the attached screenshot would make a bit more sense.

Screenshot 2019-03-30 at 00 03 46

homeassistant/components/casatunes/media_player.py Outdated Show resolved Hide resolved
homeassistant/components/casatunes/media_player.py Outdated Show resolved Hide resolved
homeassistant/components/casatunes/media_player.py Outdated Show resolved Hide resolved
homeassistant/components/casatunes/media_player.py Outdated Show resolved Hide resolved
homeassistant/components/casatunes/media_player.py Outdated Show resolved Hide resolved
homeassistant/components/casatunes/media_player.py Outdated Show resolved Hide resolved
@andrewsayre
Copy link
Member

It looks like the build status never reported for this. Can you please rebase and force push? There has also been some architectural changes recently and you will need to update to use the manifest file.

@andrewsayre
Copy link
Member

Are you still working on this? If not, can we close the PR?

@andrewsayre
Copy link
Member

I'm going to close this PR. Feel free to open a new one and tag me if you want to continue with this contribution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants