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

Continuous discovery of Sonos speakers #23484

Merged
merged 1 commit into from Apr 29, 2019

Conversation

amelchio
Copy link
Contributor

Description:

This adds a periodic discovery of Sonos so powered on speakers can be found without restarting Home Assistant.

The timestamp of the last succesful discovery is used to calculate the available property.

We no longer allow adding "invisible" players (such as stereo slaves) because we cannot control them.

Finally, it turns out that we do not need to update uids because it is not used after #23385.

Checklist:

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

@pvizeli pvizeli merged commit 0ecf152 into home-assistant:dev Apr 29, 2019
assert hass.data[media_player.DATA_SONOS].uids == {'RINCON_test'}

entity = hass.data[media_player.DATA_SONOS].entities[0]
assert entity.unique_id == 'RINCON_test'
Copy link
Member

Choose a reason for hiding this comment

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

We should avoid asserting things about entitites directly. Primarily use the state of the entity to assert when the state holds what we need to test. For this case, we can check the unique_id by getting the entity registry entry for the entity, and assert that.

The idea is by avoiding using the entity directly in our tests, the code under test can be refactored without needing to change tests. This is very useful and will decrease risk of bugs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you, I will keep that in mind.

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