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

Allow server transport selection, allow ws server to choose random bind port #270

Closed
wants to merge 2 commits into from

Conversation

roysjosh
Copy link
Contributor

I'm not sure about the background of the ws.py FIXME comment

@roysjosh
Copy link
Contributor Author

roysjosh commented Feb 6, 2022

Hello, just checking in. This and the other API issue (#271) will probably be a blocker for my pull request to Home Assistant so if there's anything you need from me here I'd be happy to help.

@roysjosh
Copy link
Contributor Author

Any feedback here? I can split this into two PRs if that makes sense.

@roysjosh
Copy link
Contributor Author

roysjosh commented Aug 9, 2022

Hello, I updated this a bit to keep the +3000 offset but only apply it if the port isn't zero. This allows dynamic server port selection while retaining the original behavior for actual port numbers. What do you think?

@bdraco
Copy link

bdraco commented Aug 15, 2022

@chrysn Sorry for the ping. We've been holding back releasing coap support in HKC a few months waiting for a solution so it would be great to get this merged and released.

Thanks!

@chrysn
Copy link
Owner

chrysn commented Aug 15, 2022

Thanks for the ping, and sorry I missed this.

The fixes are good conceptually; I'll add a note that this precise (transports=[]) API is likely to change (in favor of a good one where security properties, ports and transports are set up in one go), but it makes things better than they are now.

Adjusting a few style nits and merging ASAP.

Can you work from a (pinned, I hope) version of the master branch, or will you need a pypi release?

HKC ... not sure of the abbreviation, but if it doesn't overlap with what has been reported in #277 I'd appreciated a note there.

@bdraco
Copy link

bdraco commented Aug 15, 2022

Thanks for the ping, and sorry I missed this.

👍

The fixes are good conceptually; I'll add a note that this precise (transports=[]) API is likely to change (in favor of a good one where security properties, ports and transports are set up in one go), but it makes things better than they are now.

Adjusting a few style nits and merging ASAP.

Can you work from a (pinned, I hope) version of the master branch, or will you need a pypi release?

We need a PyPI release.

HKC ... not sure of the abbreviation, but if it doesn't overlap with what has been reported in #277 I'd appreciated a note there.

HKC is HomeKit Controller

https://github.com/Jc2k/aiohomekit

@bdraco
Copy link

bdraco commented Aug 15, 2022

@roysjosh Can you update #277 ?

@roysjosh
Copy link
Contributor Author

@roysjosh Can you update #277 ?

Added a bit about aiohomekit's usage for HAP

chrysn added a commit that referenced this pull request Aug 16, 2022
@roysjosh
Copy link
Contributor Author

Thank you!

@roysjosh roysjosh closed this Aug 16, 2022
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.

None yet

3 participants