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

Add bindings for AutomotiveSimulator #177

Merged
merged 3 commits into from
Jan 2, 2018
Merged

Add bindings for AutomotiveSimulator #177

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9045f35
move to pybind and move simulator initialization to Python side
mikaelarguedas Dec 22, 2017
06edbb0
document missing bindings and their dependencies
mikaelarguedas Dec 22, 2017
57dea01
address review comments
mikaelarguedas Jan 2, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 0 additions & 9 deletions backend/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
include (${project_cmake_dir}/Utils.cmake)
include_directories(${PYTHON_INCLUDE_DIRS})
Copy link
Contributor

Choose a reason for hiding this comment

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

BTW Should this just rely solely on pybind11::module, and not need the include_directories?
Also, I think the Python include directories should be a transitive dependency of pybind11 - could it be removed here (if you're not using it directly)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah that's what I thought as well but noticed that it was currently not the case. If I remove this line:

In file included from /home/mikael/work/delphyne_demo/install/include/pybind11/pytypes.h:12:0,
                 from /home/mikael/work/delphyne_demo/install/include/pybind11/cast.h:13,
                 from /home/mikael/work/delphyne_demo/install/include/pybind11/attr.h:13,
                 from /home/mikael/work/delphyne_demo/install/include/pybind11/pybind11.h:43,
                 from /home/mikael/work/delphyne_demo/src/delphyne/backend/simulation_runner.cc:39:
/home/mikael/work/delphyne_demo/install/include/pybind11/detail/common.h:111:20: fatal error: Python.h: No such file or directory

I agree that this should be provided by the pybind module and I think pybind doest the right thing as I don't need this line if I use pybind built with CMake. There must be something fishy happening when building it with bazel that doesnt walk the imported target properly.

I'm in favor of leaving it for now as it allows us to move forward, I can ticket it on the Drake repo and or add a todo in this repo.

include_directories(${pybind11_INCLUDE_DIRS})

# ----------------------------------------
# Prepare sources
set (sources
Expand Down Expand Up @@ -40,13 +38,6 @@ delphyne_build_tests(${gtest_sources})
# ----------------------------------------
# Python bindings

# Find necessary packages
find_package(PythonLibs 2.7 REQUIRED)
include_directories(${PYTHON_INCLUDE_DIRS})

find_package(Boost COMPONENTS python REQUIRED)
include_directories(${Boost_INCLUDE_DIR})

# Build the simulation support for python bindings as a library.
add_library(simulation-support
STATIC
Expand Down
24 changes: 22 additions & 2 deletions backend/python_bindings_example.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,14 @@
import time

from launcher import Launcher
from simulation_runner_py import SimulatorRunner

from pydrake.common import AddResourceSearchPath

from simulation_runner_py import (
AutomotiveSimulator,
SimpleCarState,
SimulatorRunner
)


def get_from_env_or_fail(var):
Copy link
Contributor

Choose a reason for hiding this comment

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

BTW Below, the stripping could be done by value.rstrip(' :').

Expand Down Expand Up @@ -102,6 +109,15 @@ def random_print():
print("One in five hundred")


def create_and_init_automotive_simulator():
simulator = AutomotiveSimulator()
state = SimpleCarState()
state.y = 0.0
simulator.AddPriusSimpleCar("0", "DRIVING_COMMAND_0", state)

return simulator


def main():
"""Spawn an automotive simulator making use of the python bindings"""
stats = SimulationStats()
Expand All @@ -111,6 +127,10 @@ def main():
lcm_ign_bridge = "duplex-ign-lcm-bridge"
ign_visualizer = "visualizer"

drake_install_path = get_from_env_or_fail('DRAKE_INSTALL_PATH')
AddResourceSearchPath(os.path.join(drake_install_path, "share", "drake"))

simulator = create_and_init_automotive_simulator()
try:
launcher.launch([lcm_ign_bridge, "1"])
teleop_config = os.path.join(delphyne_ws_dir,
Expand All @@ -120,7 +140,7 @@ def main():
"layoutWithTeleop.config")
launcher.launch([ign_visualizer, teleop_config])

runner = SimulatorRunner()
runner = SimulatorRunner(simulator, 0.001)
# Add a callback to record and print statistics
runner.AddStepCallback(stats.record_tick)
# Add a second callback that prints a message roughly every 500 calls
Expand Down
1 change: 0 additions & 1 deletion backend/simulation_runner.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@
#include <ignition/msgs.hh>
#include <ignition/transport/Node.hh>

#include <Python.h>
#include <pybind11/pybind11.h>

#include "backend/automotive_simulator.h"
Expand Down
65 changes: 47 additions & 18 deletions backend/simulation_runner_py.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,41 +35,70 @@
#include "backend/automotive_simulator.h"
#include "backend/simulation_runner.h"

namespace py = pybind11;

using delphyne::backend::AutomotiveSimulator;
using delphyne::backend::SimulatorRunner;
using drake::automotive::SimpleCarState;

// Since we are not yet exporting the AutomotiveSimulator class we need to
// provide a ready-to-run SimulatorRunner. To do so we create a parameterless
// constructor that sets up a simulation runner with a prius car in it. As we
// keep adding python bindings to C++ classes this code will be moved to the
// python scripts that launches the simulation.

namespace py = pybind11;

namespace {
PYBIND11_MODULE(simulation_runner_py, m) {
py::class_<SimulatorRunner>(m, "SimulatorRunner")
.def(py::init([](void) {
// TODO(mikaelaguedas) All this should be done in Python using pydrake
// and custom bindings for AutomotiveSimulator and SimpleCarState
drake::AddResourceSearchPath(std::string(std::getenv("DRAKE_INSTALL_PATH")) +
"/share/drake");

auto simulator =
std::make_unique<delphyne::backend::AutomotiveSimulator<double>>();

// Add a Prius car.
drake::automotive::SimpleCarState<double> state;
state.set_y(0.0);
simulator->AddPriusSimpleCar("0", "DRIVING_COMMAND_0", state);

// Instantiate the simulator runner and pass the simulator.
const double time_step = 0.001;
.def(py::init([](std::unique_ptr<AutomotiveSimulator<double>> simulator, double time_step) {
return std::make_unique<SimulatorRunner>(std::move(simulator), time_step);
Copy link
Contributor

@EricCousineau-TRI EricCousineau-TRI Jan 2, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

Just a heads up (sorry for adding this post-merge): You should be able to use py::init<Args...>() as a shorthand for py::init([](Args&& args...) { return make_unique<...>(...); }.

Copy link

Choose a reason for hiding this comment

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

Thanks @EricCousineau-TRI , done in f3232e0

}))
.def("Start", &SimulatorRunner::Start)
.def("Stop", &SimulatorRunner::Stop)
.def("AddStepCallback", &SimulatorRunner::AddStepCallback);
;
py::class_<AutomotiveSimulator<double>, std::unique_ptr<AutomotiveSimulator<double>>>(m, "AutomotiveSimulator")
Copy link
Contributor

@EricCousineau-TRI EricCousineau-TRI Dec 28, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

BTW Specifying unique_ptr as the holder type is redundant; if you're going to always specify, I'd suggest making an extension of py::class_ to handle these things by default for you.

(At some point in the near future, I'll be introducing something like pydrake::class_ or something like that, for this and other facets.)

.def(py::init([](void) {
return std::make_unique<AutomotiveSimulator<double>>();
}))
.def("Start", &AutomotiveSimulator<double>::Start)
.def("AddPriusSimpleCar", &AutomotiveSimulator<double>::AddPriusSimpleCar)
.def("AddMobilControlledSimpleCar", &AutomotiveSimulator<double>::AddMobilControlledSimpleCar)
// TODO(mikaelarguedas) bind more method depending on what we need
// Needs drake::automotive::Curve2<double>&
// .def("AddPriusTrajectoryCar", &AutomotiveSimulator<double>::AddPriusTrajectoryCar)
// Needs:
// - drake::automotive::LaneDirection
// - drake::automotive::MaliputRailcarParams<T>
// - drake::automotive::MaliputRailcarState<T>
// .def("AddPriusMaliputRailcar", &AutomotiveSimulator<double>::AddPriusMaliputRailcar)
// Needs:
// - drake::automotive::LaneDirection
// - drake::automotive::MaliputRailcarParams<T>
// - drake::automotive::MaliputRailcarState<T>
// .def("AddIdmControlledPriusMaliputRailcar", &AutomotiveSimulator<double>::AddIdmControlledPriusMaliputRailcar)
// .def("SetMaliputRailcarAccelerationCommand", &AutomotiveSimulator<double>::SetMaliputRailcarAccelerationCommand)
// Needs drake::maliput::api::RoadGeometry binding
// .def("SetRoadGeometry", &AutomotiveSimulator<double>::SetRoadGeometry)
// Needs drake::maliput::api::Lane binding
// .def("FindLane", &AutomotiveSimulator<double>::FindLane)
// Needs drake::systems::System<T> binding
// .def("GetDiagramSystemByName", &AutomotiveSimulator<double>::GetDiagramSystemByName)
// .def("Build", &AutomotiveSimulator<double>::Build)
// .def("GetDiagram", &AutomotiveSimulator<double>::GetDiagram)
// .def("StepBy", &AutomotiveSimulator<double>::StepBy)
// Needs drake::systems::rendering::PoseBundle<T> binding
// .def("GetCurrentPoses", &AutomotiveSimulator<double>::GetCurrentPoses)
;
// TODO(mikaelarguedas) Submit this to upstream Drake
py::class_<SimpleCarState<double>>(m, "SimpleCarState")
Copy link
Contributor

Choose a reason for hiding this comment

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

Following up from my comment on the issue thread, and since Jon requested this, I'll go ahead and submit a PR with this in it. (Feel free to fork that PR if you need more things.)

Copy link

Choose a reason for hiding this comment

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

Great, thanks!

.def(py::init<>())
.def_property("x", &SimpleCarState<double>::x, &SimpleCarState<double>::set_x)
.def_property("y", &SimpleCarState<double>::y, &SimpleCarState<double>::set_y)
.def_property("heading", &SimpleCarState<double>::heading, &SimpleCarState<double>::set_heading)
.def_property("velocity", &SimpleCarState<double>::velocity, &SimpleCarState<double>::set_velocity)
.def("get_coordinates_names", &SimpleCarState<double>::GetCoordinateNames)
;
}

} // namespace