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

homekit: fix late 2way setup #1461

Merged
merged 2 commits into from
May 6, 2024
Merged

homekit: fix late 2way setup #1461

merged 2 commits into from
May 6, 2024

Conversation

xdissent
Copy link
Contributor

@xdissent xdissent commented May 5, 2024

When my ring doorbell sets up a streaming session, the first message on the audio return channel isn't a request to setup 2way. In that case, the "once" handler would fire (and bail) and when the real 2way setup message would arrive, the setup listener was no longer registered and nothing would happen. This PR moves the 2way setup into the always-on listener, and creates a guard to prevent multiple setup requests from running concurrently. It should also allow retrying the setup if something temporarily went wrong on a previous attempt.


if (rtp.header.payloadType !== session.startRequest.audio.pt)
return;
let initializing = false;
Copy link
Owner

@koush koush May 6, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This code seems correct, but it was a bit confusing to read due to the addition of a second state variable. Can you make a change with something like:

let twoWayAudioState = 'stopped' | 'starting' | 'started' = 'stopped';

This would replace both initializing and playing. Then use this variable throughout to manage check change/check.

Copy link
Owner

@koush koush May 6, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Related: this would seem to allow restarting of the two way audio if it terminates for some reason. I may consider terminating two way audio automatically if homekit stops sending packets (which it does when the mic is disabled on the ios home app). Many cameras operate with echo cancellation during two way audio, which mutes incoming audio to some degree.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. Much easier to read, I agree. And yes, it does allow restarting of the two way audio!

@koush koush merged commit 5bca9b7 into koush:main May 6, 2024
@xdissent xdissent deleted the homekit-2way-init branch May 11, 2024 20:14
This pull request was closed.
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.

2 participants