-
Notifications
You must be signed in to change notification settings - Fork 30
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
Streaming #68
base: master
Are you sure you want to change the base?
Streaming #68
Conversation
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.
Thank you for making a PR, please take a look at the comments. I hope we can land this feature 😃
android/src/main/java/com/pkmnapps/nearby_connections/NearbyConnectionsPlugin.java
Outdated
Show resolved
Hide resolved
showSnackbar("Sending $a to ${value.endpointName}, id: $key"); | ||
Nearby() | ||
.sendStreamPayload(key, Uint8List.fromList(a.codeUnits)); | ||
}); |
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.
This is not really a usecase for streams, nor it depicts how to send an endless stream of data.
the linked example uses audio, but if we want to avoid that, we can instead ask the user to hold the button, and send random bytes of payload while the button is on hold. Basically an example of sending a stream of data without any known size
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.
@mannprerak2 Bro, have you tested the streams in Java code yourself? Because, the streaming only works for the first time we use Nearby.getConnectionsClient(activity).sendPayload(endpointId, Payload.fromStream(inputStream)). If I do the same for the second time, the streaming doesn't work. Here is the code which I've written for the logic. https://github.com/lakshmanprabhu49/nearby_connections/tree/Streaming_v2
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.
You can push commits to this PR itself.
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.
@mannprerak2 I've pushed the code here. Can you review? As far as I tested in my local, the stream was sent from sender to receiver. But not sure if this will work in all scenarios
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.
I've added more comments
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.
Could you also change the example to send maybe a stream of numbers at a gap of 1 second. E.g -
Stream<int>.periodic(const Duration(seconds: 1), (x) => x).take(5)
.listen((i) {
Nearby().sendStreamPayload(key, Uint8List.fromList(i.toString().codeUnits));
});
Also, the receiver side should show a short snackbar (of duration 0.9 seconds) for every received stream
# Conflicts: # android/src/main/java/com/pkmnapps/nearby_connections/NearbyConnectionsPlugin.java # lib/src/nearby.dart
byte[] bytes; | ||
while (true) { | ||
try { | ||
if (inputReceiverStream.available() <= 0) break; |
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.
Its possible the while loop ends up being faster than the stream speed and we would end up no longer listening to the receiver stream.
I think we should follow the android docs here : https://developers.google.com/nearby/connections/android/exchange-data#stream:~:text=SystemClock.elapsedRealtime()%20%2D%20lastRead)%20%3E%3D%20READ_STREAM_IN_BG_TIMEOUT
Basically, a configured timeout value (READ_STREAM_IN_BG_TIMEOUT) after which the stream could be considered dead.
We should also let the user change the timeout (maybe a method channel call to update the stream timeout)
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.
Also on timeout we should call events.endOfStream()
eventSink.success(bytes); | ||
} | ||
} catch (IOException e) { | ||
e.printStackTrace(); |
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.
Should be replaced with eventSink.error("Stream Error", "IO Exception", null)
and a Log.e(...)
senderPayloadPipe[0].close(); | ||
senderPayloadPipe[1].close(); | ||
} catch (IOException ex) { | ||
ex.printStackTrace(); |
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.
Replace all e.printStackTrace
calls with Log.e("nearby_connections", "IO Exception at <place>", e)
No description provided.