Skip to content

Conversation

@hiroshihorie
Copy link
Member

In addition to publisher ice-connect,
this PR makes it wait until data channel is open state.

@hiroshihorie hiroshihorie changed the title Wait for data channel ready Wait for data channel to open Jan 4, 2022
@hiroshihorie hiroshihorie requested a review from davidzhao January 4, 2022 11:29
if (publisher?.pc.iceConnectionState?.isConnected() == true) {
logger.warning('[$objectId] publisher is already connected');
return;
// wait for data channel to open (if not already)
Copy link
Member Author

Choose a reason for hiding this comment

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

This is where it waits for open state of the data channel.

Copy link

Choose a reason for hiding this comment

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

Is there some callback to indicate data channel open failure if it fails for some reason?

Copy link
Member Author

Choose a reason for hiding this comment

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

waitFor will throw a TimeoutException if it timeouts.

@hiroshihorie hiroshihorie requested a review from boks1971 January 4, 2022 11:31
throw UnexpectedStateException('Data channel is not ready');
}
if (publisher?.pc.iceConnectionState?.isConnected() != true) {
logger.fine('Publisher is not connected...');
Copy link

Choose a reason for hiding this comment

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

What happens if there are a bunch of data channel messages sent by the app in this state? Are all of them becoming promises and get sent (promises resolved) when the data channel connects? Or are the messages just dropped on the floor?

Copy link
Member Author

Choose a reason for hiding this comment

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

promises should be pending until waitFor condition clears (received notification) or timeouts.
actually maybe there should be more checks to prevent negotiate() being called multiple times.. 🤔
(not sure how immediate the state changes to RTCIceConnectionStateChecking)

Copy link
Member

@davidzhao davidzhao left a comment

Choose a reason for hiding this comment

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

looks good!

@hiroshihorie hiroshihorie merged commit fed657a into main Jan 7, 2022
@hiroshihorie hiroshihorie deleted the dc-wait-ready branch January 7, 2022 07:34
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