{"action":"created","issue":{"url":"https://api.github.com/repos/RobotLocomotion/drake/issues/8145","repository_url":"https://api.github.com/repos/RobotLocomotion/drake","labels_url":"https://api.github.com/repos/RobotLocomotion/drake/issues/8145/labels{/name}","comments_url":"https://api.github.com/repos/RobotLocomotion/drake/issues/8145/comments","events_url":"https://api.github.com/repos/RobotLocomotion/drake/issues/8145/events","html_url":"https://github.com/RobotLocomotion/drake/pull/8145","id":299498154,"number":8145,"title":"Adds the machinery for MBP to talk with GS.","user":{"login":"amcastro-tri","id":17601461,"avatar_url":"https://avatars3.githubusercontent.com/u/17601461?v=4","gravatar_id":"","url":"https://api.github.com/users/amcastro-tri","html_url":"https://github.com/amcastro-tri","followers_url":"https://api.github.com/users/amcastro-tri/followers","following_url":"https://api.github.com/users/amcastro-tri/following{/other_user}","gists_url":"https://api.github.com/users/amcastro-tri/gists{/gist_id}","starred_url":"https://api.github.com/users/amcastro-tri/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/amcastro-tri/subscriptions","organizations_url":"https://api.github.com/users/amcastro-tri/orgs","repos_url":"https://api.github.com/users/amcastro-tri/repos","events_url":"https://api.github.com/users/amcastro-tri/events{/privacy}","received_events_url":"https://api.github.com/users/amcastro-tri/received_events","type":"User","site_admin":false},"labels":[{"id":470891318,"url":"https://api.github.com/repos/RobotLocomotion/drake/labels/status:%20curate%20commits%20before%20merging","name":"status: curate commits before merging","color":"d93f0b","default":false},{"id":351649295,"url":"https://api.github.com/repos/RobotLocomotion/drake/labels/team:%20dynamics","name":"team: dynamics","color":"0052cc","default":false}],"state":"open","locked":false,"assignee":{"login":"SeanCurtis-TRI","id":22159799,"avatar_url":"https://avatars3.githubusercontent.com/u/22159799?v=4","gravatar_id":"","url":"https://api.github.com/users/SeanCurtis-TRI","html_url":"https://github.com/SeanCurtis-TRI","followers_url":"https://api.github.com/users/SeanCurtis-TRI/followers","following_url":"https://api.github.com/users/SeanCurtis-TRI/following{/other_user}","gists_url":"https://api.github.com/users/SeanCurtis-TRI/gists{/gist_id}","starred_url":"https://api.github.com/users/SeanCurtis-TRI/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/SeanCurtis-TRI/subscriptions","organizations_url":"https://api.github.com/users/SeanCurtis-TRI/orgs","repos_url":"https://api.github.com/users/SeanCurtis-TRI/repos","events_url":"https://api.github.com/users/SeanCurtis-TRI/events{/privacy}","received_events_url":"https://api.github.com/users/SeanCurtis-TRI/received_events","type":"User","site_admin":false},"assignees":[{"login":"jwnimmer-tri","id":17596505,"avatar_url":"https://avatars3.githubusercontent.com/u/17596505?v=4","gravatar_id":"","url":"https://api.github.com/users/jwnimmer-tri","html_url":"https://github.com/jwnimmer-tri","followers_url":"https://api.github.com/users/jwnimmer-tri/followers","following_url":"https://api.github.com/users/jwnimmer-tri/following{/other_user}","gists_url":"https://api.github.com/users/jwnimmer-tri/gists{/gist_id}","starred_url":"https://api.github.com/users/jwnimmer-tri/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/jwnimmer-tri/subscriptions","organizations_url":"https://api.github.com/users/jwnimmer-tri/orgs","repos_url":"https://api.github.com/users/jwnimmer-tri/repos","events_url":"https://api.github.com/users/jwnimmer-tri/events{/privacy}","received_events_url":"https://api.github.com/users/jwnimmer-tri/received_events","type":"User","site_admin":false},{"login":"SeanCurtis-TRI","id":22159799,"avatar_url":"https://avatars3.githubusercontent.com/u/22159799?v=4","gravatar_id":"","url":"https://api.github.com/users/SeanCurtis-TRI","html_url":"https://github.com/SeanCurtis-TRI","followers_url":"https://api.github.com/users/SeanCurtis-TRI/followers","following_url":"https://api.github.com/users/SeanCurtis-TRI/following{/other_user}","gists_url":"https://api.github.com/users/SeanCurtis-TRI/gists{/gist_id}","starred_url":"https://api.github.com/users/SeanCurtis-TRI/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/SeanCurtis-TRI/subscriptions","organizations_url":"https://api.github.com/users/SeanCurtis-TRI/orgs","repos_url":"https://api.github.com/users/SeanCurtis-TRI/repos","events_url":"https://api.github.com/users/SeanCurtis-TRI/events{/privacy}","received_events_url":"https://api.github.com/users/SeanCurtis-TRI/received_events","type":"User","site_admin":false}],"milestone":null,"comments":45,"created_at":"2018-02-22T20:32:45Z","updated_at":"2018-03-04T17:18:35Z","closed_at":null,"author_association":"CONTRIBUTOR","pull_request":{"url":"https://api.github.com/repos/RobotLocomotion/drake/pulls/8145","html_url":"https://github.com/RobotLocomotion/drake/pull/8145","diff_url":"https://github.com/RobotLocomotion/drake/pull/8145.diff","patch_url":"https://github.com/RobotLocomotion/drake/pull/8145.patch"},"body":" - Implements sugar MBP calls to register geometry.\r\n - Updates MakeAcrobotPlant to register geometry for visualization.\r\n - Updates passive simulation of the acrobot to use MBP.\r\n - Implements unit test to verify registration correctness.\n\n<!-- Reviewable:start -->\n---\nThis change is [<img src=\"https://reviewable.io/review_button.svg\" height=\"34\" align=\"absmiddle\" alt=\"Reviewable\"/>](https://reviewable.io/reviews/robotlocomotion/drake/8145)\n<!-- Reviewable:end -->\n"},"comment":{"url":"https://api.github.com/repos/RobotLocomotion/drake/issues/comments/370246369","html_url":"https://github.com/RobotLocomotion/drake/pull/8145#issuecomment-370246369","issue_url":"https://api.github.com/repos/RobotLocomotion/drake/issues/8145","id":370246369,"user":{"login":"jwnimmer-tri","id":17596505,"avatar_url":"https://avatars3.githubusercontent.com/u/17596505?v=4","gravatar_id":"","url":"https://api.github.com/users/jwnimmer-tri","html_url":"https://github.com/jwnimmer-tri","followers_url":"https://api.github.com/users/jwnimmer-tri/followers","following_url":"https://api.github.com/users/jwnimmer-tri/following{/other_user}","gists_url":"https://api.github.com/users/jwnimmer-tri/gists{/gist_id}","starred_url":"https://api.github.com/users/jwnimmer-tri/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/jwnimmer-tri/subscriptions","organizations_url":"https://api.github.com/users/jwnimmer-tri/orgs","repos_url":"https://api.github.com/users/jwnimmer-tri/repos","events_url":"https://api.github.com/users/jwnimmer-tri/events{/privacy}","received_events_url":"https://api.github.com/users/jwnimmer-tri/received_events","type":"User","site_admin":false},"created_at":"2018-03-04T17:18:35Z","updated_at":"2018-03-04T17:18:35Z","author_association":"COLLABORATOR","body":"\n\n\n\nReview status: all files reviewed at latest revision, 7 unresolved discussions.\n\n---\n\n*[multibody/multibody_tree/multibody_plant/multibody_plant.cc, line 265 at r7](https://reviewable.io:443/reviews/robotlocomotion/drake/8145#-L6YZy7O3iKuQRpEk6yM:-L6m096hwzNNbkW_SoOl:b-34u6mc) ([raw file](https://github.com/robotlocomotion/drake/blob/b1d8f9dadc5f53425a154f58a158fd14556ac3c0/multibody/multibody_tree/multibody_plant/multibody_plant.cc#L265)):*\n<details><summary><i>Previously, amcastro-tri (Alejandro Castro) wrote…</i></summary><blockquote>\n\nDone.\n</blockquote></details>\n\nYou can't count on source_id being right either.\n\n---\n\n*[multibody/multibody_tree/multibody_plant/multibody_plant.cc, line 64 at r9](https://reviewable.io:443/reviews/robotlocomotion/drake/8145#-L6d2Cu4IQDItUFOp713:-L6m-9qbQWpxnr4ksO4A:b-k6e2k5) ([raw file](https://github.com/robotlocomotion/drake/blob/7f8fa81033a8725e76ec8d02adefb4e4c0c4ea19/multibody/multibody_tree/multibody_plant/multibody_plant.cc#L64)):*\n<details><summary><i>Previously, SeanCurtis-TRI (Sean Curtis) wrote…</i></summary><blockquote>\n\nBTW Would it make sense to register under this plants `get_name()` as *that* is already guaranteed to be unique?\n</blockquote></details>\n\nGSystem::RegisterSource documentation does not require that the name is unique.  I see that the GState method it calls has such a requirement, though.  (1) The GS docs should stop being lies (2) requiring unique names seems overly burdensome -- descriptive user-supplied names are not, and should not, be used as identifiers.  That's what, well, identifiers are for.\n\nUntil that's fixed, passing the empty string here seems best, since that's the only way that the plant can meet its obligations -- the plant's own name isn't finalized until after diagram finalization, so the `get_name()` suggestion is still brittle.\n\n---\n\n*[multibody/multibody_tree/multibody_plant/multibody_plant.cc, line 104 at r9](https://reviewable.io:443/reviews/robotlocomotion/drake/8145#-L6d30bQCRZDXpO5jyL9:-L6m-oGcya0kxtFniJzD:b8xdx7h) ([raw file](https://github.com/robotlocomotion/drake/blob/7f8fa81033a8725e76ec8d02adefb4e4c0c4ea19/multibody/multibody_tree/multibody_plant/multibody_plant.cc#L104)):*\n<details><summary><i>Previously, SeanCurtis-TRI (Sean Curtis) wrote…</i></summary><blockquote>\n\nBTW This method (and `RegisterAnchoredGeometry` are private methods which only get invoked by `RegisterVisualGeometry()`. These guards have already been performed there. However, these functions have explicitly included a *subset* of guards. So, that means: \n\na) The guards *aren't* necessary and should be removed from these methods (and they prerequisites on calls to these documented in the header), or\nb) They should get *all* the guards in case they are intended to be invoked in any other public context in which the guards cannot be relied upon.\n\n</blockquote></details>\n\nIn the case of (a), where we're just being extra careful, another solution is to change them to DRAKE_ASSERT -- asserts are for _double_-checking (private contracts); throws are for public contract checking.\n\n---\n\n\n*Comments from [Reviewable](https://reviewable.io:443/reviews/robotlocomotion/drake/8145)*\n<!-- Sent from Reviewable.io -->\n"}}
{"actor":{"display_login":"jwnimmer-tri"}}