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.

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

[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

🎉 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}

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.

//************************************************************************
// 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

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
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 <d.stonier@gmail.com>
francocipollone
francocipollone previously approved these changes Jan 5, 2024
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.

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

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?

Signed-off-by: Franco Cipollone <franco.c@ekumenlabs.com>
@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
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

2 participants