orocos_ros_integration patch

Hi Ruben (and all orocos-dev),
I created a short patch for orocos_ros_integration ros package (in
attachment; there are two patches, one to apply to ocl-svn into ocl package
-I'm using ocl 1.10 - and the second one to apply to orocos_ros_integration
package).

If everything works fine, I expected this behavior:
1)If there's nothing inside the struct of a component (.xml file), nothing
happen;
2)If there is <simple name="CreateAllRosNode"
type="boolean"><value>1<value><simple>; value true, I expected that for
all my port of that component will be created an anticlone with the same
numer of ports.
3)Instead, if there is a struct like this one:
<struct name="CreateRosNode" type="PropertyBag">
<simple type="string"><value>object_point_cloud<value><simple>
<simple type="string"><value>WritePort2<value><simple>
<struct>

I expected that the ports created and connected from anticlone to orocos
component are 2 (using the name found in the value )

4)If there are both (CreateAllRosNode and CreateRosNode), I expected a
Warning message and all ports will be created (as step 2).

This should resolve the problem that I had: If I apply
orocos_ros_integration package to more than one component, launched by the
same ros-deployer, I will have one (or more) components to comunicate with
ROS (subscribing topic), and one (or more) componets to comunicate with ROS
(advising topic). In this case, the orocos component "interface" from/to ROS
will have one (or more) dataports that ROS shouldn't see, to comunicate just
with "internal" Orocos components. In the original version, all ports of
"orocos component interface" are "connected" to ROS, also ports that I don't
need. This could cause two different problems:
1)It could happen that for some dataports there isn't a datatype support: it
means an error or the necessity to add datatype support that we didn't need
for our goal.
2)When I try to kill the ros-deployer node, it's not possible to kill that
node in clean way (because it means try to disconnects dataport that don't
exist).

Probably (Honestly, I'm pretty sure!!) that it's possible to implement my
changes in a better way, so If you find mistakes or better solution, please
let's me know.
For my goal (MTTL interface with ROS) that solution should be ok (It seems
work), but it could be nice for a user have the possibility to set only the
port that he/she want.

Also it's possibile that my patch introduce other changes not necessary
(like in manifest.xml file, where I put dependences about packages that I'm
working on them, or in RosToolkit.cpp/RosTypeConversion.hpp, where I added
support to datatypes that I need).

Best Regards,
Enea Scioni

AttachmentSize
orocos_ros_integration_patch.tar.gz9.21 KB

orocos_ros_integration patch

On Wednesday 16 June 2010 11:09:39 Enea Scioni wrote:
> Hi Ruben (and all orocos-dev),
> I created a short patch for orocos_ros_integration ros package (in
> attachment; there are two patches, one to apply to ocl-svn into ocl package
> -I'm using ocl 1.10 - and the second one to apply to orocos_ros_integration
> package).

I can not apply the OCL patch fully on ocl trunk because it contains ROS
specific parts. They must be moved to the ros-deployer code (for example,
inserting the valid_names) and you probably need a virtual function declared
in DeploymentComponent that is called when an unknown valid_name is
encountered, which is then passed to the subclass (ie RosDeploymentComponent).
Such a patch I could accept. I also accept the --path option to deployer, and
the cmake changes (rpath and misc fixes.) I do not maintain the ros-side code.
I also don't maintain the motion control code, Ruben needs to take care of
that.

Actually, this is pretty good work for a first time patch :-).

>
> If everything works fine, I expected this behavior:
> 1)If there's nothing inside the struct of a component (.xml file), nothing
> happen;
> 2)If there is <simple name="CreateAllRosNode"
> type="boolean"><value>1<value><simple>; value true, I expected that for
> all my port of that component will be created an anticlone with the same
> numer of ports.

ok. Although a bit confusing naame. Isn't 'RosNodeCreateAllTopics' better ?

> 3)Instead, if there is a struct like this one:
> <struct name="CreateRosNode" type="PropertyBag">
> <simple type="string"><value>object_point_cloud<value><simple>
> <simple type="string"><value>WritePort2<value><simple>
> <struct>

Same here: 'RosNodeCreateTopics'.

>
> I expected that the ports created and connected from anticlone to orocos
> component are 2 (using the name found in the value )

ok.

>
> 4)If there are both (CreateAllRosNode and CreateRosNode), I expected a
> Warning message and all ports will be created (as step 2).

ok.

>
> This should resolve the problem that I had: If I apply
> orocos_ros_integration package to more than one component, launched by the
> same ros-deployer, I will have one (or more) components to comunicate with
> ROS (subscribing topic), and one (or more) componets to comunicate with ROS
> (advising topic). In this case, the orocos component "interface" from/to
> ROS will have one (or more) dataports that ROS shouldn't see, to
> comunicate just with "internal" Orocos components. In the original
> version, all ports of "orocos component interface" are "connected" to ROS,
> also ports that I don't need. This could cause two different problems:
> 1)It could happen that for some dataports there isn't a datatype support:
> it means an error or the necessity to add datatype support that we didn't
> need for our goal.
> 2)When I try to kill the ros-deployer node, it's not possible to kill that
> node in clean way (because it means try to disconnects dataport that don't
> exist).
>
> Probably (Honestly, I'm pretty sure!!) that it's possible to implement my
> changes in a better way, so If you find mistakes or better solution, please
> let's me know.

Since you are the primary user, I'm sure you'll be the first to notice any
mistakes :-)

> For my goal (MTTL interface with ROS) that solution should be ok (It seems
> work), but it could be nice for a user have the possibility to set only the
> port that he/she want.
>
> Also it's possibile that my patch introduce other changes not necessary
> (like in manifest.xml file, where I put dependences about packages that I'm
> working on them, or in RosToolkit.cpp/RosTypeConversion.hpp, where I added
> support to datatypes that I need).

In 2.x, these will be automatically generated anyway. The idea is that Orocos
components will communicate Ros message structs directly on their data ports,
ie as if the ros message was a user defined struct. This means that there is a
header-only dependency on these ros message structs for these orocos
components, but I guess this is no different than depending on another library.
You can still communicate ros messages between Orocos components without ROS
too.

So if my suggestions make it into your patches, I'm sure we can integrate them
on the OCL mainline.

Peter