Add resolve_filepath for url scheme such that package://, model://#2
Add resolve_filepath for url scheme such that package://, model://#2iory wants to merge 1 commit intommatl:masterfrom
Conversation
|
So sorry for the delayed reply, just saw this! Nice PR, I'll test it and merge it soon. Best, |
|
btw, I've tested this (at least for |
|
Hi @mmatl I also wanted this functionality so was looking into why this was failing... there's a few issues I found, but it seems the tests won't pass even on master? the |
mcevoyandy
left a comment
mcevoyandy
left a comment
There was a problem hiding this comment.
Hi @iory nice addition! It works well for viewing a normal URDF, but breaks the relative structure... with the two changes I suggested I can view both, but the tests still fail because of bad dae files. I'm not sure if @mmatl can address that or not. I definitely will keep using this repo and this PR so it would be good to see it merged!
| file_path = resolve_filepath(base_path, file_path) | ||
| fn = file_path | ||
| if not os.path.isabs(file_path): | ||
| fn = os.path.join(base_path, file_path) |
There was a problem hiding this comment.
when given a relative path, this gives the wrong path. resolve_filepath returns base_path + file_path, then this if makes fn = base_path + base_path + file_path. Should this be os.path.join(os.getcwd(), file_path) instead?
| if os.path.exists(resolved_filepath): | ||
| return resolved_filepath | ||
| dirname = os.path.dirname(dirname) | ||
| return False |
There was a problem hiding this comment.
in the tests, this will return false, then in get_filename, os.path.isabs fails because it's trying to evaluate False
| The resolved filepath -- just the normal ``file_path`` if it was an | ||
| absolute path, otherwise that path joined to ``base_path``. | ||
| """ | ||
| file_path = resolve_filepath(base_path, file_path) |
There was a problem hiding this comment.
with comment above, I think you need a check to see if resolve_filepath returns False
| The resolved filepath -- just the normal ``file_path`` if it was an | ||
| absolute path, otherwise that path joined to ``base_path``. | ||
| """ | ||
| file_path = resolve_filepath(base_path, file_path) |
There was a problem hiding this comment.
| file_path = resolve_filepath(base_path, file_path) | |
| resolved_file_path = resolve_filepath(base_path, file_path) | |
| if resolved_file_path != False: | |
| file_path = resolved_file_path |
| file_path = resolve_filepath(base_path, file_path) | ||
| fn = file_path | ||
| if not os.path.isabs(file_path): | ||
| fn = os.path.join(base_path, file_path) |
There was a problem hiding this comment.
| fn = os.path.join(base_path, file_path) | |
| fn = os.path.join(os.getcwd(), file_path) |
4 participants
This PR supports URL scheme such that
package://,model://.With this PR, we can load urdf contains following XML tag.