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

Use bonjour advertiser as default again, warn when Avahi is used on an unsupported platform. #927

Merged
merged 1 commit into from
Jan 19, 2022

Conversation

Supereg
Copy link
Member

@Supereg Supereg commented Jan 19, 2022

♻️ Current situation

For the beta period of 0.10.0 we changed how we select the default advertiser, moving from bonjour to avahi as a default if we our heuristic detects it being available on the platform.
This heuristic might not be perfect and might result in non advertised HAP service (see homebridge/homebridge#3062 (comment)).

💡 Proposed solution

To avoid any breaking changes for existing users, this PR revers the default advertiser back to the bonjour package.

⚙️ Release Notes

  • Revert to bonjour as the default advertiser.

➕ Additional Information

When selecting the avahi advertiser, this PR will now additionally run the availability check, and revert to bonjour if we find to be running on a non compatible platform. Additionally, we print an error message informing the user about it.

Testing

--

Reviewer Nudging

--

@coveralls
Copy link

Pull Request Test Coverage Report for Build 1719796451

  • 3 of 5 (60.0%) changed or added relevant lines in 1 file are covered.
  • 18 unchanged lines in 1 file lost coverage.
  • Overall coverage decreased (-0.2%) to 51.923%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/lib/Accessory.ts 3 5 60.0%
Files with Coverage Reduction New Missed Lines %
src/lib/Advertiser.ts 18 53.48%
Totals Coverage Status
Change from base Build 1704852637: -0.2%
Covered Lines: 5704
Relevant Lines: 10040

💛 - Coveralls

@Supereg Supereg merged commit de378a5 into beta-0.10.0 Jan 19, 2022
@Supereg Supereg deleted the feature/bonjour-default branch January 19, 2022 19:35
@codyc1515
Copy link
Contributor

What is the reasoning for non using ciao?

@NorthernMan54
Copy link
Contributor

Pls read the commentary from @adriancable in regards to advertiser issues, #918

@Supereg
Copy link
Member Author

Supereg commented Jan 21, 2022

What is the reasoning for non using ciao?

You may also refer to the Notice on native mDNS responders in the ciao repo for some context.

koush pushed a commit to koush/HAP-NodeJS that referenced this pull request Jun 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants