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

Port the "Add reset to batch optimizer" patch to ROS 2 Rolling #361

Open
wants to merge 2 commits into
base: rolling
Choose a base branch
from

Conversation

svwilliams
Copy link
Contributor

@svwilliams svwilliams commented Mar 21, 2024

Port the "Add reset to batch optimizer" patch (#360) to ROS 2 Rolling

//!< before being applied to the graph.
std::mutex combined_transaction_mutex_; //!< Synchronize access to the combined transaction
//!< across different threads
// Read-only after construction
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'm doing a bit of variable rearranging. This is not strictly required for this patch, but I wanted to mirror the FixedLagSmoother pattern in the BatchOptimizer.

@@ -131,6 +131,11 @@ TEST_F(FixedLagIgnitionFixture, SetInitialState)
pose_msg1.pose.covariance[35] = 1.0;
relative_pose_publisher->publish(pose_msg1);

/// @todo(swilliams) There is a timing or message queuing issue. This sleep "solves"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This unrelated unit test was failing on my machine. It appears to be a timing or message queue issue. The Pose2D sensor is only receiving the second published messages with a timestamp of 3.0s. The previous message with a timestamp of 2.0s is never processed. As a quick fix, I just added a delay. But this seems like a problem that should be fixable by changing a message queue size somewhere.

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.

3 participants