Skip to content

Updated monolithic demo readme to mention precompiled drake-visualizer#6362

Merged
soonho-tri merged 9 commits intoRobotLocomotion:masterfrom
naveenoid:pr/monolithic_demo_README_fix
Jun 30, 2017
Merged

Updated monolithic demo readme to mention precompiled drake-visualizer#6362
soonho-tri merged 9 commits intoRobotLocomotion:masterfrom
naveenoid:pr/monolithic_demo_README_fix

Conversation

@naveenoid
Copy link
Copy Markdown
Contributor

@naveenoid naveenoid commented Jun 16, 2017

Addresses #6354


This change is Reviewable

@naveenoid naveenoid requested a review from RussTedrake June 16, 2017 19:47
@naveenoid
Copy link
Copy Markdown
Contributor Author

+@RussTedrake for feature and platform review. (Its a straightforward fix).


Review status: 0 of 1 files reviewed at latest revision, all discussions resolved, some commit checks pending.


Comments from Reviewable

@RussTedrake
Copy link
Copy Markdown
Contributor

Review status: 0 of 1 files reviewed at latest revision, 3 unresolved discussions.


drake/examples/kuka_iiwa_arm/dev/monolithic_pick_and_place/README.md, line 19 at r1 (raw file):

$ cd drake-distro
$ bazel build //tools:drake_visualizer && bazel-build //drake/examples/kuka_iiwa_arm/dev/monolithic_pick_and_place:monolithic_pick_and_place_demo

bazel-build?

perhaps just
bazel build //tools:drake_visualizer //drake/examples/kuka_iiwa_arm/dev/monolithic_pick_and_place:monolithic_pick_and_place_demo


drake/examples/kuka_iiwa_arm/dev/monolithic_pick_and_place/README.md, line 28 at r1 (raw file):

$ bazel-bin/tools/drake-visualizer&

that doesn't work for me (on mac)


drake/examples/kuka_iiwa_arm/dev/monolithic_pick_and_place/README.md, line 36 at r1 (raw file):

$ ./bazel-bin/drake/examples/kuka_iiwa_arm/dev/monolithic_pick_and_place/

or should we prefer recommending bazel run?


Comments from Reviewable

@jwnimmer-tri
Copy link
Copy Markdown
Collaborator

drake/examples/kuka_iiwa_arm/dev/monolithic_pick_and_place/README.md, line 28 at r1 (raw file):

Previously, RussTedrake (Russ Tedrake) wrote…

that doesn't work for me (on mac)

FYI Mac still lacks some version of the #6319 fix that happened on linux.


Comments from Reviewable

@naveenoid
Copy link
Copy Markdown
Contributor Author

Review status: 0 of 3 files reviewed at latest revision, 3 unresolved discussions, some commit checks failed.


drake/examples/kuka_iiwa_arm/dev/monolithic_pick_and_place/README.md, line 19 at r1 (raw file):

Previously, RussTedrake (Russ Tedrake) wrote…

bazel-build?

perhaps just
bazel build //tools:drake_visualizer //drake/examples/kuka_iiwa_arm/dev/monolithic_pick_and_place:monolithic_pick_and_place_demo

Done.


drake/examples/kuka_iiwa_arm/dev/monolithic_pick_and_place/README.md, line 28 at r1 (raw file):

#6319
Added a comment about it not working on mac.


drake/examples/kuka_iiwa_arm/dev/monolithic_pick_and_place/README.md, line 36 at r1 (raw file):

Previously, RussTedrake (Russ Tedrake) wrote…

or should we prefer recommending bazel run?

Done. Swapped the recommendations.


Comments from Reviewable

@naveenoid naveenoid force-pushed the pr/monolithic_demo_README_fix branch from b1e4282 to cd4d829 Compare June 26, 2017 19:32
@RussTedrake
Copy link
Copy Markdown
Contributor

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


drake/examples/kuka_iiwa_arm/dev/monolithic_pick_and_place/README.md, line 19 at r1 (raw file):

Previously, naveenoid (Naveen Kuppuswamy) wrote…

Done.

not done? you don't need the && bazel build ... bazel can take multiple targets as multiple arguments.


Comments from Reviewable

@naveenoid
Copy link
Copy Markdown
Contributor Author

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


drake/examples/kuka_iiwa_arm/dev/monolithic_pick_and_place/README.md, line 19 at r1 (raw file):

