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

Changes to bot config files and some minor fixes #373

Merged
merged 15 commits into from
Dec 23, 2021

Conversation

oleg-alexandrov
Copy link
Contributor

This commit changes all the robot config files to reflect a more generic approach of defining relationships between camera transforms. All the camera transforms and timestamp offsets are now relative to nav cam. This is only relevant for ISAAC.

I also put a fix in reading config files so it does not complain when a table is read instead of number for the distortion parameters. For that, I added to config_reader a function which can peek and see if a field is a number or a table.

I modified the calibration doc to note that nowadays pico_proxy is launched automatically (as long as drivers are launched), but kept the original logic for how to launch it separately if necessary.

Then I fixed a bug in iss.rviz. The sci cam topic should be compressed, otherwise it can't be displayed.

scicam_to_hazcam_transform = transform(vec3(0.0070938664, 0.061024816, -0.11184277), quat4(0.065639737, -0.031237584, 0.99720414, -0.017307023)),
nav_cam_to_haz_cam_transform = transform(vec3(0.071421481, -0.00030319673, 0.0018058249), quat4(-0.0030431141, 0.0092646368, 0.99993195, 0.0064039206)),
nav_cam_to_sci_cam_transform = transform(vec3(-0.076526224, 0.011869553, 0.045409155), quat4(-0.0035414316, 0.0048089362, -0.0071515076, 0.99995659)),
nav_cam_to_haz_cam_timestamp_offset = -0.02,
Copy link
Member

Choose a reason for hiding this comment

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

I don't think the geometry section is the best place for time offsets. Maybe in the calibrations section would be better? Or if you have another idea.

@oleg-alexandrov
Copy link
Contributor Author

Strictly speaking the camera transforms are also calibrated quantities, just like the offsets. But I see your point. The offsets would probably belong under each camera's intrinsics, further down the config file, in the robot_calibrations subsection.

I'll implement this tomorrow if there's no further comments.

@bcoltin
Copy link
Member

bcoltin commented Dec 22, 2021

Strictly speaking the camera transforms are also calibrated quantities, just like the offsets. But I see your point. The offsets would probably belong under each camera's intrinsics, further down the config file, in the robot_calibrations subsection.

I'll implement this tomorrow if there's no further comments.

Otherwise it looks good to me! Thanks!

@oleg-alexandrov
Copy link
Contributor Author

I moved the offsets to the robot_calibrations namespace. I checked that the bot files for bumble, honey, queen, and bsharp load well and work as expected, and that the rest follow the same logic (a script was used to consistently convert their format).

I will merge this once the tests pass.

@oleg-alexandrov oleg-alexandrov merged commit 56be397 into nasa:develop Dec 23, 2021
luisa-mao pushed a commit to luisa-mao/astrobee-1 that referenced this pull request Jul 11, 2023
* Check if a given value is a number before reading it as a real

The fix was trickier than that

* Update bumble and bsharp2 to new transforms

* Update the placeholder transforms for the rest of the bots

* iss.rivz: Sci cam is compressed

* astrobee readme: Point out to simulator page

* build_map: Print the robot name upfront, to avoid mistakes

* calibrate: Mention that pico_proxy is now started automatically

* simulation: wording fix

* Minor indentation fix in robot config files

* iss_viz.rviz: Sci cam is compressed

* build_map: Make the linter happy

* Move timestamp offsets from geometry to robot calibration namespace

* Minor typo fix

* Wipe file added by mistake
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