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

[RST-2128] fixed lag smoother reset #61

Merged
merged 5 commits into from Jun 14, 2019

Conversation

svwilliams
Copy link
Contributor

Added a "reset" service to the fixed-lag smoother. This reverts the smoother back to its original state, waiting for a transaction from the configured ignition sensors.

@svwilliams svwilliams requested a review from ayrton04 June 13, 2019 00:29
@@ -268,6 +279,28 @@ void FixedLagSmoother::processQueue(fuse_core::Transaction& transaction)
}
}

bool FixedLagSmoother::resetServiceCallback(std_srvs::Empty::Request& req, std_srvs::Empty::Response& res)
Copy link
Contributor

Choose a reason for hiding this comment

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

Neither req nor res are used, so it might be good to make that clear here and in the header:

bool FixedLagSmoother::resetServiceCallback(std_srvs::Empty::Request&, std_srvs::Empty::Response&)

{
ROS_WARN_STREAM("Optimization exceeded the configured duration by " <<
(optimization_complete - optimization_deadline) << "s");
std::lock_guard<std::mutex> lock(optimization_mutex_);
Copy link
Contributor

Choose a reason for hiding this comment

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

I assume this is necessary because the reset request will come from another thread.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Correct. Previously the graph_ was only modified by the optimizer thread. Now I am modifying the graph_ from the ROS callback thread as well.

std::lock_guard<std::mutex> lock(pending_transactions_mutex_);
pending_transactions_.clear();
}
// Clear the graph and marginal tracking states
Copy link
Contributor

Choose a reason for hiding this comment

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

And there's no weird state that we can get into from a transaction getting somehow inserted in between these two blocks? Just checking.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think so.
The ROS spinner is configured for a single thread. Since this is called from the ROS callback thread, no other ROS callbacks can fire while this is running. And thus new pending transactions cannot be added between blocks.

@svwilliams svwilliams merged commit 63227bf into devel Jun 14, 2019
@svwilliams svwilliams deleted the RST-2128-fixed-lag-smoother-reset branch June 14, 2019 14:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants