Skip to content

Add support for happy eyeballs (RFC 8305)#349

Merged
bdraco merged 18 commits intomainfrom
happy_eyeballs
Dec 12, 2023
Merged

Add support for happy eyeballs (RFC 8305)#349
bdraco merged 18 commits intomainfrom
happy_eyeballs

Conversation

@bdraco
Copy link
Copy Markdown
Collaborator

@bdraco bdraco commented Dec 10, 2023 •
edited
Loading

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

bdraco added a commit to home-assistant/core that referenced this pull request Dec 10, 2023
@bdraco
Add support for Happy Eyeballs to homekit_controller
9a5f6d5 
needs Jc2k/aiohomekit#349
@bdraco bdraco mentioned this pull request Dec 10, 2023
Merged
20 tasks
@codecov
Copy link
Copy Markdown

codecov bot commented Dec 10, 2023 •
edited
Loading

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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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

bdraco
bdraco commented Dec 10, 2023
View reviewed changes
This was referenced Dec 10, 2023
Merged
Merged
Jc2k
Jc2k reviewed Dec 11, 2023
View reviewed changes
Copy link
Copy Markdown
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.

@bdraco
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
@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

Reviewers

@Jc2k Jc2k Jc2k approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Implement happy eye balls with aiohappyeyeballs Controme Miniserver - loses connection - homekit_controller integration not ready yet

2 participants

@bdraco @Jc2k