-
Notifications
You must be signed in to change notification settings - Fork 11
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
Use Eclipse uProtocol for sending vehicle status updates #60
Use Eclipse uProtocol for sending vehicle status updates #60
Conversation
aade6f0
to
5300973
Compare
5597d1a
to
c35ceea
Compare
@eriksven would you mind taking a look? |
c9e5e1d
to
710a8e8
Compare
The FMS Forwarder has been changed to use uProtocol Publish messages for sending vehicle status updates to the back end (FMS Consumer). The direct connection to the InfluxDB has been removed in favor of the Zenoh based uProtocol transport. As an alternative, custom uProtocol transports for Hono's MQTT adapter and the Kafka messaging infrastructure have been added.
710a8e8
to
0f7ff6a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added some comments. But I am fine with merging the PR as is.
The comments could be resolved in subsequent PRs as well.
I am very sorry about the long response time. Somehow this PR slipped through on my end during the end last year.
|
||
# Quick Start | ||
|
||
The easiest way to set up and start the services is by means of using the Docker Compose file in the top level folder: | ||
|
||
```sh | ||
docker compose -f ./fms-blueprint-compose.yaml up --detach | ||
docker compose -f ./fms-blueprint-compose.yaml -f ./fms-blueprint-compose-zenoh.yaml up --detach |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At this point, I did not read through the whole PR yet. But to me it seems easier to have the "default" configuration in a single compose file and then have the different customizations in separate files. My hope is that it becomes easier to get familiar with this blueprint if there is "red thread" provided before one starts to deviate from the default route.
Just by reading the Readme it seems that you made this blueprint even more customizable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After reading the whole PR, I see the compromise that you made to not repeat yourself in the "-zenoh.yaml" or "-hono.yaml". With that in mind I can live with having to list two YAML-files in the command.
|
||
<img src="img/architecture.drawio.svg"> | ||
<img src="img/architecture-uprotocol.drawio.svg"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Someone who is new to the blueprint and the underlying technology stack of Eclipse uProtocol and Eclipse Zenoh might be a bit confused about what is going on here. Because there are two architecture pictures (architecture-uprotocol
and architecture-zenoh
) where the text says for both that they are the default and that differ in the connection between the FMS-Forwarder and the FMS-Consumer (connection with uProtocol, or additional component with Zenoh router). Both are correct, if you deploy the Zenoh version by default. But newcomers might not be aware that the picture show different layers of the communication stack.
My proposal would be to make this difference more clear in the description of the Zenoh transport. E.g. by explaining that uProtocol can operate over multiple transports and that we in this specific case recommend to use Zenoh with an additional Zenoh router.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will replace the Zenoh and Hono specific component diagrams with deployment diagrams. This should make clear that one is the logical (component) view while the other is about the containers being deployed (and involved).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It could be an idea to add a logo for Eclipse uProtocol next to the connection between FMS Consumer and FMS Forwarder. Using Eclipse uProtocol is one of the main features here and this way it would be promoted more explicitly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will do
components/fms-util/src/lib.rs
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fms-util could be a bit misleading. How about something more Zenoh specific? I think, it would be better to reflect the multiple options for the transport across (e.g., Hono, Zenoh), across the whole project. This way it seems a bit Zenoh is the "first-class citizen" and not one of mulitple options even it is the default at the moment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you suggest to rename the crate to zenoh-utils
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, in that case something in the direction with at least zenoh in the name.
@sophokles73 I could merge this PR, but left some comments too. Let me know if should wait for further changes or can merge this PR. |
Let me make a few changes based on your (very valid) comments and then I will merge it myself... |
Updated README and diagrams Renamed fms-util crate to fms-zenoh
The FMS Forwarder has been changed to use uProtocol Notifications for
sending vehicle status updates to the back end (FMS Consumer).
The direct connection the back end InfluxDB has been removed in favor
of the Zenoh based uProtocol transport. As an alternative, uProtocol
transports for Hono's MQTT adapter and the Kafka based north boud API
have been added.