-
Notifications
You must be signed in to change notification settings - Fork 7
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
Add rudimentary stl support #64
base: main
Are you sure you want to change the base?
Conversation
Really cool, ill test it shortly 🎉
…On Thu, 28 Nov, 2024, 9:16 am Sebastian Jahr, ***@***.***> wrote:
@sjahr <https://github.com/sjahr> requested your review on: #64
<#64> Add rudimentary stl
support.
—
Reply to this email directly, view it on GitHub
<#64 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AFNFZJBTNCI3TKQ3T72SELT2C3NN5AVCNFSM6AAAAABSUUT4M6VHI2DSMVQWIX3LMV45UABCJFZXG5LFIV3GK3TUJZXXI2LGNFRWC5DJN5XDWMJVGQ3DENBZGU2DSMI>
.
You are receiving this because your review was requested.Message ID:
***@***.***>
|
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.
Cool!
""" | ||
for root, _, files in os.walk(directory): | ||
for file in files: | ||
if file.lower().endswith(".stl"): |
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.
Since you're already calling os.path.splitext()
later, I believe the second element of that will already give you the extension. So you don't need to additionally do an endswith()
@@ -13,8 +13,8 @@ repositories: | |||
version: main | |||
moveit_resources: | |||
type: git | |||
url: https://github.com/moveit/moveit_resources | |||
version: ros2 | |||
url: https://github.com/sjahr/moveit_resources |
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.
Why do you need a new branch of this?
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.
It has the .obj files already, so no need to run the script again
@@ -147,4 +147,12 @@ getPiecewisePolynomial(const ::robot_trajectory::RobotTrajectory& robot_trajecto | |||
void getRobotTrajectory(const ::drake::trajectories::Trajectory<double>& drake_trajectory, const double delta_t, | |||
const ::drake::multibody::MultibodyPlant<double>& plant, | |||
std::shared_ptr<::robot_trajectory::RobotTrajectory>& moveit_trajectory); | |||
|
|||
/** | |||
* @brief Converts all STL file paths in a string to OBJ file paths |
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.
Make it clear that it's in a URDF description string
|
||
### .stl support | ||
|
||
Unfortunately, Drake does not support `.stl` files (11/28/2024, see [drake#19408](https://github.com/RobotLocomotion/drake/issues/19408)). We're working around this by replacing the `.stl` files in the urdf string |
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.
One sentence per line would be preferable
|
||
``` | ||
./scripts/convert_stl_to_obj.py /PATH/TO/YOUR/MESH/DIR | ||
``` |
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.
Do you need to instruct people to rebuild the package after running the script? I assume the OBJ files won't make it into install space otherwise.
std::string replaceSTLWithOBJ(const std::string& input) | ||
{ | ||
std::string result = input; | ||
const std::string target = ".stl"; |
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.
Should .STL
(upper case) also be supported?
@@ -147,4 +147,12 @@ getPiecewisePolynomial(const ::robot_trajectory::RobotTrajectory& robot_trajecto | |||
void getRobotTrajectory(const ::drake::trajectories::Trajectory<double>& drake_trajectory, const double delta_t, | |||
const ::drake::multibody::MultibodyPlant<double>& plant, | |||
std::shared_ptr<::robot_trajectory::RobotTrajectory>& moveit_trajectory); | |||
|
|||
/** | |||
* @brief Converts all STL file paths in a string to OBJ file paths |
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.
* @brief Converts all STL file paths in a string to OBJ file paths | |
* @brief Converts all STL file paths in a URDF description string to OBJ file paths |
I wouldn't call it .stl support but at least everybody should be able to use moveit configs other than the panda with this 😅