-
Notifications
You must be signed in to change notification settings - Fork 123
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
Adding stop and start functionality to the fixed lag smoother #362
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Reduce code duplication between the stop and reset services
- Can you implement this same functionality in the batch optimizer? The batch optimizer now has a reset service (as of a day ago), so it should be easy to follow this same pattern.
{ | ||
stop(); | ||
return true; | ||
} | ||
|
||
bool FixedLagSmoother::startServiceCallback(std_srvs::Empty::Request&, std_srvs::Empty::Response&) | ||
{ | ||
start(); | ||
return true; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The request and response are not used. I assume this is only a stub for future filling in.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not a stub, there's just no information currently needed for a start or stop, the same as how the reset callback also doesn't use the request or response fields.
@@ -497,6 +529,7 @@ void FixedLagSmoother::transactionCallback( | |||
// ...check if we should | |||
if (sensor_models_.at(sensor_name).ignition) | |||
{ | |||
ROS_INFO_STREAM("Ignition occured"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have no opinions here whatsoever. Just asking questions:
Do you like this new log line printed all the time? Or would DEBUG be a better fit? Should the same message be implemented in the batch optimizer?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Whoops, left this in by accident. I was testing... something, I don't remember what, and forgot to remove this. Doing that now
This PR adds the ability to stop the fixed lag smoother and restart it at a later time, to keep it from consuming CPU when it's not needed.
This PR makes the assumption that whenever you restart the optimizer, any data that was previously held in the optimizer is no longer valid, and should be removed.