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

Tidy up some contollers vs robotInterfaces deps details #7428

Merged

Conversation

jwnimmer-tri
Copy link
Collaborator

@jwnimmer-tri jwnimmer-tri commented Nov 9, 2017

Relates #6996. Relates #6464.

This came up in my 6996 feature branch. I thought that the circular dependency was causing trouble, so I fixed that in the first commit. It wasn't my real problem, but will end up being useful for 6464 anyway so I've included it here.

The real problem ended up being the last commit, so that's the import important part here.

The middle two commits are just small tidies that came up along the way.


This change is Reviewable

@jwnimmer-tri
Copy link
Collaborator Author

+@siyuanfeng-tri for feature review, please.
+@soonho-tri for platform review per schedule, please.

@jwnimmer-tri jwnimmer-tri force-pushed the build-robotInterfaces-deps branch 2 times, most recently from c1d4ae2 to 9eb15d0 Compare November 9, 2017 02:20
The move makes the dependency direction only robotInterfaces deps
on controllers, instead of circular.  The rename just comes for
free since I'm here anyway.
This works around Bazel issue 3115.
@jwnimmer-tri jwnimmer-tri force-pushed the build-robotInterfaces-deps branch from 9eb15d0 to 7e6f81e Compare November 9, 2017 03:01
@soonho-tri
Copy link
Member

:lgtm:


Reviewed 13 of 13 files at r1.
Review status: all files reviewed at latest revision, 1 unresolved discussion.


drake/systems/controllers/BUILD.bazel, line 120 at r1 (raw file):

    srcs = ["side.cc"],
    hdrs = ["side.h"],
    deps = [],

BTW, this line can be removed?


Comments from Reviewable

@jwnimmer-tri
Copy link
Collaborator Author

Review status: all files reviewed at latest revision, 1 unresolved discussion.


drake/systems/controllers/BUILD.bazel, line 120 at r1 (raw file):

Previously, soonho-tri (Soonho Kong) wrote…

BTW, this line can be removed?

OK I have been leaving it to highlight the lack of any deps.


Comments from Reviewable

@siyuanfeng-tri
Copy link
Contributor

:lgtm: thanks Jeremy!


Reviewed 13 of 13 files at r1.
Review status: all files reviewed at latest revision, all discussions resolved.


Comments from Reviewable

@jwnimmer-tri jwnimmer-tri merged commit b7d494d into RobotLocomotion:master Nov 9, 2017
@jwnimmer-tri jwnimmer-tri deleted the build-robotInterfaces-deps branch November 9, 2017 12:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants