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

Make inheriting from simulation world possible #317

Merged
merged 3 commits into from
Apr 15, 2020

Conversation

ToniRV
Copy link
Contributor

@ToniRV ToniRV commented Mar 30, 2020

This just allows users to be able to inherit from the simulation world to build their own simulation worlds 🚀

@ethzasl-jenkins
Copy link

Can one of the admins verify this patch?

@ToniRV
Copy link
Contributor Author

ToniRV commented Apr 10, 2020

@alexmillane Hi Alex, would it be possible to merge this? Thank you!

@alexmillane
Copy link
Collaborator

Can one of the admins get this tested please :) @mfehr @helenol

Copy link
Contributor

@mfehr mfehr left a comment

Choose a reason for hiding this comment

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

Since you turned the sim world member into a pointer, you should check if it is a nullptr everywhere it is dereferenced, or at least once per function that is using it.

@mfehr
Copy link
Contributor

mfehr commented Apr 11, 2020

test this please

@ToniRV
Copy link
Contributor Author

ToniRV commented Apr 11, 2020

Since you turned the sim world member into a pointer, you should check if it is a nullptr everywhere it is dereferenced, or at least once per function that is using it.

@mfehr Done! Thanks for the review Marius!

@mfehr
Copy link
Contributor

mfehr commented Apr 11, 2020

ok to test

Copy link
Contributor

@mfehr mfehr left a comment

Choose a reason for hiding this comment

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

lgtm
I was expecting the CHECK_NOTNULL(unique_ptr) to fail, I vaguely remember there being some issues with that, which is why we are using CHECK(unique_ptr) or CHECK(unique_ptr != nullptr) everywhere for smart pointers, also in the rest of voxblox. But when it didn't fail here I checked and it turns out this was fixed 3 years ago. Good to know :P

@ToniRV
Copy link
Contributor Author

ToniRV commented Apr 11, 2020

Yes! Although I think I did switch to CHECK(unique_ptr) in Kimera as well... bcs earlier versions of glog had this problem.

It's really up to you, I can change that to be consistent with voxblox, but, in principle, new users should not have issues.

@ToniRV
Copy link
Contributor Author

ToniRV commented Apr 15, 2020

Friendly ping @mfehr :)

@mfehr mfehr merged commit 5df0774 into ethz-asl:master Apr 15, 2020
@ToniRV ToniRV deleted the feature/inherit_from_simulation_world branch April 15, 2020 23:29
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.

None yet

4 participants