search

ros2 / rmw_iceoryx

rmw implementation for iceoryx
Apache License 2.0
153 stars 27 forks source link

force lower case for ros package name #12

Closed jeising closed 4 years ago

jeising commented 4 years ago

To be able to use iceoryxs native applications with ros 2 based applications the package name defined from service and event name must be valid for ros 2 as package name. Forcing lowercase in the package name only partially fixes this issue. As far as I know there are changes planned regarding types in iceoryx, so I would propose this fix, till we can focus on the new type handling.

Karsten1987 commented 4 years ago

Can you please rebase this PR?

jeising commented 4 years ago

@Karsten1987: Should be fine now

jeising commented 4 years ago

@Karsten1987: Is there anything missing and keeps us from merging this PR?

jeising commented 4 years ago

@michael-poehnl & @Karsten1987: Please review this change :)

Karsten1987 commented 4 years ago

As far as I know there are changes planned regarding types in iceoryx, so I would propose this fix, till we can focus on the new type handling.

@michael-poehnl What's the status on this? Is there something new coming to iceoryx?

jeising commented 4 years ago

@Karsten1987: Thanks for your comments! As you can see e.g. in the tests I am not done with the changes yet. I'll ping you once the PR is ready again. I feel "push early" is good to avoid code loss.

jsqu4re commented 4 years ago

@Karsten1987 & @michael-poehnl: Feel free to review the code, thx!

budrus commented 4 years ago

.

As far as I know there are changes planned regarding types in iceoryx, so I would propose this fix, till we can focus on the new type handling.

@michael-poehnl What's the status on this? Is there something new coming to iceoryx?

I'm not sure what @jeising has in mind. There is no type support topic in the near future planned. Frameworks like ROS2 or eCAL solve this on top of iceoryx. So far iceoryx is data agnostic

jeising commented 4 years ago

I'm not sure what @jeising has in mind. There is no type support topic in the near future planned. Frameworks like ROS2 or eCAL solve this on top of iceoryx. So far iceoryx is data agnostic

Hm .. I thought I read it somewhere Sorry for the confusion

budrus commented 4 years ago

We have to rebase because of #14 The passing of std::string to iceoryx methods which expect our fixed string changed. (Explicit truncate must be stated). See here https://github.com/ros2/rmw_iceoryx/pull/14/files

Maybe this also affects your test.