{"action":"created","issue":{"url":"https://api.github.com/repos/RobotLocomotion/drake/issues/8855","repository_url":"https://api.github.com/repos/RobotLocomotion/drake","labels_url":"https://api.github.com/repos/RobotLocomotion/drake/issues/8855/labels{/name}","comments_url":"https://api.github.com/repos/RobotLocomotion/drake/issues/8855/comments","events_url":"https://api.github.com/repos/RobotLocomotion/drake/issues/8855/events","html_url":"https://github.com/RobotLocomotion/drake/pull/8855","id":325485548,"number":8855,"title":"multibody sdf parsing: Load meshes from relative paths","user":{"login":"sammy-tri","id":17596504,"avatar_url":"https://avatars3.githubusercontent.com/u/17596504?v=4","gravatar_id":"","url":"https://api.github.com/users/sammy-tri","html_url":"https://github.com/sammy-tri","followers_url":"https://api.github.com/users/sammy-tri/followers","following_url":"https://api.github.com/users/sammy-tri/following{/other_user}","gists_url":"https://api.github.com/users/sammy-tri/gists{/gist_id}","starred_url":"https://api.github.com/users/sammy-tri/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/sammy-tri/subscriptions","organizations_url":"https://api.github.com/users/sammy-tri/orgs","repos_url":"https://api.github.com/users/sammy-tri/repos","events_url":"https://api.github.com/users/sammy-tri/events{/privacy}","received_events_url":"https://api.github.com/users/sammy-tri/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":{"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},"assignees":[{"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},{"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":6,"created_at":"2018-05-22T22:27:18Z","updated_at":"2018-05-23T17:45:50Z","closed_at":null,"author_association":"CONTRIBUTOR","pull_request":{"url":"https://api.github.com/repos/RobotLocomotion/drake/pulls/8855","html_url":"https://github.com/RobotLocomotion/drake/pull/8855","diff_url":"https://github.com/RobotLocomotion/drake/pull/8855.diff","patch_url":"https://github.com/RobotLocomotion/drake/pull/8855.patch"},"body":"\n\n<!-- Reviewable:start -->\nThis change is [<img src=\"https://reviewable.io/review_button.svg\" height=\"34\" align=\"absmiddle\" alt=\"Reviewable\"/>](https://reviewable.io/reviews/robotlocomotion/drake/8855)\n<!-- Reviewable:end -->\n"},"comment":{"url":"https://api.github.com/repos/RobotLocomotion/drake/issues/comments/391437676","html_url":"https://github.com/RobotLocomotion/drake/pull/8855#issuecomment-391437676","issue_url":"https://api.github.com/repos/RobotLocomotion/drake/issues/8855","id":391437676,"user":{"login":"sammy-tri","id":17596504,"avatar_url":"https://avatars3.githubusercontent.com/u/17596504?v=4","gravatar_id":"","url":"https://api.github.com/users/sammy-tri","html_url":"https://github.com/sammy-tri","followers_url":"https://api.github.com/users/sammy-tri/followers","following_url":"https://api.github.com/users/sammy-tri/following{/other_user}","gists_url":"https://api.github.com/users/sammy-tri/gists{/gist_id}","starred_url":"https://api.github.com/users/sammy-tri/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/sammy-tri/subscriptions","organizations_url":"https://api.github.com/users/sammy-tri/orgs","repos_url":"https://api.github.com/users/sammy-tri/repos","events_url":"https://api.github.com/users/sammy-tri/events{/privacy}","received_events_url":"https://api.github.com/users/sammy-tri/received_events","type":"User","site_admin":false},"created_at":"2018-05-23T17:45:50Z","updated_at":"2018-05-23T17:45:50Z","author_association":"CONTRIBUTOR","body":"\n\n\n\nReview status: 10 of 14 files reviewed at latest revision, 16 unresolved discussions.\n\n---\n\n*[multibody/multibody_tree/parsing/scene_graph_parser_detail.h, line 32 at r4](https://reviewable.io/reviews/robotlocomotion/drake/8855#-LDChGqNBfGY5FzA3y8C:-LDD4jGM0k7BXQFxRMRy:b-896fix) ([raw file](https://github.com/robotlocomotion/drake/blob/06918c804743a5b70177a6110dd115ad992b64fb/multibody/multibody_tree/parsing/scene_graph_parser_detail.h#L32)):*\n<details><summary><i>Previously, SeanCurtis-TRI (Sean Curtis) wrote…</i></summary><blockquote>\n\nBTW This seems overly specific; it could just as easily be applied to geometries for collisions. And, quite possibly, other URIs? However, if you want to defer this until we're actually *parsing* collision meshes, then I'm open to that. But how about a TODO?\n</blockquote></details>\n\nDone.\n\n---\n\n*[multibody/multibody_tree/parsing/scene_graph_parser_detail.cc, line 215 at r4](https://reviewable.io/reviews/robotlocomotion/drake/8855#-LDCkC0B3yLdoE5u9oPS:-LDD56h03p7Ynx268IPR:b-3unish) ([raw file](https://github.com/robotlocomotion/drake/blob/06918c804743a5b70177a6110dd115ad992b64fb/multibody/multibody_tree/parsing/scene_graph_parser_detail.cc#L215)):*\n<details><summary><i>Previously, SeanCurtis-TRI (Sean Curtis) wrote…</i></summary><blockquote>\n\nBTW I this formatting won't survive clang format. Same for `resolved_name =`.  And I prefer the clang-formatted version.\n</blockquote></details>\n\nI don't, but as long as our tool is still more hyperactive than the style guide requires, fine.\n\n---\n\n*[multibody/parsers/BUILD.bazel, line 61 at r3](https://reviewable.io/reviews/robotlocomotion/drake/8855#-LDCbCGg4jnlfLyrPBVb:-LDD0R3nCQZCjbRP04i-:b-9xaj7l) ([raw file](https://github.com/robotlocomotion/drake/blob/4a8056aee5ca2ebcb2fa554b19438bfdca3c759a/multibody/parsers/BUILD.bazel#L61)):*\n<details><summary><i>Previously, SeanCurtis-TRI (Sean Curtis) wrote…</i></summary><blockquote>\n\nBTW Doesn't seem that `find_files` depends on tinyxml2 and tinydir.\n</blockquote></details>\n\n`package_map.cc` uses both.\n\n---\n\n*[multibody/parsers/parser_path_utils.h, line 10 at r3](https://reviewable.io/reviews/robotlocomotion/drake/8855#-LDCcAFdBwVsZ8oa0cW3:-LDD12kY43S2x96xC-pz:b-896fix) ([raw file](https://github.com/robotlocomotion/drake/blob/4a8056aee5ca2ebcb2fa554b19438bfdca3c759a/multibody/parsers/parser_path_utils.h#L10)):*\n<details><summary><i>Previously, SeanCurtis-TRI (Sean Curtis) wrote…</i></summary><blockquote>\n\nBTW I'm not in love with the fact that this file has both \"full path\" and \"fully-qualified name\" which, as near as I can tell, are the same thing.\n</blockquote></details>\n\nDone.\n\n---\n\n*[multibody/parsers/parser_path_utils.cc, line 48 at r3](https://reviewable.io/reviews/robotlocomotion/drake/8855#-LDCceH461JZoqNFFWbU:-LDD18Vt-gLfr1CdqmVc:b-896fix) ([raw file](https://github.com/robotlocomotion/drake/blob/4a8056aee5ca2ebcb2fa554b19438bfdca3c759a/multibody/parsers/parser_path_utils.cc#L48)):*\n<details><summary><i>Previously, SeanCurtis-TRI (Sean Curtis) wrote…</i></summary><blockquote>\n\nBTW missing `@` on `@p`? Probably just drop it since this isn't doxygen.\n</blockquote></details>\n\nDone.\n\n---\n\n*[multibody/parsers/parser_path_utils.cc, line 66 at r3](https://reviewable.io/reviews/robotlocomotion/drake/8855#-LDCej0eEebpINP0TKwf:-LDD1MRSBWfde-oRF090:br3nuk2) ([raw file](https://github.com/robotlocomotion/drake/blob/4a8056aee5ca2ebcb2fa554b19438bfdca3c759a/multibody/parsers/parser_path_utils.cc#L66)):*\n<details><summary><i>Previously, SeanCurtis-TRI (Sean Curtis) wrote…</i></summary><blockquote>\n\nBTW What happens if filename is an absolute path to begin with?\n</blockquote></details>\n\nIt seems to do what the doxygen in the header states and prepends `root_dir` to it.  This would break absolute paths, though I'm not sure why you'd have an absolute path in an SDF/URDF.   Does it seem worth changing?\n\n---\n\n*[multibody/parsers/parser_path_utils.cc, line 68 at r3](https://reviewable.io/reviews/robotlocomotion/drake/8855#-LDCeCun0_7v6uPPR31P:-LDD2Kkf3pQTnVBxfH0T:b-niim19) ([raw file](https://github.com/robotlocomotion/drake/blob/4a8056aee5ca2ebcb2fa554b19438bfdca3c759a/multibody/parsers/parser_path_utils.cc#L68)):*\n<details><summary><i>Previously, SeanCurtis-TRI (Sean Curtis) wrote…</i></summary><blockquote>\n\nBTW Related to the removed \"Mesh\" warning, `mesh_filename_spruce` should probably be `qualified_filename`.\n</blockquote></details>\n\nI went with `full_filename_spruce` since I dropped the \"qualified\" language due to the defect raised regarding the doxygen.\n\n---\n\n*[multibody/parsers/parser_path_utils.cc, line 90 at r3](https://reviewable.io/reviews/robotlocomotion/drake/8855#-LDCdn-e9hv1M_pVTSRh:-LDD2lMt7utuVNgYqZjx:b-896fix) ([raw file](https://github.com/robotlocomotion/drake/blob/4a8056aee5ca2ebcb2fa554b19438bfdca3c759a/multibody/parsers/parser_path_utils.cc#L90)):*\n<details><summary><i>Previously, SeanCurtis-TRI (Sean Curtis) wrote…</i></summary><blockquote>\n\nBTW Isn't `split_raw` exactly `split_filename`.\n</blockquote></details>\n\nDone.\n\n---\n\n*[multibody/parsers/parser_path_utils.cc, line 108 at r3](https://reviewable.io/reviews/robotlocomotion/drake/8855#-LDCeQV0E7H04X7eVCHl:-LDD3Jo84x1q7krDlhuM:b-896fix) ([raw file](https://github.com/robotlocomotion/drake/blob/4a8056aee5ca2ebcb2fa554b19438bfdca3c759a/multibody/parsers/parser_path_utils.cc#L108)):*\n<details><summary><i>Previously, SeanCurtis-TRI (Sean Curtis) wrote…</i></summary><blockquote>\n\nBTW `GetFullPath` uses a *different* mechanism to determine if a path is absolute (`substr`). It'd be nice if a single mechanism were used in the span of a single file.\n</blockquote></details>\n\nDone.\n\n---\n\n*[multibody/parsers/parser_path_utils.cc, line 119 at r3](https://reviewable.io/reviews/robotlocomotion/drake/8855#-LDCes7UB5maPQl1qAmE:-LDD3KDl1sO1qRw_Agwe:bsqlz3b) ([raw file](https://github.com/robotlocomotion/drake/blob/4a8056aee5ca2ebcb2fa554b19438bfdca3c759a/multibody/parsers/parser_path_utils.cc#L119)):*\n<details><summary><i>Previously, SeanCurtis-TRI (Sean Curtis) wrote…</i></summary><blockquote>\n\nBTW Rather than deleting *all* of the warnings, I'd take these two warnings and merge them into one message:\n\n\"The file '{}' resolved to '{}' and could not be found.\"  Or some such thing.\n</blockquote></details>\n\nAll callers to this function were displaying an error message and throwing an exception already, so having a warning here about being ignored by drake was actually a bit of a lie about what was actually a fatal error.  Put back a more general warning.\n\n---\n\n*[multibody/parsers/parser_path_utils.cc, line 62 at r4](https://reviewable.io/reviews/robotlocomotion/drake/8855#-LDCh6OV0BbRjgKzulOb:-LDD1BVE-IHOBJHUp9FR:b-896fix) ([raw file](https://github.com/robotlocomotion/drake/blob/06918c804743a5b70177a6110dd115ad992b64fb/multibody/parsers/parser_path_utils.cc#L62)):*\n<details><summary><i>Previously, SeanCurtis-TRI (Sean Curtis) wrote…</i></summary><blockquote>\n\nBTW Wouldn't this be:\n\n>  // namespace\n</blockquote></details>\n\nDone.\n\n---\n\n\n*Comments from [Reviewable](https://reviewable.io/reviews/robotlocomotion/drake/8855)*\n<!-- Sent from Reviewable.io -->\n"}}
{"actor":{"display_login":"sammy-tri"}}