-
Notifications
You must be signed in to change notification settings - Fork 22
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
Reconnect camera streams #96
Conversation
_channel = channel; | ||
_channel.rtcPeerConnection.onTrack = (event) { | ||
_errorHandler?.cancel(); // Cancel the error handler -- clearly we're connected if we're receiving this event |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(q) why are we doing this twice?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The first time happens whenever we change the channel.
This one is on a callback -- so it's possible it gets called out of order/after the error handler exists again.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The first one happens whenever the channel changes. This one is happening on a callback which could be called out of order/delayed (after the _errorHandler
exists again). The one that happens a bit further down is in another callback, where we want to cancel if the returned state is not an error state.
@@ -75,7 +80,10 @@ class RobotClient { | |||
/// Refresh the resources of this robot | |||
Future<void> refresh() async { | |||
final ResourceNamesResponse response = await _client.resourceNames(ResourceNamesRequest()); | |||
if (response.resources == resourceNames) { | |||
if (setEquals(response.resources.toSet(), resourceNames.toSet())) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(q) why if(setEquals(response.resources.toSet(),.......
as opposed to if (response.resources == resourceNames)
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mostly because the second way wasn't working. I didn't dive too deep into it but converting both to lists, sorting those lists, making everything lowercase strings, nothing actually make things equal except converting these to sets and using setEquals
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
left questions for my own understanding but LGTM
Camera streams now automatically reestablish after a loss and regain of connection.