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

SCRIPTS: switch bag variable to image_rect_color, add fps alias #250

Merged
merged 1 commit into from
May 18, 2017

Conversation

kev-the-dev
Copy link
Contributor

We found with Navigator that it's more convientent to have image_rect_color in bags since all perception nodes (should) use it instead of image_raw. This makes it easier to test without having to run image_proc, and you could pottentialy still unrectify back to image_raw if needed. So this switches the bag variables to use image_rect_color.

I also added the alias I was using at testing today to see if the cameras were up. It just runs rostopic hz on the camera topics so we can know which ones are up and what their framerates are. Useful for diagnostics.

Copy link
Member

@DSsoto DSsoto left a comment

Choose a reason for hiding this comment

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

This is fine, but I'd prefer if you added my request.


# Topic variables that can be used from the bag command
export bag_front_left_cam="/camera/front/left/camera_info /camera/front/left/image_raw"
export bag_front_right_cam="/camera/front/right/camera_info /camera/front/right/image_raw"
export bag_front_left_cam="/camera/front/left/camera_info /camera/front/left/$CAM_SUFFIX"
Copy link
Member

@DSsoto DSsoto May 14, 2017

Choose a reason for hiding this comment

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

Can you add a cli option to bag to set CAM_SUFFIX? I'm fine with having image rect color as the default.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good idea, but that would be done in mil_common and probably best by @whispercoros

@sentree sentree self-assigned this May 15, 2017
Copy link
Member

@sentree sentree left a comment

Choose a reason for hiding this comment

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

I would appreciate it if you could copy the comment header for this file from mil_common/scripts/bag.sh as it has been changed.

@@ -12,12 +12,14 @@ export BAG_ALWAYS="/odom /absodom /clock /tf /tf_static"
# Define the directory that the bags will be stored in
export BAG_DIR=~/bags

# Which camera image type to use
export CAM_SUFFIX="image_rect_color"
Copy link
Member

Choose a reason for hiding this comment

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

Please note that a command line argument should be added to mil_common/scripts/bag.sh for the CAM_SUFFIX variable. This can be copied from the sections that address the BAG_ALWAYS or BAG_HOME variables. Also, I would prefer that it be called BAG_CAM_SUFFIX to keep with my naming convention in this file as well as to have the comment begin with the word "Define" (sorry to nit pick).

Copy link
Contributor Author

@kev-the-dev kev-the-dev May 16, 2017

Choose a reason for hiding this comment

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

If you want to add that flag for CAM_SUFFIX (or, better, arbitrary meta variables) it would be cool. Until than it was just a way to save me typing the same thing thrice and we shouldn't over think it. I'll change the variable name and comment.

Copy link
Member

Choose a reason for hiding this comment

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

Alright, I will add this to the list and merge this once it is done.

Copy link
Member

@sentree sentree left a comment

Choose a reason for hiding this comment

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

All of my comments were addressed.

@sentree sentree merged commit 0d7ddc3 into uf-mil:master May 18, 2017
@kev-the-dev kev-the-dev deleted the script branch May 23, 2017 20:43
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