ros / common_msgs

Commonly used messages in ROS. Includes messages for actions (actionlib_msgs), diagnostics (diagnostic_msgs), geometric primitives (geometry_msgs), robot navigation (nav_msgs), and common sensors (sensor_msgs), such as laser range finders, cameras, point clouds.
http://wiki.ros.org/common_msgs
177 stars 191 forks source link

Organizing sensor messages by stamped and unstamped #143

Open kunaltyagi opened 5 years ago

kunaltyagi commented 5 years ago

Currently, all sensor messages have the stamp built in. This is the expected behavior. However, when the sensors drivers provided by OEM return data that's covering multiple messages, (and when using message filter is either not feasible or overkill) I felt the need for using Unstamped form of the message.

Basically, for new messages being proposed such as those in #101, can we have the following:

This allows combination of messages to be done for sensors.

AngularVelocityUnstamped ang_vel TemperatureUnstamped temp


As a followup, if we added code during generation, we can allow for explicit conversion (implicit might be bad for obv reasons) between messages which have a `data` or `value` field in them without having to specify the field such as

Int32 i32; AngularVelocityUnstamped ang_vel;

// current int val = i32.data; double ang = ang_val.value;

// proposed int val(i32); double ang(ang_vel);

This might help in reducing the number of indirections required to refer to the data member in some places.

explicit operator T() const { return value; } // OR explicit operator T() const { return data; }

tfoote commented 5 years ago

In the case that you have a device that outputs multiple different datatypes I would recommend against making a custom message of this sort. This breaks the abstraction of the individual messages and means that you will need to have custom logic for processing the data from that manufactures sensors.

For the abstraction the timestamp and frame_id of each reading is inherent to the measurement and is critical to being able to process and utilize the data contained in the message. Grouping the messages based on their physical/device association doesn't make sense for downstream consumers. In your example I could have a simple widget that displays the temperature, that knows how to render a Temperature.msg. If you publish that combined message the simple widget will have to know how to subscribe to the VelocityAndTemperature msg as well and then select the right subfields. The most common way to do this would be to put another node in the middle that would republish the standard submessage from the conglomerate so the widget would not need to be recompiled. This would be a large step backwards for standard messages to break apart the encapsulation.

If you have a aggregate message that has a semantic meaning it might make sense to create a custom message for your application with both contained. However if you do that I'd recommend using the standard message datatypes so that they can be used with libraries that leverage the standard message datatypes. (Maybe the temperature widget has a c++ API you can use directly not just the topic.)

And lastly just because readings come from the same sensor does it mean that they have the same timestamp or frame_id. If the device is large, the position of the angular velocity sensor might be different than the temperature sensor. For this example it might not be meaningful, but consider a stereo camera with an IMU built in. This is very common for visual odometry, but their different origins are important. And also in that case the IMU usually has a much higher datarate than the cameras so the timestamp too is potentially different.

kunaltyagi commented 5 years ago

Thanks for your thoughts. You make perfectly reasonable points:

  1. Sensors might (+ve: stereo-camera + imu, -ve: (imu + temp + pressure)) have different timestamps for different types of data
  2. It's better (usually) to use standard messages rather than custom messages. This allows downstream applications to be sensor-agnostic

I have a few questions regarding good choices of message structure based on usage pattern. I hope you can help me identify the best practices for messages:

  1. Would your ideal Imu.msg be the following:
    # This is ideal Imu.msg
    # There is a Header field in each one of this
    AngularVelocity ang_vel
    LinearAcceleration lin_acc
    MagnetometerOrientation orientation
  2. In-case the timestamps are the same and downstream user wants to take advantage of that, would a stamped and unstamped msg format help the user?
    # Ideal Imu.msg part 2
    # As the name suggests, no message has Header except this one
    Header header
    AngularVelocityUnstamped ang_vel
    LinearAccelerationUnstamped lin_acc
    MagnetometerOrientationUnstamped orientation
  3. It might make sense to break version (1) up into its standard parts and simply send the 3 different Stamped messages. Would you prefer this to be a better design over the prev 2? What about the current Imu.msg?
  4. Debugging messages are a valid use-case where I find myself reaching for sensor-speciic messages. Having to whiff up a few lines of msg files and CMakeLists is not a big trouble but a friction point. What do you suggest in such cases? IMO having the unstamped versions would ease the development but this is not normal development that we are talking about.
  5. Does message_filter do something such as informing the publishers so that the overhead is minimized especially on resource constrained devices? This might matter if we have a real-time flavor of ROS. I only noticed client side code and may have missed it. This ties up to the fact that even while using image_transport with message_filters, I sometime have to use approximate timestamp policy which shows some overhead from publishing side as well as lack of special communication between image_transport like publishers and message_filters.
  6. Due to the std::vector in message_filters, there's almost no way those can be used on bare-metal devices. This would force using combined messages for usage with rosserial. What's your suggestion in such cases?
tfoote commented 5 years ago

As proposed in #101 there would not be an aggregate message which is following your point 3.

I think that you're coming at this from the wrong point of view. Your limiting factor for using the parallel channels is that the message filter implementation is too large/complex for your use case on bare metal. But I would suggest in general that the design goals of the message_filters is inappropriate for running on resource constrained environments like bare metal. It's designed to stack everything up in memory and hold everything until arbitrary conditions are met. This is not what you need for using and processing IMU messages on a small microcontroller. So a much simpler logic could be used specifically to merge and synchronize the messages that you need. And in fact having smaller messages will be better in bare metal environments because you won't have to get any duplicate information if the data sources are coming in at different frequencies and they are smaller to allocate on the microcontroller.

Debugging messages are a valid use-case where I find myself reaching for sensor-speciic messages. Having to whiff up a few lines of msg files and CMakeLists is not a big trouble but a friction point. What do you suggest in such cases? IMO having the unstamped versions would ease the development but this is not normal development that we are talking about.

I'm not sure how the header gets in the way of debugging, you can always ignore them.

Does message_filter do something such as informing the publishers so that the overhead is minimized especially on resource constrained devices? This might matter if we have a real-time flavor of ROS. I only noticed client side code and may have missed it. This ties up to the fact that even while using image_transport with message_filters, I sometime have to use approximate timestamp policy which shows some overhead from publishing side as well as lack of special communication between image_transport like publishers and message_filters.

message_filters are only on the receive side. If your publishers are not coordinated then you will likely need an approximate time policy as the probability of two publishers hitting the exact same timestamp are negligable. That's not really something that the message_filter can change in any way.

Due to the std::vector in message_filters, there's almost no way those can be used on bare-metal devices. This would force using combined messages for usage with rosserial. What's your suggestion in such cases?

Requiring the combined message is not really a corollary to the message_filters implementation being too bing. There's nothing magic in the message_filters and instead of changing the datatypes that will effect the entire system, just implementing a simpler version of the synchronization logic on your target than the message_filters implementation that is too big, is a simpler solution.