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.

Sign up for GitHub

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

Jump to bottom

controller_manager: Fix doSwitch execution point - jade #210

Merged
merged 1 commit into from
Nov 19, 2015
Merged

controller_manager: Fix doSwitch execution point - jade #210

merged 1 commit into from
Nov 19, 2015

Conversation

adolfo-rt
Copy link
Member

Same as #209, plus an extra commit that makes canSwitch(...) non-const

@adolfo-rt adolfo-rt mentioned this pull request Sep 4, 2015
Closed
@mathias-luedtke mathias-luedtke mentioned this pull request Oct 27, 2015
Closed
@adolfo-rt adolfo-rt changed the title Fix do switch jade controller_manager: Fix doSwitch execution point - jade Nov 13, 2015
@adolfo-rt adolfo-rt force-pushed the fix-do-switch-jade branch 2 times, most recently from 8e49e46 to b7eeceb Compare November 13, 2015 16:13
@adolfo-rt
Copy link
Member Author

@ipa-mdl ptal.

@adolfo-rt
Copy link
Member Author

Notice that this PR is deprecating canSwitch. It proposes to have prepareSwitch as the only alternative for preparing a mode switch in non real-time.

@adolfo-rt adolfo-rt mentioned this pull request Nov 13, 2015
Closed
@davetcoleman
Copy link
Member

+1

@mathias-luedtke
Copy link
Contributor

This PR rebased #218, this looks wrong.
The jade version should keep the same commit order as indigo.

I would suggest to supersede #210 and #218 by a PR that syncs all changes from indigo-devel to jade-devel (jade-devel...indigo-devel), and e6a109c should be applied on top of it.
(This would pull in b1a758c as well)

As an alternative the commits can be cherry-picked in the right order.

@mathias-luedtke
Copy link
Contributor

I have tested the indigo version with a prepareSwitch implementation successfully.
So the jade version should be fine. I can test it on monday.

@mathias-luedtke
Copy link
Contributor

I have applied the changes in robot_hw.h to indigo-devel and it works as expected :)

@adolfo-rt
controller_manager: Fix doSwitch execution point
ad68d85 
The doSwitch method needs to be executed in the update() method,  that is, in
the real-time path, which is where controller switching actually takes place.

It was previously done in the switchController callback, which is non real-time.
In this method controller switching is scheduled, but not actually executed.

This changeset fixes a bug in which hardware interface  modes could switch
before controllers, leading to undefined behavior.
adolfo-rt added a commit that referenced this pull request Nov 19, 2015
@adolfo-rt
Merge pull request #210 from ros-controls/fix-do-switch-jade
33f0bf7 
controller_manager: Fix doSwitch execution point - jade
@adolfo-rt adolfo-rt merged commit 33f0bf7 into jade-devel Nov 19, 2015
@adolfo-rt adolfo-rt deleted the fix-do-switch-jade branch November 19, 2015 08:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@adolfo-rt @davetcoleman @mathias-luedtke