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.

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

Fix cam name var #105

Merged
merged 5 commits into from
Nov 16, 2023
Merged

Conversation

marinagmoreira
Copy link
Member

the cam name wasn't being initialized after the changes that were made to integrate the camera class into the camera_model

Copy link
Contributor

@trey0 trey0 left a comment

Choose a reason for hiding this comment

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

Thank you!

@@ -69,7 +69,7 @@ class CameraView : public camera::CameraModel {
// f: far clip, maximum camera distance (m)
// n: near clip, minimum camera distance (m)
// cam_transform: transform from body->camera, useful for offline applications where tf is not available
CameraView(const camera::CameraParameters & params, const float f = 2.0, const float n = 0.19,
CameraView(const std::string, const camera::CameraParameters & params, const float f = 2.0, const float n = 0.19,
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
CameraView(const std::string, const camera::CameraParameters & params, const float f = 2.0, const float n = 0.19,
CameraView(const std::string& cam_name, const camera::CameraParameters & params, const float f = 2.0, const float n = 0.19,

(Usually better to pass std::string as ref to avoid unnecessary extra copy, also style consistency of including the argument name.)

@@ -34,10 +34,9 @@ namespace inspection {
the camera frame and the other way around. It automatically reads the camera parameters
from the config files based on the camera name, such that no setup is necessary.
*/
CameraView::CameraView(const camera::CameraParameters & params, const float f, const float n,
CameraView::CameraView(const std::string cam_name, const camera::CameraParameters& params, const float f, const float n,
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
CameraView::CameraView(const std::string cam_name, const camera::CameraParameters& params, const float f, const float n,
CameraView::CameraView(const std::string& cam_name, const camera::CameraParameters& params, const float f, const float n,

@@ -215,7 +215,7 @@ int main(int argc, char** argv) {
new geometry_msgs::Transform(msg_conversions::eigen_transform_to_ros_transform(transform_body_to_cam)));

camera::CameraParameters cam_params(&config, camera_name.c_str());
inspection::CameraView camera(cam_params, 2.0, 0.19, msg_pointer);
inspection::CameraView camera(camera_name.c_str(), cam_params, 2.0, 0.19, msg_pointer);
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
inspection::CameraView camera(camera_name.c_str(), cam_params, 2.0, 0.19, msg_pointer);
inspection::CameraView camera(camera_name, cam_params, 2.0, 0.19, msg_pointer);

(Converting to char * shouldn't be needed here unless I missed something.)

@marinagmoreira marinagmoreira merged commit 5010fc9 into nasa:develop Nov 16, 2023
5 checks passed
@marinagmoreira marinagmoreira deleted the fix_cam_name_var branch November 16, 2023 17:11
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.

2 participants