-
Notifications
You must be signed in to change notification settings - Fork 32
Merge push_anything_dev into push_anything #388
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
base: push_anything
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall looks good, though I'd like to review again after some changes I suggested are implemented.
While reviewing, I merged push_anything
into push_anything_merge
since there were some changes already in push_anything
that were still showing up here in the diff. After doing that, I tried ensuring the code still ran, but ran into issues with python scripts requiring packages I did not have installed. Can you do:
- Document somewhere what python packages are required for your python scripts.
- Ensure the code on this branch still runs, after my merge.
examples/sampling_c3/anything/parameters/sampling_c3_controller_params.yaml
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another round of comments. In addition to the inline comments, ensure all the URDFs/SDFs in this PR were autogenerated with the latest version of the generation scripts. That way, the first time you re-run the autogeneration script, it won't show changes to the file.
|
||
SDF_TEMPLATE = """<?xml version="1.0"?> | ||
<sdf version="1.7"> | ||
<model name="t_shape"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's fair -- at least remove the extra _
at the end of one of them so their names are consistent.
This change is