Async UPnP and Echo service #158
Conversation
lionel-faber
force-pushed
the
upnp-async
branch
from
fb0b50b
to
232d86b
Compare
There was a problem hiding this comment.
I think we should consider if it's possible to remove the feature upnp as it make the code more unreadable and perhaps it's over killing, perhaps we just need a function which is specific for using upnp for discovering the public address, and the consumer of the api decides to call it if that's what it needs.
| } | ||
| } | ||
|
|
||
| // Helper to read the message's bytes from the provided stream | ||
| async fn read_bytes(recv: &mut quinn::RecvStream) -> Result<WireMsg> { |
There was a problem hiding this comment.
I'm not sure we really should do this, we are adding a function which has one line of code, so we are not re-using code really, and it's having more lines of code but no gain, calling WireMsg::read_from_stream(recv).await should be good enough from wherever it's needed, and it should be self-explanatory already, if not, then we can rename read_from_stream to make it self-explanatory.
src/connections.rs
Outdated
| Ok(WireMsg::EndpointEchoReq) => { | ||
| let message = WireMsg::EndpointEchoResp(peer_addr); | ||
| message.write_to_stream(&mut send).await.ok()?; | ||
| None |
There was a problem hiding this comment.
I think we shouldn't return None in this case as that would mean for the user that there won't be more messages arriving in the stream, and therefore the user will end the loop. We should send the echo response, but then go back and read the next message by continuing the loop, so if next message is for the user it will receive it as thee next message without noticing there was an echo response sent in beforehand.
src/connections.rs
Outdated
| } | ||
| Ok(msg) => { | ||
| error!("Unexpected message type: {:?}", msg); | ||
| None |
There was a problem hiding this comment.
Same as above comment, we should log the error but we shouldn't return None, instead read next message by continuing the loop.
from stream error occours so that the client can continue the loop for other messages
lionel-faber
force-pushed
the
upnp-async
branch
from
9ce4b34
to
d679447
Compare
lionel-faber
force-pushed
the
upnp-async
branch
2 times, most recently
from
975b6ad
to
620f63f
Compare
lionel-faber
force-pushed
the
upnp-async
branch
from
620f63f
to
bdd8967
Compare
|
These changes are incorporated in #162 |
No description provided.