Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[applications] xodr_to_obj #259

Merged
merged 2 commits into from
Jan 5, 2024
Merged

[applications] xodr_to_obj #259

merged 2 commits into from
Jan 5, 2024

Conversation

stonier
Copy link
Collaborator

@stonier stonier commented Dec 23, 2023 •
edited
Loading

🎉 New feature

Adds a command line utility for dumping .obj/.mtl files from an xodr.

Also:

  • Bugfixes the return values of all applications to 0 on success
  • Adds an xodr that is currently unsupported (spiral geometries) <- I was trying to get a .obj of this road network

Summary

Not fancy, but has the basics for getting a .obj file.

Test it

bazel run //:xodr_to_obj -- --xodr-file=${PWD}/resources/Town07.xodr --out-dir=${PWD}

@stonier stonier mentioned this pull request Dec 23, 2023
Closed
francocipollone
francocipollone requested changes Dec 26, 2023
View reviewed changes
Copy link
Collaborator

@francocipollone francocipollone left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the app. 🚀
ptal at the comments

MODULE.bazel Outdated
@@ -16,3 +16,5 @@ bazel_dep(name = "maliput", version = "1.1.1")
bazel_dep(name = "tinyxml2", version = "9.0.0")

bazel_dep(name = "googletest", version = "1.14.0", dev_dependency = True)

local_path_override(module_name="maliput", path="../maliput")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Worth for local use case but probably we don't want to add this local_path_override statement to the repo, right?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aye, fixed.

.devcontainer/bazel-zen/devcontainer.json Outdated
//************************************************************************
// 1. Add the following to MODULE.bazel
// local_path_override(module_name="maliput", path="../maliput")
// 2. Mount maliputr by uncommenting and patching the path in the section below
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit:

Suggested change
// 2. Mount maliputr by uncommenting and patching the path in the section below
// 2. Mount maliput by uncommenting and patching the path in the section below

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

resources/SmallTownRoads.xodr Outdated
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does it make sense to defer adding this map until we support Spiral Geometries? #260

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't mind having it there as a test-driven stub for #260, but I'll take it out of here and create a separate PR for that.

@stonier stonier force-pushed the stonier/xodr_to_obj branch 2 times, most recently from 4df8f6a to f6157a1 Compare January 5, 2024 13:46
@stonier
[applications] xodr_to_obj
2e16456 
Also bugfixes the return values of all applications to 0 on success
and adds an xodr that is currently unsupported (spiral geometries).

Signed-off-by: Daniel Stonier <[email protected]>
francocipollone
francocipollone previously approved these changes Jan 5, 2024
View reviewed changes
Copy link
Collaborator

@francocipollone francocipollone left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

CI failing due to clang-format.

There is bash script tool under tools folder that you can run to automatically format the repo.

./src/maliput_malidrive/tools/reformat_code.sh

@stonier
Copy link
Collaborator Author

stonier commented Jan 5, 2024 •
edited
Loading

There is bash script tool under tools folder that you can run to automatically format the repo.

Dang, no ament in my bazel devcontainer. Can you run that for me and push a commit to this branch?

Q: Would it be useful for you to have a ROS devcontainer?

@francocipollone
Makes clang format happy.
f486b55 
Signed-off-by: Franco Cipollone <[email protected]>
@francocipollone
Copy link
Collaborator

Q: Would it be useful for you to have a ROS devcontainer?

I would say yes for quick development setup, however as we work with several repositories and in a colcon workspace not sure if will be that straightforward. In Bazel you can work directly on the repo and add as many local_override as you want. So it matches just right.

@francocipollone francocipollone merged commit cd42b4f into main Jan 5, 2024
4 checks passed
@francocipollone francocipollone deleted the stonier/xodr_to_obj branch January 5, 2024 14:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@francocipollone francocipollone francocipollone approved these changes

Assignees
No one assigned
Labels
None yet
Projects
Maliput - Core development
Status: ✅ Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@stonier @francocipollone