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

Retain the original format of the UUID #441

Merged
merged 13 commits into from
Jun 17, 2023

Conversation

bdraco
Copy link
Contributor

@bdraco bdraco commented Jun 3, 2023

apple seems to keep changing the case on these so we will now keep the binary blob and send back the exact bytes that was used in pairing when the client list pairings.

This should fix the users having trouble after migrating to the new home arch

apple seems to keep changing the case on these so we will
now keep the binary blob
@codecov
Copy link

codecov bot commented Jun 3, 2023

Codecov Report

Merging #441 (c44c3e2) into dev (7675cc5) will decrease coverage by 0.10%.
The diff coverage is 88.88%.

@@            Coverage Diff             @@
##              dev     #441      +/-   ##
==========================================
- Coverage   91.68%   91.59%   -0.10%     
==========================================
  Files          20       20              
  Lines        2152     2176      +24     
  Branches      270      273       +3     
==========================================
+ Hits         1973     1993      +20     
- Misses        136      139       +3     
- Partials       43       44       +1     
Impacted Files Coverage Δ
pyhap/hap_handler.py 80.92% <85.71%> (-0.31%) ⬇️
pyhap/accessory_driver.py 91.66% <100.00%> (ø)
pyhap/encoder.py 100.00% <100.00%> (ø)
pyhap/state.py 100.00% <100.00%> (ø)

@Ghawken
Copy link

Ghawken commented Jun 4, 2023

Hi
Thanks for you continued support of these libraries.

With your update here - don't we need hap_handler.py pair_five also updated to:

        should_confirm = self.accessory_handler.pair(
            client_username_bytes, client_ltpk, HAP_PERMISSIONS.ADMIN
        )

https://github.com/bdraco/ha-HAP-python/blob/ce6858cb36b0fafb3e19135e611d7bbf876d1370/pyhap/hap_handler.py#L439
as well?

Geting an exception as pair_five is sending uuid rather than bytes to accessory_handler.pair?

Above seems to fix.

@bdraco
Copy link
Contributor Author

bdraco commented Jun 4, 2023

There is still more. Ran out of time before takeoff. Will be undrafted when finished and tested

@bdraco
Copy link
Contributor Author

bdraco commented Jun 4, 2023

I think I got everything. At least mypy checks out. Traveling though so can't test locally until I get back home

@Ghawken
Copy link

Ghawken commented Jun 4, 2023

Thanks. Happy travels. Testing here no issues thus far…

@bdraco
Copy link
Contributor Author

bdraco commented Jun 5, 2023

all seems ok, but more testing needed and I made some zeroconf changes today so I need to wait to finish testing that for 48 hours before I can validate this to make sure I know which change has which side effects.

speaking of that, this lib needs to accept multiple addresses to publish via mdns since ipv6/ipv4 dual stacks won't work otherwise which would probably make homekit work for a lot more people

@Ghawken
Copy link

Ghawken commented Jun 5, 2023

Good luck.
One thing I have recently noticed is pyHAP uses 8.8.8.8 gmail/DNS to decide local ivp4 interface. If 8.8.8.8 blocked (which many might do to prevent other DNS servers) probably would also lead to issues?

s.connect(("8.8.8.8", 80))

Interestingly still seems to be preferred option - although can use:
s = socket.socket(socket.AF_INET, socket.SOCK_DGRAM) try: s.setsockopt(socket.SOL_SOCKET, socket.SO_BROADCAST, 1) s.connect(('10.255.255.255', 80)) addr = s.getsockname()[0]

Which I am testing currently….

@ikalchev
Copy link
Owner

ikalchev commented Jun 6, 2023

Hey - I will go through the PRs and merge on Sunday probably (not around a computer till then)

@bdraco
Copy link
Contributor Author

bdraco commented Jun 6, 2023

Great. I will try to get the mdns / IPv6 stuff done before than.

I’m still testing this one

@bdraco
Copy link
Contributor Author

bdraco commented Jun 6, 2023

testing looks good

@bdraco bdraco marked this pull request as ready for review June 6, 2023 15:39
@bdraco
Copy link
Contributor Author

bdraco commented Jun 12, 2023

I've been running all 4 of the open PRs on production for a few days. Everything seems nice and snappy

@bdraco
Copy link
Contributor Author

bdraco commented Jun 16, 2023

@ikalchev gentle reminder

@ikalchev
Copy link
Owner

@bdraco there seem to be some merge conflicts. Do you mind having a look.

Will merge and release tomorrow if you can address these - that's a promise

@bdraco
Copy link
Contributor Author

bdraco commented Jun 17, 2023

I'll fix them in a moment

@bdraco
Copy link
Contributor Author

bdraco commented Jun 17, 2023

@ikalchev retested on production after conflicts fixed. all good 👍

thanks

@jimmy927
Copy link

Has this made it into main yet ?
Has anyone tested if this fixes home-assistant/core#110540 ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants