Skip to content

allow pause on saving clouds - #14

Open
planthaber wants to merge 4 commits into
masterfrom
feature/pause
Open

allow pause on saving clouds#14
planthaber wants to merge 4 commits into
masterfrom
feature/pause

Conversation

@planthaber

Copy link
Copy Markdown
Member

This allows to call (disable) in a storage implementation, so that e.g. point clouds are not actually saved, the contraint is still created, but the cloud is not saved. On enable() the initial guess is trusted and mapping can go on.


Measurement::Ptr MeasurementStorage::get(const boost::uuids::uuid& uuid)
{
return mMeasurements.at(uuid);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I don't agree with this change. When you use get(), you should catch and handle std::out_of_range, not be forced to check the returned pointer.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

We can change that, imo there is not much difference.

Currently the behavior is similar to of std::map::get().

void MeasurementStorage::add(Measurement::Ptr measurement)
{
mMeasurements[measurement->getUniqueId()] = measurement;
if (enabled) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I am not sure this pausing function should go into the MeasurementStorage. It feels kinda wrong to call add() and then have this method silently not add the measurement because of some internal state. Wouldn't it be better to do this on a higher level (node/application) and just not call MeasurementStorage::add() when in paused mode?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yes, I also thought about this, but it requires a lot of changes in the other slma3d classes, as there still should run a icp and a vertex should be created, adding it here was the least intrusive way of having this funcionality.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

We could add a pause function to the graph class, but that info was to be passed down to all the implementations that use add() on the storage.

// throw BadMeasurementType();
// is there was an emptyx cloud, use the guess
Covariance<6> covariance = Covariance<6>::Identity() * mCovarianceScale;
return Constraint::Ptr(new SE3Constraint(mName, guess, covariance.inverse()));

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Adding the guess (which is just the current relative position of the two vertices) as a constraint is a bad idea. A PointcloudSensor can only create constraints between pointclouds. So similar as above, when an application wants to do something else, catch the BadMeasurement and do everything needed in the handler.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This code is only used, when the previous cloud was not saved ( slam was paused), so there is no cloud to compare to. The hope is that eventually a loop closure will catch this ans optimize correctly, as all the vertices in between are available

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