Skip to content

Update compressed_image_transport to ros2#26

Merged
mjcarroll merged 5 commits intoros2from
ros2-devel
Aug 21, 2018
Merged

Update compressed_image_transport to ros2#26
mjcarroll merged 5 commits intoros2from
ros2-devel

Conversation

@mjcarroll
Copy link
Copy Markdown
Contributor

@mjcarroll mjcarroll commented Aug 14, 2018

Basic port
TODO:

  • Use ros2 parameters to set encoding
  • Use parameter events to replace dynamic reconfigure.

@j-rivero
Copy link
Copy Markdown

This PR should be reviews used changes in ros-perception/image_common#84, right?

@mjcarroll
Copy link
Copy Markdown
Contributor Author

Correct, it depends on the image_transport port to ROS2.

{

void CompressedPublisher::advertiseImpl(ros::NodeHandle &nh, const std::string &base_topic, uint32_t queue_size,
const image_transport::SubscriberStatusCallback &user_connect_cb,
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are not porting the SubscriberStatusCallback to ROS2. Looking for the similiar feature I found a answer.ros.org by William explaining in March that the feature is not implemented yet. I assume that it is still unimplemented.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct, it remains unimplemented.

float cRatio = (float)(cv_ptr->image.rows * cv_ptr->image.cols * cv_ptr->image.elemSize())
/ (float)compressed.data.size();
ROS_DEBUG("Compressed Image Transport - Codec: jpg, Compression Ratio: 1:%.2f (%lu bytes)", cRatio, compressed.data.size());
//ROS_DEBUG("Compressed Image Transport - Codec: jpg, Compression Ratio: 1:%.2f (%lu bytes)", cRatio, compressed.data.size());
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

RCUTILS_LOG_DEBUG here and in the rest of removals?

else
{
ROS_ERROR("cv::imencode (png) failed on input image");
//ROS_ERROR("cv::imencode (png) failed on input image");
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

RCUTILS_LOG_ERROR here and in the rest of removals?

void CompressedPublisher::advertiseImpl(ros::NodeHandle &nh, const std::string &base_topic, uint32_t queue_size,
const image_transport::SubscriberStatusCallback &user_connect_cb,
const image_transport::SubscriberStatusCallback &user_disconnect_cb,
const ros::VoidPtr &tracked_object, bool latch)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tracked_object was removed from the arguments, just checking that it was on propose.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct, it was removed in the image_transport as well.

}


void CompressedSubscriber::configCb(Config& config, uint32_t level)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sure if it could help the future implementation of parameters to leave the configCb code as a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I made a config struct that mirrors the config, I figure we can always jump back in the git log to get this.

@j-rivero
Copy link
Copy Markdown

j-rivero commented Aug 16, 2018

Latest changes look good to me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants