Skip to content
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

Update up-client-zenoh-rust with up-spec 1.5.7 #21

Merged
merged 27 commits into from
Apr 4, 2024

Conversation

evshary
Copy link
Contributor

@evshary evshary commented Mar 26, 2024

Since up-rust is upgraded to up-spec 1.5.7 in eclipse-uprotocol/up-rust#66, the PR is to keep the code updated.
I'll update https://github.com/eclipse-uprotocol/up-zenoh-example-rust later

Signed-off-by: ChenYing Kuo <evshary@gmail.com>
Signed-off-by: ChenYing Kuo <evshary@gmail.com>
Signed-off-by: ChenYing Kuo <evshary@gmail.com>
Signed-off-by: ChenYing Kuo <evshary@gmail.com>
Signed-off-by: ChenYing Kuo <evshary@gmail.com>
Signed-off-by: ChenYing Kuo <evshary@gmail.com>
Signed-off-by: ChenYing Kuo <evshary@gmail.com>
Signed-off-by: ChenYing Kuo <evshary@gmail.com>
Signed-off-by: ChenYing Kuo <evshary@gmail.com>
Signed-off-by: ChenYing Kuo <evshary@gmail.com>
Signed-off-by: ChenYing Kuo <evshary@gmail.com>
Signed-off-by: ChenYing Kuo <evshary@gmail.com>
Signed-off-by: ChenYing Kuo <evshary@gmail.com>
@evshary
Copy link
Contributor Author

evshary commented Mar 28, 2024

@PLeVasseur I think the PR is ready. There are a lot of changes, but it should not affect your uStreamer implementation since I didn't change the API (except removing RpcServer)

Here are the modifications:

  • Upgrade to the latest up-rust to match up-spec 1.5.7
  • Update the way of error logging
  • Support initializing the constructor with UUri (will generate randomly if we don't assign it)
  • Reorganize the test code

Hope it's compatible with your uStreamer implementation

src/lib.rs Outdated Show resolved Hide resolved
src/lib.rs Outdated Show resolved Hide resolved
src/lib.rs Outdated Show resolved Hide resolved
src/lib.rs Show resolved Hide resolved
src/lib.rs Outdated Show resolved Hide resolved
src/rpc.rs Outdated Show resolved Hide resolved
src/rpc.rs Outdated Show resolved Hide resolved
src/utransport.rs Outdated Show resolved Hide resolved
src/utransport.rs Outdated Show resolved Hide resolved
src/lib.rs Outdated Show resolved Hide resolved
Signed-off-by: ChenYing Kuo <evshary@gmail.com>
Signed-off-by: ChenYing Kuo <evshary@gmail.com>
Signed-off-by: ChenYing Kuo <evshary@gmail.com>
Signed-off-by: ChenYing Kuo <evshary@gmail.com>
…tener

Signed-off-by: ChenYing Kuo <evshary@gmail.com>
Signed-off-by: ChenYing Kuo <evshary@gmail.com>
Signed-off-by: ChenYing Kuo <evshary@gmail.com>
Signed-off-by: ChenYing Kuo <evshary@gmail.com>
Signed-off-by: ChenYing Kuo <evshary@gmail.com>
Signed-off-by: ChenYing Kuo <evshary@gmail.com>
@evshary
Copy link
Contributor Author

evshary commented Apr 2, 2024

@PLeVasseur I think the PR is ready for your second review 😄

src/lib.rs Show resolved Hide resolved
src/lib.rs Show resolved Hide resolved
src/lib.rs Show resolved Hide resolved
@PLeVasseur
Copy link
Contributor

@PLeVasseur I think the PR is ready for your second review 😄

Finished round 2 -- looking good 🙂

Had one item that I think would be good to check on:
#21 (comment)

Signed-off-by: ChenYing Kuo <evshary@gmail.com>
@evshary
Copy link
Contributor Author

evshary commented Apr 3, 2024

@PLeVasseur Thank you! Let me know if you have any other comments.

src/utransport.rs Outdated Show resolved Hide resolved
src/utransport.rs Outdated Show resolved Hide resolved
@PLeVasseur
Copy link
Contributor

@PLeVasseur Thank you! Let me know if you have any other comments.

Hey @evshary -- I missed this tiny nitpick which I think gives symmetry. Could you take a look?

Signed-off-by: ChenYing Kuo <evshary@gmail.com>
@evshary
Copy link
Contributor Author

evshary commented Apr 3, 2024

@PLeVasseur Done and your comment also reminds me to remove some TODO in code since eclipse-uprotocol/up-rust#75 is merged

Copy link
Contributor

@PLeVasseur PLeVasseur left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@evshary
Copy link
Contributor Author

evshary commented Apr 3, 2024

@stevenhartley Could you help me to merge the PR? I would like to work on eclipse-uprotocol/up-spec#103 based on the PR's changes.

@stevenhartley stevenhartley merged commit 69122c2 into eclipse-uprotocol:main Apr 4, 2024
4 checks passed
@evshary evshary deleted the upgrade_up_rust branch April 5, 2024 02:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants