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

Add support for happy eyeballs (RFC 8305) #349

Merged
merged 18 commits into from
Dec 12, 2023
Merged

Add support for happy eyeballs (RFC 8305) #349

merged 18 commits into from
Dec 12, 2023

Conversation

bdraco
Copy link
Collaborator

@bdraco bdraco commented Dec 10, 2023

Adds Happy Eyeballs support (https://datatracker.ietf.org/doc/html/rfc8305)

--

This PR is a first step as I need to make sure this library works for the other projects as well. Its likely to sit for a bit while I test the library with all the projects that need it so there aren't any need to do breaking changes before all the other projects start consuming it:

--

This should fix all the cases where

  • We have dual stack ipv4/ipv6
  • There are multiple IPs to connect to and we don't know which one is the correct one

closes #348

likely fixes home-assistant/core#102901

Copy link

codecov bot commented Dec 10, 2023

Codecov Report

Attention: 11 lines in your changes are missing coverage. Please review.

Comparison is base (48ad2b9) 74.42% compared to head (c674500) 74.50%.

Files Patch % Lines
aiohomekit/controller/ip/connection.py 77.27% 10 Missing ⚠️
aiohomekit/controller/ip/pairing.py 75.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #349      +/-   ##
==========================================
+ Coverage   74.42%   74.50%   +0.08%     
==========================================
  Files          99       99              
  Lines        9273     9301      +28     
==========================================
+ Hits         6901     6930      +29     
+ Misses       2372     2371       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@bdraco
Copy link
Collaborator Author

bdraco commented Dec 10, 2023

I'm seeing duplicate IPv6 addresses

I think its a bug in zeroconf.

Going to have to fix that first

@bdraco
Copy link
Collaborator Author

bdraco commented Dec 10, 2023

bug in zeroconf fixed in 0.128.2+

Will bump the min version here and the dep in HA

@bdraco
Copy link
Collaborator Author

bdraco commented Dec 10, 2023

tests fail with new zeroconf because _set_properties is no longer exposed.

Will re-expose it for now python-zeroconf/python-zeroconf#1327

.github/workflows/ci.yml Outdated Show resolved Hide resolved
Copy link
Owner

@Jc2k Jc2k 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 looking good. Do you think it would be safe to allow ipv6 link local whilst working on this? It's much more likely to be a valid addresss than an ipv4 link local.

aiohomekit/controller/ip/connection.py Outdated Show resolved Hide resolved
aiohomekit/controller/ip/connection.py Outdated Show resolved Hide resolved
@bdraco
Copy link
Collaborator Author

bdraco commented Dec 11, 2023

This is looking good. Do you think it would be safe to allow ipv6 link local whilst working on this? It's much more likely to be a valid addresss than an ipv4 link local.

I fixed up zeroconf to pass the scope_id correctly so we could make ipv6 link local work with some more work but it would need a lot more refactoring to be able to split the %<scope_id> off of the addresses so I kept them excluded for now. Something for the future.

@bdraco
Copy link
Collaborator Author

bdraco commented Dec 11, 2023

Thanks.

I've just finished the code for all 3 of these PRs.

I'm doing testing aiohomekit and aioesphomeapi but I don't want to merge until I'm sure there won't be any breaking changes in the lib

@bdraco bdraco closed this Dec 12, 2023
@bdraco bdraco reopened this Dec 12, 2023
@bdraco bdraco marked this pull request as ready for review December 12, 2023 16:54
@bdraco bdraco changed the title Add support for happy eyeballs Add support for happy eyeballs (RFC 8305) Dec 12, 2023
@bdraco
Copy link
Collaborator Author

bdraco commented Dec 12, 2023

Thanks.

Happy with the other libs, will push a release shortly

@bdraco bdraco merged commit 1409cd6 into main Dec 12, 2023
12 of 16 checks passed
@bdraco bdraco deleted the happy_eyeballs branch December 12, 2023 17:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants