-
-
Notifications
You must be signed in to change notification settings - Fork 80
feat: initial ROS2 AsyncAPI contribution by SIEMENS AG #270
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
Conversation
…ort and is corporate internal legally cleared for OSS contribution.
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.
Welcome to AsyncAPI. Thanks a lot for creating your first pull request. Please check out our contributors guide useful for opening a pull request.
Keep in mind there are also other channels you can use to interact with AsyncAPI community. For more details check out this issue.
fmvilas
left a comment
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.
Left a round of review. Thanks for putting it together, Florian 🙏
Achllle
left a comment
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.
My 2c as a ROS dev: ROS 2 parameters are a must-have for this to represent a ROS 2 interface.
@Achllle while we think that could be really interesting to include the parameters, we don't think that it is a hard requirement to have them to be able to have the first version of the ROS2 bindings. |
|
Considering the ROS 2 version of this effort, node_idl / NoDL includes them in their spec, I would argue this is a must-have for a complete spec. It's fine by me if it happens in two phases, but the point of a spec is that it's complete. |
yes. I agree that parameters need to be added to the ros2 bindings. In our use cases, we did not use them so far. That is why they slipped our implementation. I would like to add them in a second step. From what I've gathered, parameters are interfaced through fixed services that all nodes have, unless pro-actively deactivated. What can be transported is also fixed in terms of data / message types. So like you put it, it might only be necessary to add to a node==application what parameters it has and probably not list all the additional We need to investigate this more. I still am having trouble how it embeds into an asyncAPI binding.. Is somebody in the ros-infrastructure team already familiar with asyncAPI @Achllle ? |
|
The best way to get involvement from the ROS community would be via a post in ROS discourse! |
https://discourse.ros.org/t/invitation-to-collaborate-on-asyncapi-specification-for-ros2/42915 @fmvilas if you have the time, it would be a pleasure to have you join such a meeting. |
1. Include in the server binding links and change to none instead of localhost. 2. Include the link to the non-negative less than 232. 3. Explain better the message binding with links. 4. Put at the end a whole example of a ROS application with its explanation.
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.
ROS2 -> ROS 2
Co-authored-by: Christophe Bedard <bedard.christophe@gmail.com>
|
@fmvilas there might be another repo waiting for this to become official.. 😇 |
|
haha! love the name 🌹 |
|
@fmvilas any update on the V3.1 minor release? 🙂 |
Release on me, my bad. I'll prepare needed things in a few days |
|
in case you missed asyncapi/spec-json-schemas#582 (comment) @amparo-siemens @gramss I'm available for onboarding call. Please reach out to me at lpgornicki@gmail.com to sync on the exact day and time. It's purely bindings maintainance onboarding but I'll also share overall info about how the AsyncAPI Initiative operates. |
Sorry, we missed it. We will reach you now 🙂 |
|
Hey folks, fyi we scheduled our Spec Triage meeting: asyncapi/spec#1131 (comment) First time in a very, very long time 😄 I think v3-trauma has passed 😄 |
|
/dnm |
fmvilas
left a comment
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.
LGTM 👍
|
/rtm |
|
next steps -> asyncapi/spec-json-schemas#588 |
Description
Maintaining will be done through @amparo-siemens and me.
Further maintainers and improvements suggested in #254 are highly welcome.
This binding is tested already in a few internal tools and generators as seen in this presentation: ROSCon DE
cc: @renzo-sie (thank you for the preparations!) and @fmvilas