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

Refactor scanner #158

Merged
merged 2 commits into from
Sep 4, 2021
Merged

Refactor scanner #158

merged 2 commits into from
Sep 4, 2021

Conversation

KapJI
Copy link
Collaborator

@KapJI KapJI commented Sep 3, 2021

Breaking changes

  • Rename google_device to network_device.
  • Remove ip_address and port fields from Device constructor, they're only used in tests and add code complexity.

Scanner changes

  • Stop discovery after the timeout. Inspired by pychromecast. Otherwise it can modify list of discovered devices while it's being processed. Also it spams logs.
  • Store devices in dict to filter duplicates. Before update_service was inserting copies in devices list.
  • Handle remove remove_service.

Other changes

  • Replace GoogleDevice class with NetworkDevice named tuple. It should really just hold the data, all checks are moved outside.
  • Remove unnecessary type hints from Final, they can be inferred automatically.
  • Remove few tests which are no longer needed.
  • Update some comments.

Tested locally on my HA instance.

@KapJI KapJI added the refactoring Refactoring label Sep 3, 2021
@KapJI KapJI requested a review from leikoilja September 3, 2021 14:25
@KapJI
Copy link
Collaborator Author

KapJI commented Sep 3, 2021

Jobs failed because of pypa/setuptools-scm#608.

@KapJI KapJI added the minor Minor changes. Used for release bump label Sep 3, 2021
Copy link
Owner

@leikoilja leikoilja left a comment

Choose a reason for hiding this comment

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

Bud, amazing job! Thanks for a good refactoring. Make sense to move the ip and port related things into the scanner and extract it as a network_device. 🔥 💯

tests/test_client.py Show resolved Hide resolved
@KapJI KapJI merged commit 4d87cd4 into leikoilja:master Sep 4, 2021
@KapJI KapJI deleted the match-ids2 branch September 4, 2021 15:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
minor Minor changes. Used for release bump refactoring Refactoring
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants