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.

Sign up for GitHub

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

Jump to bottom

Initial implementation of up-linux-streamer #16

Merged
merged 1 commit into from
Aug 7, 2024
Merged

Initial implementation of up-linux-streamer #16

merged 1 commit into from
Aug 7, 2024

Conversation

PLeVasseur
Copy link
Contributor

@PLeVasseur PLeVasseur commented Jun 17, 2024 •
edited
Loading

Outline

Basic showcase of how a up-linux-streamer can work to bridge the RPC flow between mE and uE between SOME/IP <--> Zenoh

TODO

Covers #4, #5

@PLeVasseur PLeVasseur force-pushed the feature/linux-streamer branch from 1151b40 to 10b6358 Compare June 19, 2024 14:46
This was referenced Jun 20, 2024
Closed
Merged
@PLeVasseur PLeVasseur mentioned this pull request Jul 17, 2024
Closed
sophokles73
sophokles73 reviewed Jul 18, 2024
View reviewed changes
Cargo.toml Show resolved Hide resolved
up-linux-streamer/src/main.rs Outdated Show resolved Hide resolved
up-linux-streamer/vsomeip-configs/point_to_point.json Outdated Show resolved Hide resolved
up-linux-streamer/examples/uE_client.rs Show resolved Hide resolved
up-linux-streamer/examples/uE_client.rs Show resolved Hide resolved
up-linux-streamer/examples/uE_service.rs Show resolved Hide resolved
up-linux-streamer/examples/mE_service.rs Show resolved Hide resolved
up-linux-streamer/examples/mE_client.rs Show resolved Hide resolved
@PLeVasseur PLeVasseur mentioned this pull request Jul 18, 2024
Open
@PLeVasseur PLeVasseur changed the title Initial implementation - very thread-bare - of up-linux-streamer Initial implementation of up-linux-streamer Jul 18, 2024
@PLeVasseur PLeVasseur requested a review from sophokles73 July 31, 2024 16:16
@PLeVasseur PLeVasseur marked this pull request as ready for review July 31, 2024 16:16
sophokles73
sophokles73 reviewed Aug 1, 2024
View reviewed changes
Copy link

@sophokles73 sophokles73 left a comment

Choose a reason for hiding this comment

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

It looks like currently the streamer is limited to using the Zenoh and the VSomeIP transport only. I would suggest to start decoupling from the concrete transport ASAP because this will make apparent any required changes to the transports very quickly ...

@PLeVasseur
Copy link
Contributor Author

PLeVasseur commented Aug 1, 2024 •
edited
Loading

It looks like currently the streamer is limited to using the Zenoh and the VSomeIP transport only. I would suggest to start decoupling from the concrete transport ASAP because this will make apparent any required changes to the transports very quickly ...

The underlying pluggable library (up-streamer) is decoupled and independent.

The binary which does the work of plugging in concrete transports does need to depend on the concrete implementations, right? I plan to peg those to a tagged version for Zenoh and SOME/IP in the Cargo.toml to prevent main from breaking.

Does that plan make sense?

Or would it make more sense to leave pointing to main and then enable a nightly CI to check for breaking changes in transports & up-rust?

@sophokles73
Copy link

FMPOV you should guard support for specific transports using Rust features. When building the application, users can then easily enable only those transport(s) that they need.

@PLeVasseur PLeVasseur mentioned this pull request Aug 5, 2024
Open
@PLeVasseur PLeVasseur requested a review from sophokles73 August 5, 2024 22:42
sophokles73
sophokles73 reviewed Aug 6, 2024
View reviewed changes
README.md Outdated Show resolved Hide resolved
example-utils/hello-world-protos/Cargo.toml Outdated Show resolved Hide resolved
example-utils/hello-world-protos/build.rs Outdated Show resolved Hide resolved
example-utils/integration-test-utils/Cargo.toml Show resolved Hide resolved
@PLeVasseur PLeVasseur force-pushed the feature/linux-streamer branch from 896433a to c5f58b6 Compare August 6, 2024 14:57
@PLeVasseur
Copy link
Contributor Author

FMPOV you should guard support for specific transports using Rust features. When building the application, users can then easily enable only those transport(s) that they need.

This makes some sense. I'll have to think about this a bit. Opened an issue here: #32

@PLeVasseur PLeVasseur requested a review from sophokles73 August 6, 2024 15:15
sophokles73
sophokles73 approved these changes Aug 7, 2024
View reviewed changes
Copy link

@sophokles73 sophokles73 left a comment

Choose a reason for hiding this comment

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

LGTM

@PLeVasseur
[#16] Add stand-alone uStreamer binary
4716a05 
* Add up-linux-streamer
  * Include ability to pass config file
  * Include default config file with comment explaining
  * Ability to pass --no-default-features to bring own vsomeip shared libraries
* Add examples and documentation on how to use them with up-linux-streamer
  * These use the HelloRequest and HelloResponse COVESA uService helloworld .proto objects
  * mE_client <=> uE_service
  * uE_client <=> mE_service
* Update CI to test both bundled and unbundled vsomeip library configurations

Implements [#4], [#5]
@PLeVasseur PLeVasseur force-pushed the feature/linux-streamer branch from c5f58b6 to 4716a05 Compare August 7, 2024 12:06
@PLeVasseur PLeVasseur merged commit 74a2a5a into eclipse-uprotocol:main Aug 7, 2024
8 checks passed
@PLeVasseur PLeVasseur mentioned this pull request Aug 7, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dmacattack dmacattack dmacattack left review comments

@sophokles73 sophokles73 sophokles73 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@PLeVasseur @sophokles73 @dmacattack