Previously, RussTedrake (Russ Tedrake) wrote…

not done? you don't need the && bazel build ... bazel can take multiple targets as multiple arguments.

Done.


Comments from Reviewable

@RussTedrake
Copy link
Copy Markdown
Contributor

:lgtm:


Review status: 0 of 1 files reviewed at latest revision, all discussions resolved, some commit checks failed.


Comments from Reviewable

@naveenoid
Copy link
Copy Markdown
Contributor Author

+@soonho-tri for platform review.


Review status: 0 of 1 files reviewed at latest revision, all discussions resolved, some commit checks failed.


Comments from Reviewable

@soonho-tri
Copy link
Copy Markdown
Member

@drake-jenkins-bot linux-xenial-clang-ninja-experimental-open-source please


Review status: 0 of 1 files reviewed at latest revision, all discussions resolved, some commit checks failed.


Comments from Reviewable

@soonho-tri
Copy link
Copy Markdown
Member

Review status: 0 of 1 files reviewed at latest revision, 2 unresolved discussions.


drake/examples/kuka_iiwa_arm/dev/monolithic_pick_and_place/README.md, line 28 at r2 (raw file):

bazel-bin/tools/drake-visualizer

It should be drake-visualizer => drake_visualizer.


drake/examples/kuka_iiwa_arm/dev/monolithic_pick_and_place/README.md, line 42 at r2 (raw file):

bazel run :monolithic_pick_and_place_demo --config snopt -- --box_choice=1

Is --box_choice option still valid? I've rebased this PR over the current master and follow the instructions, but I get:

ERROR: unknown command line flag 'box_choice'

Comments from Reviewable

@soonho-tri
Copy link
Copy Markdown
Member

Reviewed 1 of 1 files at r2.
Review status: all files reviewed at latest revision, 2 unresolved discussions.


Comments from Reviewable

@jwnimmer-tri
Copy link
Copy Markdown
Collaborator

This is in dev, you don't need platform review, eh?

@naveenoid
Copy link
Copy Markdown
Contributor Author

Right! But good catch by soonho anyways. I will wait for his LGTM nonetheless.


Review status: 0 of 1 files reviewed at latest revision, 2 unresolved discussions.


drake/examples/kuka_iiwa_arm/dev/monolithic_pick_and_place/README.md, line 28 at r2 (raw file):

Previously, soonho-tri (Soonho Kong) wrote…

bazel-bin/tools/drake-visualizer

It should be drake-visualizer => drake_visualizer.

Done.


drake/examples/kuka_iiwa_arm/dev/monolithic_pick_and_place/README.md, line 42 at r2 (raw file):

Previously, soonho-tri (Soonho Kong) wrote…

Is --box_choice option still valid? I've rebased this PR over the current master and follow the instructions, but I get:

ERROR: unknown command line flag 'box_choice'

Done.


Comments from Reviewable

@soonho-tri
Copy link
Copy Markdown
Member

:lgtm: (aside a BTW)


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


drake/examples/kuka_iiwa_arm/dev/monolithic_pick_and_place/README.md, line 55 at r3 (raw file):

Command line arguments can be passed for : box_choice an integer between 1

BTW, we still have box_choice here.


Comments from Reviewable

@soonho-tri
Copy link
Copy Markdown
Member

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


Comments from Reviewable

@naveenoid
Copy link
Copy Markdown
Contributor Author

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


drake/examples/kuka_iiwa_arm/dev/monolithic_pick_and_place/README.md, line 55 at r3 (raw file):

Previously, soonho-tri (Soonho Kong) wrote…

BTW, we still have box_choice here.

Done.


Comments from Reviewable

@soonho-tri
Copy link
Copy Markdown
Member

@drake-jenkins-bot linux-xenial-clang-ninja-experimental-open-source please


Review status: 0 of 1 files reviewed at latest revision, all discussions resolved, some commit checks failed.


Comments from Reviewable

@soonho-tri
Copy link
Copy Markdown
Member

Reviewed 1 of 1 files at r4.
Review status: all files reviewed at latest revision, all discussions resolved.


Comments from Reviewable

@soonho-tri soonho-tri merged commit 421382f into RobotLocomotion:master Jun 30, 2017
@naveenoid naveenoid deleted the pr/monolithic_demo_README_fix branch September 3, 2018 01:44
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.

4 participants