Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed crashes due to unhandeled exception in infrared_id_compensation code #24

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Fixed crashes due to unhandeled exception in infrared_id_compensation code #24

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
57adb60
fix crash during converting corrupt received image data to cv image
Sep 2, 2021
4748669
remove extra line
Sep 2, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 11 additions & 6 deletions src/simulator_processing/infrared_id_compensation.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,14 +54,19 @@ bool InfraredIdCompensation::setupFromRos(const ros::NodeHandle& nh,
}

void InfraredIdCompensation::imageCallback(const sensor_msgs::ImagePtr& msg) {
cv_bridge::CvImagePtr img = cv_bridge::toCvCopy(msg, msg->encoding);
for (int v = 0; v < img->image.rows; v++) {
for (int u = 0; u < img->image.cols; u++) {
img->image.at<uchar>(v, u) =
infrared_compensation_[img->image.at<uchar>(v, u)];
try {
Copy link
Contributor

Choose a reason for hiding this comment

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

Prefer specific checks to try blocks where possible. E.g. if the issue is that the messages are sometimes empty you can directly check for that. If it fails for various reasons then try-catch is fine. The entire image conversion loop does not need to be in the try block I think? The only thing that could go wrong here is that the data is out of range, which you can statically check for when loading the infrared corrections (i.e. that all values between 0-255 are covered)

cv_bridge::CvImagePtr img = cv_bridge::toCvCopy(msg, msg->encoding);
for (int v = 0; v < img->image.rows; v++) {
for (int u = 0; u < img->image.cols; u++) {
img->image.at<uchar>(v, u) =
infrared_compensation_[img->image.at<uchar>(v, u)];
}
}
pub_.publish(img->toImageMsg());
} catch (const cv_bridge::Exception& e) {
std::cerr << "Could not convert image to CvImage\n";
Copy link
Contributor

Choose a reason for hiding this comment

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

Better to stay consistent with logging (i.e. we use glog throughout the project). Consider replacing with more informative text, e.g. LOG(WARNING) << "Could not convert image for infrared_id_compensation, skipping frame (" << e.what() << ").";

std::cout << e.what();
}
pub_.publish(img->toImageMsg());
}

} // namespace unreal_airsim::simulator_processor