search

ros-perception / vision_opencv

Apache License 2.0
536 stars 599 forks source link

Allow users to override encoding string in ROSCvMatContainer #505

Closed Yadunund closed 1 year ago

Yadunund commented 1 year ago

Fixes #504

The implementation here relies on using an std::optional<std::string> encoding_override argument which can be supplied during instantiation of ROSCVMatContainer. If provided, the value will be used, else default to the existing behavior of inferring the encoding from cv::Mat::type(). In order to use std::optional, I also had to bump the C++ version to C++17.

Also it looks like ROSCvMatContainer::get_sensor_msgs_msg_image_copy() has duplicate code from convert_to_ros_message(). I've left a TODO to explore reusing the latter within this function. Happy to implement it if the change is acceptable.

Signed-off-by: Yadunund yadunund@openrobotics.org

ijnek commented 1 year ago

@Yadunund thanks for raising an issue and the PR too. Could you create an issue as an alternative to a todo comment. The changes itself look good.

Yadunund commented 1 year ago

Thanks for the quick review. I've removed the todo comment and opened this ticket instead https://github.com/ros-perception/vision_opencv/issues/506

ijnek commented 1 year ago

Thanks for the contribution!