-
Notifications
You must be signed in to change notification settings - Fork 58
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
Is Bidirectional working (client to server to client)? #4
Comments
Hello! This is something that I spend yesterday looking at it, you're absolutely right, it's currently not working, that is because the Client should not receive the data with a callback, like the server (because the program exists, and doesn't wait for anything and the Client and Server both use the same implementations for Streams and Connections). I've created a branch called refactor-api, to try to tackle the issue by separating the Client and Server stream/connection implementations. In a perfect scenario, I imagine the stream data being sent like so: StreamContext sc = client.CreateStream();
sc.Send(/*data*/);
// Block while waiting for data
byte[] data = sc.Receive(); Because usually the client "waits" the response from the Server. It's not a simple one, and I might be changing a lot of stuff in the process. I might as well refactor the way the server treats uni-directional and bi-directional streams. Thank you! |
More on topic: The QuicContext and the QuicStreamContext are a code smell. I'll delete them in a new branch, delete the refactor-api branch, and push the new one with a couple of major changes. |
Resolved with branch new-api. Still not merged, as I need to do a couple of more tests. You can switch and give it a try too. Don't forget to check out the new examples in the README inside the new branch. Thanks! P.S. I'm leaving the issue open, because the differentiation between uni/bi-directional streams, and the hardcoded StreamId = 0, is still something 'TODO'. |
Thanks a lot for the clarification. It makes a lot more sense now. I didn't have a look at your branch yet, but I'm wondering on the client side if it would be best to have 3 different "context" that implements each of the directional (uni-client/uni-server/bi)? For the QuicStreamContext, I think it should be encapsulated somehow in the QuicClient. It's not such a bad idea to have a QuicContext, but I would pass it as a parameter in the constructor of the QuicClient. I would remove all business logic as well (The send, for example, should be moved to QuicClient ). And since it's object-oriented, when you connect, you shouldn't need to return a "handle". I currently don't have a lot of time to put on dev projects, but I'll see if I can find some. I will probably have a look at some apis such as WebClient, WebRequest (for methods and events) to get some ideas and see it can be applied in this design. |
Hello! I've merged the branch to master, the implementation there is latest. I actually did remove the Contexts, but you're right, all I needed to do is get the business logic out of it and leave it with contextual properties. _connection.Send(/*data*/); This probably could be tackled with encapsulating the Streams and keeping them internal as well. For the QuicConnection "handle" you mentioned, thats true. That is too way C style API, and your proposition for encapsulating it ,stands valid. Thank you, much appreciated! |
Hello, I found out that if you call the "new QuicStream", it doesn't work. When you look at the CreateStream in Connection, it uses "_streams.Add(streamId, stream);". So maybe QuicStream shouildn't be available (internal). Also, I was sure that you could play with the streamID in order to send multiple message using the same connection. But from what I understand, every time you want to do a new "Send", you need to create a new connection. Am I right? Thanks Class QuicClient2
Class Program from client
|
Hello, and thank you! Impressive example, I like how separate tasks are being run for Send and Receive, this solves a couple of issues, I'll look into a way of integrating it (or possibly rewriting) with the current QuicClient implementation, thanks! One thing that made me wonder is : "But from what I understand, every time you want to do a new "Send", you need to create a new connection. Am I right?". Very good question, to which I don't have answer yet. The 17th draft, point 5.3 is still TBD. Possibly you're right, but then we're only using a single stream for a single connection, and as far as the current draft document goes, QUIC allows for multiple streams for a single connection, but then, when do we close it? Manually? This is still to be determined, and I'll try to look up for more information. I imagine a client can do something along the lines of: QuicConnection connection = _client.Connect(..);
connection.Send(/*file 1*/); // Opens Stream 1
connection.Send(/*json*/); // Opens Stream 2
connection.Send(/*file2*/); // Opens Stream 3
connection.Close(); I'll keep this issue posted with the progress. Once again, thank you. Appreciate it! :) |
I was thinking with the current implementation, it doesn't seem far from a "websocket" implementation in UDP. That's why I was asking. Thanks for your reply |
Hello again,
I'm not a 100% sure that I understand the meaning of "Bidirectional"
But in your console client sample, I've added
I saw that in your server code you send an "Echo!", and was expecting a way to get the data through the delegate, but for some reason, I'm not getting anything. I've traced in the code, and the bytes are sent successfully, but maybe at a lower level, the data is not processed in the client?
The text was updated successfully, but these errors were encountered: