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

Propose ROS 2 launch front-end design hints. #208

Merged
merged 6 commits into from
Jul 15, 2019

Conversation

hidmic
Copy link
Contributor

@hidmic hidmic commented Feb 6, 2019

This is a proposal on how to go about implementing something like #207 in an extensible way. Approaches described in here are a combination of ideas I had and ideas that @wjwwood had and shared with me, thus the dual authorship.

I hope that by the time this gets merged we either have chosen one of these approaches or came up with a another one to actually implement.

@tfoote tfoote added the in progress Actively being worked on (Kanban column) label Feb 6, 2019
@hidmic hidmic added in review Waiting for review (Kanban column) and removed in progress Actively being worked on (Kanban column) labels Feb 6, 2019
Copy link
Member

@wjwwood wjwwood left a comment

Choose a reason for hiding this comment

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

Looks like a good start to me.

As for which implementation to use, the one I mostly closely identified with was FDM+, though perhaps this document is just not compelling enough. We might need to discuss off-line to hash out more detail.

Also, a big thing that is missing (but possibly orthogonal to these approaches) is substitutions and how they would be handled.

articles/160_roslaunch_frontend.md Outdated Show resolved Hide resolved
articles/160_roslaunch_frontend.md Outdated Show resolved Hide resolved
articles/160_roslaunch_frontend.md Outdated Show resolved Hide resolved
articles/160_roslaunch_frontend.md Outdated Show resolved Hide resolved
@hidmic
Copy link
Contributor Author

hidmic commented Feb 13, 2019

Also, a big thing that is missing (but possibly orthogonal to these approaches) is substitutions and how they would be handled.

And I think that's exactly why I skipped them in the first place. The document now covers a bit on how substitutions could be handled. PTAL! And thanks for reviewing it in the first place!


### Forward Description Mapping (FDM)

In FDM, the parser relies on a schema and well-known rules to map an static description (markup) to implementation specific instances (objects).
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
In FDM, the parser relies on a schema and well-known rules to map an static description (markup) to implementation specific instances (objects).
In FDM, the parser relies on a schema and well-known rules to map a static description (markup) to implementation specific instances (objects).


*+* Straightforward to implement.

*+* Launch implementations are completely unaware of the static description existence and its parsing process (to the extent that type agnostic instantiation mechanisms are available).
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
*+* Launch implementations are completely unaware of the static description existence and its parsing process (to the extent that type agnostic instantiation mechanisms are available).
*+* Launch implementations are completely unaware of the existence of the static description formats and their parsing processes (to the extent that type agnostic instantiation mechanisms are available).


*-* Launch system implementations are aware of the parsing process, being completely involved with it if sugars are to be provided.

*+* Allows leveraging the strenghts of each markup language.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
*+* Allows leveraging the strenghts of each markup language.
*+* Allows leveraging the strengths of each markup language.


*+* Allows leveraging the strenghts of each markup language.

*-* Opens the door to big differences in the representation of launch entities across different front end, and even within a given one by allowing the users to introduce multiple custom representations for the same concepts (e.g. a list of numbers).
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
*-* Opens the door to big differences in the representation of launch entities across different front end, and even within a given one by allowing the users to introduce multiple custom representations for the same concepts (e.g. a list of numbers).
*-* Opens the door to big differences in the representation of launch entities across different front ends, and even within a given one by allowing the users to introduce multiple custom representations for the same concept (e.g. a list of numbers).

@ivanpauno
Copy link
Member

@hidmic I addressed the comments, and updated it to match better with the actual implementation (it was actually matching pretty well).

@@ -412,9 +418,6 @@ The abstraction layer allows.

*+* The static description abstraction effectively decouples launch frontends and backends, allowing for completely independent development and full feature availability at zero cost.

*-* No markup language specific sugars are possible.
REVISIT(hidmic): IMHO explicitly disallowing this is a good thing, it makes for more homogeneus descriptions and avoids proliferation of multiple representation of the same concepts (e.g. a list of strings).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ivanpauno this still applies.

@@ -336,31 +343,30 @@ Each launch entity that is to be statically described must provide a parsing pro

*Python*
```python
@launch.expose('some-action') # Infers it's an Action
@launch.frontend.expose_action('some-action') # Infers it's an Action
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ivanpauno I think we should remove # Infers it's an Action.

]
)

@launch.expose('some-subst') # Infers it's a Substitution
@launch.frontend.expose_substitution('some-subst') # Infers it's a Substitution
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ivanpauno I think we should remove # Infers it's a Substitution.

Signed-off-by: ivanpauno <[email protected]>
@ivanpauno
Copy link
Member

I think this PR is ready. We will deal with event_handlers in the next iteration, but I didn't delete reference to them as this is a design hints document.

@ivanpauno ivanpauno merged commit cc9b399 into roslaunch Jul 15, 2019
@delete-merged-branch delete-merged-branch bot deleted the hidmic/roslaunch-frontends branch July 15, 2019 15:24
@wjwwood wjwwood mentioned this pull request Jul 23, 2019
34 tasks
wjwwood added a commit that referenced this pull request Sep 18, 2019
* wip

* more work in progress

* more work done on the calling conventions section

* added context section, still a WIP

* updated event sections

* event filters

* fixup even handler subsection

* typos

* add subsections about the system description (language agnostic)

* Proposal for launching dynamically composable nodes (#206)

* Proposal for dynamically composed nodes

* allow multiple extra_arguments

* Allow node_name and namespace to be empty

* Human readable error message

* Update articles/150_roslaunch.md

Co-Authored-By: sloretz <[email protected]>

* Assign nodes unique ids, but still forbid duplicates

* Update articles/150_roslaunch.md

Co-Authored-By: sloretz <[email protected]>

* Update articles/150_roslaunch.md

Co-Authored-By: sloretz <[email protected]>

* Section to list

* More generic wording about container processes

* namespace -> node_namespace

* _launch/ -> ~/_container/

Signed-off-by: Shane Loretz <[email protected]>

* Propose ROS 2 launch front-end design hints. (#208)

Signed-off-by: Michel Hidalgo <[email protected]>
Signed-off-by: ivanpauno <[email protected]>

* Adds ROS 2 launch XML format design document (#207)

Signed-off-by: Michel Hidalgo <[email protected]>
Signed-off-by: ivanpauno <[email protected]>

* Update ROS launch documentation (frontend documents) (#247)

Signed-off-by: ivanpauno <[email protected]>

* review fixup

Signed-off-by: William Woodall <[email protected]>

* clarify lifecycle transition due to review comment

Signed-off-by: William Woodall <[email protected]>

* provide rationale for shutdown procedure

Signed-off-by: William Woodall <[email protected]>

* Add launch XML substitution for a packages share directory (#254)

* Add launch XML substitution for a packages share directory

Rename find-pkg to find-pkg-prefix.
Add find-pkg-share substitution for the share directory.

Signed-off-by: Jacob Perron <[email protected]>

* remove some old rfc's and general cleanup for first merge

Signed-off-by: William Woodall <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in review Waiting for review (Kanban column)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants