tud-phi / ros2-elastica

ROS2 package implementing Elastica
0 stars 0 forks source link

Fix RodState msg type #23

Closed mstoelzle closed 2 years ago

mstoelzle commented 2 years ago

I find the structure of the RodState.msg way too inflexible. I propose the following structure:

In the RodStates.msg, you report the state of all rods (e.g. pneumatically actuated segments). Accordingly, the message is structured like:

std_msgs/Header header 

# number of rods
int num_rods

# all the rod states
RodState[] rod_states

Then, we have another message type RodState.msg, where we report the state of all elements within that rod (so for example within the order of 51 elements as currently configured in the code):

std_msgs/Header header 

# number of element for the rod
int num_elements

# the poses of all elements in the Cosserat rod
geometry_msgs/PoseStamped[] poses

It's always the better choice to use a standard message format such as PoseStamped whenever possible. Also, this gives us the flexible to use however many rods we want to use

RUFFY-369 commented 2 years ago

Hi @mstoelzle , yeah the rod state msg structure was too inflexible for the time being and also too loosen. I have the refactored the code and structured new one. I was delaying to use the above approach for the future code after changing the actuation model. From now on I will maintain the standard messages in the new message structure beforehand. Thanks I've pushed the required changes in the recent commits. Please review them.

mstoelzle commented 2 years ago

I think it looks good in general. Two small changes I would do for consistency:

  1. Rename velocity to velocities in RodState.msg: https://github.com/tud-cor-sr/ros2-elastica/blob/main/elastica_msgs/msg/RodState.msg#L10
  2. Rename rods_state to rod_states in RodsState.msg: https://github.com/tud-cor-sr/ros2-elastica/blob/main/elastica_msgs/msg/RodsState.msg#L9
RUFFY-369 commented 2 years ago

Hi @mstoelzle , pushed the minor changes.