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

Multiroom support for snapcast #24061

Merged
merged 5 commits into from Jul 19, 2019

Conversation

@lyghtnox
Copy link
Contributor

commented May 23, 2019

Description:

This changes aim to add a multiroom support for the snapcast component in the same way than the sonos component. It adds 2 services media_player.snapcast_join and media_player.snapcast_unjoin to group/ungroup a client with another client. I also added source selection support in the snapcast clients as the groups are unusable due to their dynamic nature.

Checklist:

  • The code change is tested and works locally.
  • Local tests pass with tox. Your PR cannot be merged unless tests pass
  • There is no commented out code in this PR.
  • I have followed the development checklist

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

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

  • The manifest file has all fields filled out correctly. Update and include derived files by running python3 -m script.hassfest.
  • New or updated dependencies have been added to requirements_all.txt by running python3 -m script.gen_requirements_all.
  • Untested files have been added to .coveragerc.
@@ -185,6 +185,23 @@ snapcast_restore:
description: Name(s) of entities that will be restored. Platform dependent.
example: 'media_player.living_room'

snapcast_join:

This comment has been minimized.

Copy link
@balloob

balloob May 23, 2019

Member

Move these services under the snapcast domain: snapcast.join

})

JOIN_SERVICE_SCHEMA = SERVICE_SCHEMA.extend({
vol.Required(ATTR_MASTER): cv.entity_id,

This comment has been minimized.

Copy link
@balloob

balloob May 23, 2019

Member

Why not use ATTR_ENTITY_ID to make it clear that it's just another entity, and no special value?

This comment has been minimized.

Copy link
@lyghtnox

lyghtnox May 24, 2019

Author Contributor

I used ATTR_MASTER to distinguish the clients moving to the master's group and the master not moving from his group.
So after the join, all clients are playing the master's stream

@lyghtnox

This comment has been minimized.

Copy link
Contributor Author

commented May 25, 2019

Oh, this now has breaking changes:

  • media_player.snapcast_snapshot becomes snapcast.snapshot
  • media_player.snapcast_restore becomes snapcast.restore
"""Dispatch a service call."""
hass.data[DATA_SERVICE_EVENT].clear()
async_dispatcher_send(hass, DOMAIN, service.service, service.data)
await hass.data[DATA_SERVICE_EVENT].wait()

This comment has been minimized.

Copy link
@balloob

balloob May 26, 2019

Member

Why not pass the event via the dispatcher? that way it will be possible to run multiple services at the same time.

@balloob
Copy link
Member

left a comment

Ok to merge when final comment addressed 👍 🐬

@lyghtnox

This comment has been minimized.

Copy link
Contributor Author

commented Jun 13, 2019

Hi,
I didn't really understand your last comment @balloob
Do I need to do something more for this to be merged?

@balloob

This comment has been minimized.

Copy link
Member

commented Jun 14, 2019

Your current approach with events does not allow 2 services to be called at the same time. You should not store the event in hass.data but pass it as an argument to the dispatcher signal listener.

@MartinHjelmare

This comment has been minimized.

Copy link
Member

commented Jul 9, 2019

@lyghtnox are you planning to continue here? Do you need further input?

@lyghtnox

This comment has been minimized.

Copy link
Contributor Author

commented Jul 10, 2019

I got a lot of things to do at the moment but I will work on the changes when I have some free time.

lyghtnox added some commits Jul 17, 2019

@balloob balloob merged commit caa7a3a into home-assistant:dev Jul 19, 2019

9 checks passed

CI Build #20190717.52 succeeded
Details
CI (FullCheck Mypy) FullCheck Mypy succeeded
Details
CI (FullCheck Pylint) FullCheck Pylint succeeded
Details
CI (Overview Lint) Overview Lint succeeded
Details
CI (Overview Validate) Overview Validate succeeded
Details
CI (Tests PyTest Python35) Tests PyTest Python35 succeeded
Details
CI (Tests PyTest Python36) Tests PyTest Python36 succeeded
Details
CI (Tests PyTest Python37) Tests PyTest Python37 succeeded
Details
cla-bot Everyone involved has signed the CLA

KJonline added a commit to Rendili/home-assistant that referenced this pull request Jul 19, 2019

Merge branch 'dev' of https://github.com/home-assistant/home-assistant
…into hive-splitfiles-testing

* 'dev' of https://github.com/home-assistant/home-assistant: (109 commits)
  Multiroom support for snapcast (home-assistant#24061)
  Turn on device before setting mode (home-assistant#25314)
  Turn on device before setting mode (home-assistant#25314)
  Bumped version to 0.96.2
  Fix plant error when adding new value (home-assistant#25302)
  Fix HM with use wrong datapoint for off (home-assistant#25298)
  [climate] Correct honeywell supported_features (home-assistant#25292)
  Fixed python-wink method names (home-assistant#25285)
  Fix fritzbox climate HVAC mode / temperature (home-assistant#25275)
  Updated frontend to 20190719.0
  Updated frontend to 20190719.0
  Fix plant error when adding new value (home-assistant#25302)
  Fix HM with use wrong datapoint for off (home-assistant#25298)
  [climate] Correct honeywell supported_features (home-assistant#25292)
  Fix fritzbox climate HVAC mode / temperature (home-assistant#25275)
  Fixed python-wink method names (home-assistant#25285)
  Add services to set and remove Simplisafe PINs (home-assistant#25207)
  Add MQTT climate precision (home-assistant#25265)
  Add support for Rainforest Eagle-200 (home-assistant#24919)
  Update azure-pipelines-ci.yml for Azure Pipelines
  ...

# Conflicts:
#	homeassistant/components/hive/climate.py

KJonline added a commit to Rendili/home-assistant that referenced this pull request Jul 19, 2019

Merge branch 'dev' of https://github.com/home-assistant/home-assistant
…into dev

* 'dev' of https://github.com/home-assistant/home-assistant: (112 commits)
  Avoid creating temporary lists (home-assistant#25317)
  Multiroom support for snapcast (home-assistant#24061)
  Turn on device before setting mode (home-assistant#25314)
  Turn on device before setting mode (home-assistant#25314)
  Bumped version to 0.96.2
  Fix plant error when adding new value (home-assistant#25302)
  Fix HM with use wrong datapoint for off (home-assistant#25298)
  [climate] Correct honeywell supported_features (home-assistant#25292)
  Fixed python-wink method names (home-assistant#25285)
  Fix fritzbox climate HVAC mode / temperature (home-assistant#25275)
  Updated frontend to 20190719.0
  Updated frontend to 20190719.0
  Fix plant error when adding new value (home-assistant#25302)
  Fix HM with use wrong datapoint for off (home-assistant#25298)
  [climate] Correct honeywell supported_features (home-assistant#25292)
  Fix fritzbox climate HVAC mode / temperature (home-assistant#25275)
  Fixed python-wink method names (home-assistant#25285)
  Add services to set and remove Simplisafe PINs (home-assistant#25207)
  Add MQTT climate precision (home-assistant#25265)
  Add support for Rainforest Eagle-200 (home-assistant#24919)
  ...

@lyghtnox lyghtnox deleted the lyghtnox:snapcast_multiroom_integration branch Jul 19, 2019

@balloob balloob referenced this pull request Aug 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.