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

Add dynamic reconfigure #6

Merged
merged 2 commits into from
Jul 10, 2015
Merged

Add dynamic reconfigure #6

merged 2 commits into from
Jul 10, 2015

Conversation

efernandez
Copy link
Member

Add dynamic reconfigure (only) for calibration parameters.
This is useful to tune this calibration parameters on the fly.

RT-safe implementation achieved on the second commit.

Only supports the following parameters:
* wheel_separation_multiplier
* left_wheel_radius_multiplier
* right_wheel_radius_multiplier
* k_l
* k_r

This implementation is NOT real-time safe!
@efernandez
Copy link
Member Author

@paulbovbel

@efernandez efernandez force-pushed the dynamic_reconfigure branch from c3255cd to f45ed8d Compare July 10, 2015 16:12
@@ -298,6 +314,27 @@ namespace diff_drive_controller

void DiffDriveController::update(const ros::Time& time, const ros::Duration& period)
{
// UPDATE DYNAMIC PARAMS
// Retreive dynamic params:
DynamicParams dynamic_params = *(dynamic_params_.readFromRT());

Choose a reason for hiding this comment

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

This is going to update all these parameters on control cycle, which seems like a lot. Would it be safe to use a boolean flag across threads to notify of update?

Copy link
Member Author

Choose a reason for hiding this comment

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

The problem is that there's not write from RT methods supported: see this proposal.

I could add a comment with that link, and we can go back to this discussion when this goes upstream; this is fast enough anyway. What do you think?

Copy link
Member Author

Choose a reason for hiding this comment

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

Comment added. Merging...

@paulbovbel
Copy link

LGTM, as per prior discussion

@efernandez efernandez force-pushed the dynamic_reconfigure branch 2 times, most recently from 8595a5d to 03fabb0 Compare July 10, 2015 21:24
@efernandez efernandez force-pushed the dynamic_reconfigure branch from 03fabb0 to fb4b05e Compare July 10, 2015 21:25
efernandez pushed a commit that referenced this pull request Jul 10, 2015
@efernandez efernandez merged commit 6c259cb into indigo-devel Jul 10, 2015
@efernandez efernandez deleted the dynamic_reconfigure branch July 10, 2015 21:25
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