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

Bindings using RoadNetwork unique_ptr are not working. #846

Closed
francocipollone opened this issue Jun 23, 2022 · 4 comments · Fixed by #848
Closed

Bindings using RoadNetwork unique_ptr are not working. #846

francocipollone opened this issue Jun 23, 2022 · 4 comments · Fixed by #848

Comments

@francocipollone
Copy link
Collaborator

francocipollone commented Jun 23, 2022

Summary

maliput_py stopped relying on custom pybind11 version. This seems to be causing a failure in delphyne bindings that are using std::unique_ptr<RoadNetwork>

m.def("create_malidrive_road_network_from_xodr", &delphyne::roads::CreateMalidriveRoadNetworkFromXodr,
"Load an full RoadNetwork based on malidrive backend", py::arg("name"), py::arg("file_path"),
py::arg("rule_registry_file_path"), py::arg("road_rulebook_file_path"), py::arg("traffic_light_book_path"),
py::arg("phase_ring_path"), py::arg("intersection_book_path"), py::arg("linear_tolerance") = 1e-3,
py::arg("angular_tolerance") = 1e-3);

Here delphyne::roads::CreateMalidriveRoadNetworkFromXodr returns a std::unique_ptr<RoadNetwork>. When using this binding this it is throwing an error because:

     File "/__w/delphyne_demos/delphyne_demos/maliput_ws/install/delphyne/lib/python3.8/site-packages/delphyne/behaviours/roads.py", line 146, in setup
        delphyne.roads.create_malidrive_road_network_from_xodr(
    TypeError: Unable to convert function return value to a Python type! The signature was
    	(name: str, file_path: str, rule_registry_file_path: str, road_rulebook_file_path: str, traffic_light_book_path: str, phase_ring_path: str, intersection_book_path: str, linear_tolerance: float = 0.001, angular_tolerance: float = 0.001) -> maliput::api::RoadNetwork

When maliput_py's bindings are compiled using the custom pybind11 version it works correctly.

Related

maliput/maliput_infrastructure#225

@agalbachicar
Copy link
Collaborator

Some ideas:

  • Move the delphyne python api that depends on maliput to not use maliput python bindings.
  • Build a service like API in delphyne to interact with maliput. Agents would make calls to the service instead of maliput pointers directly.

@francocipollone
Copy link
Collaborator Author

Some ideas:

  • Move the delphyne python api that depends on maliput to not use maliput python bindings.
  • Build a service like API in delphyne to interact with maliput. Agents would make calls to the service instead of maliput pointers directly.

I like the idea and I think that for how things are right now it's the tidier option.
What I found a bit bittersweet is that because of just using unique_ptr on already binded maliput type we can't rely directly on the maliput_py bindings which, not only is what they are for, but also delphyne could be seen as an example for the community on how using the bindings.

I have a workaround in this branch for this.
We could drop a real nice docstring explaining there, and once we move to Jammy, (technically we would be able to deal with this particular case), we could remove this.

Wdyt?

@agalbachicar
Copy link
Collaborator

I concur. Note my comment in a15cc30#r77070044 which I guess it should help to reduce the code maintenance in delphyne for this workaround.

The second part of the comment might not be as useful because of use cases though.

@francocipollone
Copy link
Collaborator Author

This issue hasn't been fixed, there was a case I missed and I think I'd mistakenly tried the solution out.

CI still failing, not because of using unique_ptr (That was fixed) but because crossing over bindings created from maliput_py with bindings created in delphyne. I assume this happens because of bindings being created with different pybind11 versions(system vs robotlocomotion's fork)

4:   File "/__w/delphyne_demos/delphyne_demos/maliput_ws/install/delphyne/lib/python3.8/site-packages/delphyne/behaviours/roads.py", line 156, in setup
4:     self.road_geometry = self.road_network.get().road_geometry()
4: TypeError: Unable to convert function return value to a Python type! The signature was
4: 	(self: delphyne.roads.RoadNetworkWrapper) -> maliput::api::RoadNetwork

Here self.road_network.get() is returning a maliput::api::RoadNetwork however it isn't recognized as a maliput.api.RoadNetwork.

I am tempted to say that going with the option

Move the delphyne python api that depends on maliput to not use maliput python bindings.

Seems the way to go at this point.

CC: @agalbachicar

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants