Actions #583
Comments
|
rcl implementation ticket: ros2/rcl#306 |
|
@sloretz - I was looking for a status update on Actions, I believe that they have been implemented everywhere except for rclpy at this point, but not totally sure. Can you clarify? Also, I don't see anything here about adding tutorials or examples to the wiki pages, or any other documentation. Is usage documented somewhere? Sorry if this is the wrong place to ask this question you can redirect me. |
|
@mkhansen-intel
This is correct, all that remains is the rclpy implementation (currently in progress) along with some more communication tests.
Some minimal examples for rclcpp can be found here: https://github.com/ros2/examples/tree/master/rclcpp Python examples (WIP): ros2/examples#222 I think it would be nice to add some tutorials to the wiki at some point. I can add it to the checklist. |
|
@jacobperron - thank you for the update and for adding a line item for tutorials. I'll keep watching for updates. |
|
@jacobperron - Thanks for the pointers. I see that the rclcpp_actions.hpp is separate from rclcpp.hpp. Do you plan to include rclcpp_actions.hpp within rclcpp.hpp or keep them separate? |
We decided to keep them separate for flexibility. It allows the user to optionally depend on the ROS action library. This is following the same pattern as |
|
It still seems to me like actions is being treated like a second-class citizen then. I thought the goal for ROS2 Actions was for it to be a first-class citizen, equal to topics and services. |
First-class citizen doesn't need to imply monolithically build. First-class citizen means that it is fully supported and conceptionally doesn't have any cases which don't fit the rest of the system. An example for second-class would be nodelets in ROS 1 because once you converted nodes into nodelets you wouldn't be able to introspect the topic between them anymore. |
|
Sorry if I'm splitting hairs here, but I thought that rclcpp was a facade design pattern for the system, which implies to me that all first class features should be accessible through that interface. |
|
Actually, as long as I'm stirring the pot here, I may as well do it right. I think actions should be part of the node interface such that the code to use it would go from this: To this: |
|
For me, "first class" refers to support and availability, not location in the API, but that's just my opinion. Long term my preference would be (as @sloretz said) to have rclcpp broken into parts and then the However, we may not have time to do that, so in the meantime I would prefer what we're currently doing which is adding new things to their own packages rather than continuing to put everything in As for:
It's just a matter of taste I suppose, but I very much disagree with this idea. I would much rather have interfaces or classes which used node's as an input than have the node class have methods for each concept. Some of the reasons for this are:
Again, in this case we're inconsistent, as concepts like Parameter (which to me a similar in classification with Action) are part of the Node class rather than free-functions or classes that interact with Node. If I had my way we'd deprecate and then remove the methods related to parameters from the Node class and instead have a Parameter class which takes one or more Node Interfaces as arguments. Topics (pub/sub) and Services (client/server) are special for me, however, because they are "primitives" to me, in ways that Parameter and Action are not, i.e. Parameter and Action are not in the That all being said, it's mostly my opinion. I think there's a valid argument to prefer a method on Node for creating actions so that it is symmetric with how topics work or that it's just more convenient or that it provides a more "first class" feel. |
|
@wjwwood - thank you for the detailed explanation. As a ROS2 user, I find the inconsistency to be confusing. It seems like rclcpp was going to be a Facade, but now it's not. So having it be a partial Facade means someone like me has to remember the exceptions to the pattern. I think you need to be consistent. Likewise, if topics and services are available through the node interface, so should actions and parameters. I know why you're saying they topics and services are 'special' because they map to rmw layer concepts, but isn't the point of the rcl and rclcpp layers to encapsulate and abstract that away from the user so that they don't have to know about the rmw layer? That's why I thought actions would be treated as equal to topics and services, even though under the hood they are wrappers around those. The idea is that the user doesn't have to know those internal details. They just have to know the message definitions and what callbacks to expect. So I guess I could see going either to the "No Facade" design where everything is included explicitly when it is needed, or sticking with the Facade design pattern and being consistent. Right now this feels like its a half and half solution which means users like me have to know all the exceptions to the rules. |
|
Well, given enough resources to actually implement it, my preferred end state is that At that point whether or not it is in the Node class or not is aesthetic only in my opinion. Even if we have Unfortunately, in my opinion things like API review/refactoring do not get the attention they deserve, as stake holders in ROS 2 are always asking for more features rather than polish on what already exists. |
I agree. A facade API doesn't preclude good design below. In fact it should make refactoring easier. :)
Also agree, this is essentially the same as create_publisher.
Well, this is why I'm trying to bring it up now. I don't think many users have started to use Actions yet, so it's not to late in this case IMO to put it behind the facade, ideally also within the Node class. Along these same lines, can we also talk about lifecycle nodes? :) Why aren't they derived from and extending the node class instead of duplicating it? :) Maybe another thread is needed for that discussion. |
Briefly, LifecycleNode did not want to expose a |
|
I agree with @mkhansen-intel - he's brought up some great concerns here! We should fix the actions API now to be consistent with the messages, services, and parameters API. The idea that "Long term" we'll fix rclcpp seems like another way of saying never, given how actions never changes in ROS 1. How has actions2 changed its status as a "first class citizen" compared to ROS1? It seems like they are still add-ons given the current state in ROS2.
Why is this an acceptable end result? The original ROS1 was designed to make software development for roboticits easier. Why can't we keep that as a focus?
Why is it important to keep actions out of the core Node? Actions aren't some heavy dependency, they simply combine messages and services in a useful way to many many robot applications. They are a very core part of ROS that many many ROS packages use. So to say otherwise is to call them second class citizens.
Why would actions need to be decoupled later? Since the earliest times of ROS2 discussion everyone has said we wanted actions to be part of the core ROS2 feature set, and now we're changing direction? What do you mean by couple them in bad ways? We use a ton of actions in Navigation and MoveIt! and I don't know what you're referring to.
Actions are not a new concept and are something that should be 100% supported by "us", or the maintainers of core ROS2.
This seems to contradict the claim that ROS2 will treat actions as 1st class citizens Let's keep the ROS2 API simple and consistent for the masses to use, without lots of special rules and irregular patterns! Not everyone geeks out on middleware all day and instead must focus on higher level robot applications. Let's make it easier for them to have preempt-able communication between robot nodes, not harder. |
I agree they should be made consistent, but I disagree with which direction we should move to make things consistent. I think parameters should be modularized like actions are currently modularized. This does not, however, speak at all to the level of support or availability for Actions compared to Topics or Services. This is expressed in things like support in the shared C API and support in more client libraries like rclpy, not by where the function to create them lives (in the Node class or as a function in the
They're going to be supported in Python and C++ and Python will share an implementation in the C API, and there's support for them in the typesupport system, and there's command line tools. rosbag will also be "actions" aware. All of these things make it more integrated, or "first-class". Why do you feel that it is an add-on?
It's not the end result, as I've said multiple times in this thread, my preferred end state is not like that comment describes, it would be consistent and convenient.
It is the focus, but features have to work correctly before they can be polished. And I'm sure you guys know that this kind of thing doesn't just happen, it takes a lot of work and we have constrained resources. So if you see something like this I'd ask you to not assume it's "acceptable" to us or the "end result", but instead try to understand that it may not be finished, ask if it is or is not what we intended or if it's unfinished, and maybe consider how you might help make it better.
Perhaps my perspective is uncommon due to working with a multitude of different ROS users, but modularity and a lean core is (in my opinion) important because not everyone wants or needs every feature. There's a design principal that says "if I don't use a feature, I shouldn't pay for it". This shouldn't be followed blindly or absolutely, but there is wisdom in it, and a common complaint of ROS 1 was that, despite the federated design making large parts optional (tf, robot model, etc...), parts of the core not being optional was a barrier to entry. For some users, every feature that goes beyond their needs is just something else they need to verify, certify, or otherwise process when incorporating it into their software. I much prefer a "small sharp tools" approach to design which has many decoupled parts working together to make your system exactly as complex as needed but no more. That leads me to prefer that all concepts that can be done via composition rather than aggregation should be outside of the core. Providing a unified API to make development convenient and polished is also important and not at odds with a modular design. I also think that the design can be modular and still provide "first-class" support for a feature or concept, but I've already spoken about that in this thread already.
Again, having it in a separate package or not be a method in the Node class does not equate to being "out of the core" or that we're demoting it in anyway. Again, my preference would be that actions are part of
I mean couple them such that topics cannot be used without actions, or that community members cannot extend the functionality without getting code changes in our repositories (because we're not dog-fooding and instead are using back channels to make things happen). There's nothing wrong with Navigation or MoveIt! using the breadth of features, but I think it's important to remember, you are not the only shareholders in the community and your use cases are not the only ones that matter.
No one said otherwise as far as I can tell. The point is that if a third-party cannot add a feature like actions without major changes in our code, then we've got some design issues.
Why? If I could decouple services, topics, and nodes in a meaningful way, I would do so (and at least for services it may be possible), so the distinction that makes them "special" is that they cannot be decoupled, but that is all. Keeping actions or parameters integrated, despite the fact that they do not need to be, does not make them first-class. I'll say it again, location in the API does not equate to "first-class", support and availability do. The status of actions in ROS 2 hasn't changed, no one has said that having it in the Node as a method is out of the question, nor has anyone suggested demoting the importance of actions, nor is anyone in favor of making it harder to use. |
|
Thanks for your quick and detailed response @wjwwood !
It feels like an add-on if its a different header file than messages and topics and if its used in a different pattern with ROS nodes. But I understand your overall point to be that, just because it feels like an addon, doesn't mean it is.
Yes, I do understand that. Notably I have not been involved in developing actions, so in a do-acracy I have no votes.
This point does concern me, though, because it is likely the end result unless people like @mkhansen-intel make a fuss about it.
Interesting points. If this is the measuring stick, MoveIt! fails horribly :-/
You did say "a new concept (like we have done with actions)" but I guess I misinterpreted that.
I don't think we need to conflate the decision to keep actions more external from the capability to allow other third-party features to be external. Actions can be fully integrated and yet we still have the ability to allow third-party add-ons. Perhaps you can use some other feature as the example of how to have add-ons?
While many of your points I can't argue any further without having a deeper understanding of ROS2, I have used ROS2 enough to believe that plenty of people are in favor of making ROS harder to use. Of course, "harder to use" may be absolutely necessary to achieve the more robust software design patterns necessary for industry and so it may be unavoidable. But I think we should be honest and admit that ease of use is low on the priority list of ROS2 design, as evidenced for example by these discussions. |
I stated several times that the end goal (at least for me) would be to have actions be included automatically when including
I just don't see why this is important. If it's included in
I don't agree. It implies that the people working on it are satisfied with an unfinished product and would not do anything about it in time without being prodded into it. It simply isn't true. We also have big holes in our documentation, but it's not our intention to leave it that way, nor will "fussing" about it magically help us fill those holes. You don't have to convince us that polishing the API and improving documentation is important.
I think we're misunderstanding each other here, I mean that actions is like an as-yet-unimagined new concept in that it could be added with minimal modification to the core, just as we did with actions. Not that actions is a new concept or somehow third party or less important.
The whole idea of "dog fooding" is that core, fully supported features are the example of how a third party could extend the system. For everything but the last step, i.e. putting actions in the rclcpp header or in the Node class itself, it should be the same for a core feature like parameters or actions and some third party concept. A good example of this is rviz. All of the "default" functionality in rviz is realized as plugins, meaning that anything one of "our" plugins can do, your plugin can do. Until now we've done everything but that last step for actions, mostly because we haven't had time to do so. I mean we barely got actions done in time for Crystal (didn't even get Python done).
This is again misconstruing a work in progress with an intentional final state. I think it's ridiculous to conclude we sat in a meeting and decided to make it hard to use. It is true that we have to change some things to enable new use cases, and sometimes that makes the interface more complex, but we work hard to allow "simple things to be simple" even if we cannot realize them today. We take added complexity very seriously, often drawing criticism from people that we don't expose more complexity (e.g. exposing more and more features from DDS). It's constant task to balance functionality and complexity. It is also true that we have to prioritize feature development over documentation, polish, and general ease of use, but that's not because we want to, or because we want to make it "elite" or hard to use. It's because the resources flowing into the project are all marked for feature development, full stop.
Perhaps, but I do not believe that's the case here.
I don't dispute this point (that it's a low priority), but it's not low priority because we don't care about it (which has been the implication I've gotten from this conversation), instead it's because we have to deliver the features and the minimal viable product to deliver those features does not include API review, extensive documentation, or superfluous work (that includes extra sophistication in the technical design as well as syntax sugar). Also, just because it's a low priority right now doesn't mean it's not still a goal of the project. We've done quite a bit in the name of ease of use and ease of migration which has cost us significantly. It's very frustrating to have someone come in on this narrow bikeshed issue and declare we care nothing about it nor have we invested anything into it. It simply isn't true, and honestly until you've worked on these core features some and tried to deliver a feature on schedule you wouldn't know. |
|
@wjwwood - Early in the thread I asked if the intention was to move Actions into the rclcpp interface. I got conflicting answers.
Reply from @jacobperron:
Reply from @sloretz:
So I made the technical argument (which @davetcoleman has now also chimed in agreeing to) that putting it in the rclcpp interface and preferrably in the Node class interface is desirable from the users standpoint for consistency and ease of use. I believe your latest response is saying that you agree that the ideal long-term design should be to put Actions in the rclcpp interface, but since you're resource constrained it hasn't been done yet and you don't know when it can or will be done. SO we need to separate the technical design discussion from the resource discussion. I think we're in agreement on the technical design discussion at least up to the point of including it in the rclcpp interface. Am I right? I think the technical desire we have (Dave and I) is to go one step further to include it in the Node interface, but I won't die on that hill today, if we can at least agree on the rclcpp inclusion. Separately, we can have a discussion on the resourcing of the work, if that's the real issue preventing this from being done. Maybe we can help with that. |
Yes, you're right. Though I think I should be as clear as possible on my position: I think the code for actions should continue to live in Perhaps something missing from this discussion is why we haven't done what I described above yet (either for lifecycle or actions). Basically you cannot just So the actual (minimal) work to be done is to:
Now, that's also not that much work, though you have to schedule it at a time where it is least disruptive to other But there's a lot of other design choices to be made too, e.g.:
namespace rclcpp { using namespace rclcpp_core; }
So, maybe if you didn't have this context, I would have sounded like I was making a mountain out of a mole hill, but it's actually more involved that it first appears. We have discussed doing this already, that's when we came up with these issues blocking it. I would also consider actually moving Finally, I want to apologize for letting out some frustration in the previous comments, it pushed a few of my buttons, because I know our team worked really hard on actions (I actually did basically nothing on it) and I felt the criticism was accurate in places but in others not deserved. Either way, I almost derailed the conversation when we basically started out agreeing, which is silly. So sorry about that, and thanks for taking the time to give your feedback, it is helpful even if we've already thought about addressing this issue. <3 |
|
@wjwwood sorry for taking this conversation off topic and being overly judgmental
If this is your stance than I totally understand
Again, my apologies I do want to assure you that my overall opinion of the core ROS team's efforts / OSRF's efforts are very positive and understanding of limited resources. I'm a big proponent of ROS2 and am advocating its adoption worldwide. It greatly concerns me when ROS2 is harder to use than ROS1 and making a clear case for upgrading is difficult, and I think little examples like this one get me on a soap box. But now I feel like I fully understand your position and background reasoning so will shut up. |
|
@wjwwood - no apology needed. I do realize you guys have too much to do (thus the reason we're trying to engage more to help), and that @jacobperron and @sloretz did a lot of work on this. And I'm even OK with this being the "first release" version. I never meant anything to be critical of anyone's work. I just wanted to talk about this from a purely technical standpoint. This points to another issue in my mind which is lack of a design forum. I know there's a github repo for design discussion but for things like this maybe we need a working group for more real-time discussion? As far as the rclcpp circular dependency issue, can that be broken by relying on the underlying interfaces / headers that are contained within rclcpp instead of rclcpp itself? |
Possibly, but I think it currently uses the rclcpp versions of things. So we'd have to at least refactor it to not use those anymore if that's even possible. |
|
For those interested, the first iteration of |
|
@gbiggs - this is the actions thread (and lifecycle discussion) I mentioned to you |
Did I miss another one of the Evil ROS Committee meetings? I'm going to get thrown to the lasersharkrobots if I miss too many more. I completely missed this discussion because I have been changing jobs and took a month completely off from everything work related. Actually I guess I completely missed every discussion for the past month, but anyway. In general I agree with the approach being advocated by @wjwwood. Splitting the different functionalities out and then composing them as needed, with an "all of it" option provided by the core source for ease of use, is the best approach. There are different use cases that want different sets of ROS2 features, and I don't believe there is any overlap between every use case, with the possible exception of topics (and I have my doubts about that). Regarding the lifecycle node implementation, I have concerns about that. Rather than inheriting the basic node interface, it re-defines the whole thing. This is going to be an increasingly annoying maintenance burden, I think. Different/new node types should not have to duplicate all of The approach used by the lifecycle node implementation works for now, but what will happen when someone produces another type of node that alters another aspect of the basic node's behaviour? I won't be able to use that and lifecycle nodes together easily. As it currently stands that new type would have to inherit from the lifecycle node interface and use a lifecycle node internally for the node implementation. This becomes an increasing problem as new types of nodes get developed. Maybe we won't see many or any new types of nodes and this won't be a problem, but I think this is in the same boat as @wjwwood's wish to allow new features to be implemented by everyone: we should not preclude it just because we don't know if it will ever happen or not. The approach taken by the lifecycle node implementation is exactly the sort of thing Scott Meyer goes on about in his books all the time. In languages that support interfaces, I would prefer to see an approach more like mixins or something similar. I should be able to create a new node type as an interface and an implementation, and allow (advanced) users to mix that interface with other node type interfaces that also exist. I know that this will require documentation of what each interface expects and careful implementation of the mixed node type by the user, which is why I say it's for advanced users. But I believe this sort of flexibility would be a benefit to the ROS2 API, and is achievable without making the API harder to use or maintain. Another concern I have is that the lifecycle node definition being a redefinition of the basic node means that I have encountered situations where there were functions that worked with "standard" nodes but not lifecycle nodes. I forget exactly where and I was stupid to not ticket them as I found them because obviously they need to be fixed, but this sort of thing shouldn't happen at all and is an example of the maintenance burden created. It wouldn't happen if the lifecycle node used interface inheritence or the API followed a policy of making anything that operated on a node a template method (probably the former is better if we need to keep binaries smaller and compile times lower). Either approach provides for mixing interfaces and I would be happy with either. The above is really rambly but I hope my points are understandable. 😃
I'm confused about this. Why doesn't the C++ client library also share an implementation with the C library? I thought the whole point was that every client library would have a common implementation through the C library ensuring consistency. Is this a resourcing issue where in the future the C++ library will just wrap the C library but for now a C++ API was the priority? |
I think this is a misunderstanding. The C++ client library does use a common C library, rcl_action. |
|
So my team is planning to implement Actions and Lifecycle nodes in Navigation2 within the next few months, in time for the 'Dashing' release. I believe there is still discussion needed on both. However, I don't believe this is the correct place for the discussion. Please tell me what the correct place / thread is for that? |
I'm sure you're familiar with this already, but for anyone else http://wiki.ros.org/Support has general guidelines. Specifically for actions, ros2/design#193 is a good spot for issues with how actions are designed. Problems with how the implementation or feature requests would be good as issues on https://github.com/ros2/rcl or https://github.com/ros2/rclcpp . Questions about using actions would be good at https://answers.ros.org/questions/ . This issue is a good spot to talk about Actions features that will be released into Crystal. Anything else probably fits at https://discourse.ros.org/c/ng-ros For lifecycle nodes, I think issues at |
|
Python support for actions has been implemented and backported to Crystal. We have tickets for the unchecked tasks:
I don't think these are planned to be backported into Crystal and so I think we can lay this ticket to rest 🎉 |
ROS 2 Action Plan
At a high level the goal for actions in Crystal is to enable porting moveit and the navigation stack to ROS 2.
Required by Crystal release
ros2 topic listandros2 service listshould not show action topics by defaultIssues
g1
actionrosidl#301 Neeed to be able to generate msg and srv c and c++ code in anactionnamespaceactionfolder rosidl_typesupport_fastrtps#9 Allow fastrtps typesupport to generate service in action folderactionfolder rosidl_typesupport_opensplice#6 Allow opensplice typesupport to generate services in actionactionfolder rosidl_typesupport_connext#11 Allow connext typesupport to generate services in action folderg2
.actionfiles into intermediate representationg2.5
actionrosidl_python#13 Need to be able to generate msg and srv python code inactionmoduleg2.75
g3
g4
folder
g5
rclcan depend on in ROS 2 There is https://github.com/ros-geographic-info/unique_identifier/tree/ros2 , but it depends on boost/rospy/roscpp , will need something with fewer dependencies.g5.5
g6
g7
g8
g9
Likely not included in Crystal release
Good places for community contribution
ros2 actionPlaces for Action discussion
The text was updated successfully, but these errors were encountered: