Update UListener interface and UUID builder #24
Conversation
Signed-off-by: ChenYing Kuo <[email protected]>
|
@AnotherDaniel and @PLeVasseur I'm still fixing the tests in the PR, but you can take a look if you need this. |
Signed-off-by: ChenYing Kuo <[email protected]>
|
@AnotherDaniel and @PLeVasseur You can review the PR now. Since Daniel wants this urgently, I'll deal with some minor issues in another PR. |
| } | ||
| }; | ||
| resp_callback(msg); | ||
| } | ||
| }; | ||
|
|
||
| // Send query |
There was a problem hiding this comment.
I'm not great at Zenoh :)
I'm trying to understand if there's any issue with us using the zenoh_key here alone. Is it possible that if we fired off two requests, one after the other that we'd end up getting them back in the wrong order? Should we make use of the req_id at all?
Sorry for newb questions 🙃
There was a problem hiding this comment.
Zenoh will take care of the order of queries and not mix them. That's why we don't need to use req_id while sending the query.
There was a problem hiding this comment.
Could you explain a little detail behind how the queries are ordered correctly?
If we sent out two Request messages one immediately after the other, call them A and B. And a Response messages was sent back for B first, how does the ordering get enforced?
If there's some bit of documentation or better yet code / tests that show this I think that'd be great.
There was a problem hiding this comment.
Each time we create a Zenoh query to send out, there is also a qid inside Zenoh, which is used to differentiate different messages. While receiving the response message, Zenoh will use the qid to find the corresponding buffer to put the message.
In your case, we called invoke_method(A) and invoke_method(B). invoke_method(A) should only get the response message from its buffer with the correct qid. Response message to B will never go inside the buffer of A.
Since this is the mechanism inside Zenoh, I prefer to hide it instead of exposing to users.
There was a problem hiding this comment.
Ahhh okay. So even though the zenoh_key may be the same, there's the qid in the implementation of Zenoh that will correctly tie together each request with the corresponding response. Makes sense.
Maybe not on this PR, but I think it'd be nice to show executing a couple of invoke_method() one after the other and showing they come back in the correct order. Ensuring in the test that we invoke_method() on another task so we can immediately call it again and show that even though we fired request_A, _request_B and were returned response_B, response_A they were correlated correctly.
There was a problem hiding this comment.
Then we should make sure the callback won't block while accepting the first request.
Maybe related to the issue
There was a problem hiding this comment.
@evshary there will be an issue if you do not pass reqid in the zenoh message we will have issues correlating a message across transports. Is the reqid passed in the UMessage returned from the transport??
There was a problem hiding this comment.
reqid is still passed in UAttributes (in UMessage). It's just not used in Zenoh mechansim.
There was a problem hiding this comment.
@stevenhartley -- as we chatted about offline, I think the handling is done correctly in the Request / Response flow, even in usage by the UStreamer, based on my current understanding.
@evshary -- can you confirm if this diagram accurately represents usage of up-client-zenoh-rust?
sequenceDiagram
participant ue as uE
participant ue_z as up-client-zenoh-rust
participant st_z as up-client-zenoh-rust
participant st as up-streamer-rust
participant st_s as up-client-someip-rust
participant me_s as up-client-someip-rust
participant me as mE
ue->>ue_z: register_listener<br>(my_uuri, my_listener)
ue_z->>ue_z: register_response_listener()
me->>me_s: register_listener<br>(service_uuri, service_listener)
me_s->>me_s: registering
st->>st_z: register_listener<br>(zenoh_auth_uuri,<br> forwarding_listener)
st_z->>st_z: register_publish_notification_listener()
st_z->>st_z: register_request_listener() <br> (Zenoh callback puts <br> request_msg.attributes.id <br> into a HashMap<reqid, Zenoh Query>)
st_z->>st_z: register_response_listener()
st->>st_s: register_listener<br>(someip_auth_uuri,<br> forwarding_listener)
st_s->>st_s: registering
ue->>ue_z: send(request_msg)
ue_z->>ue_z: send_request(request_msg)
ue_z->>st_z: Zenoh Queryable
st_z->>st_z: Zenoh Callback for zenoh_auth_uuri <br> (which inserts request_msg.attributes.id <br> into query_map)
st_z->>st_z: on_receive(request_msg)
st_z->>st: send request_msg <br> to TransportForwarder
st->>st_s: send(request_msg)
st_s->>me_s: SOME/IP
me_s->>me_s: on_receive(request_msg)
me_s->>me: react to request_msg
me->>me: process request_msg
me->>me: create response_msg
me->>me_s: send(response_msg)
me_s->>st_s: SOME/IP
st_s->>st_s: on_receive(response_msg)
st_s->>st: send response_msg <br> to TransportForwarder
st->>st_z: send(response_msg)
st_z->>st_z: send_response(response_msg) <br> (Extracts response_msg.attributes.reqid <br> and will look up the stored Zenoh Query)
st_z->>ue_z: Zenoh Reply
ue_z->>ue_z: on_receive(response_msg)
ue_z->>ue: react to response_msg
There was a problem hiding this comment.
@PLeVasseur Good diagram! I think this is correct.
Signed-off-by: ChenYing Kuo <[email protected]>
The PR is to update up-rust to the latest version, including the following changes:
UListenerinterface change withinUTransport#22UUIDs correctly #10