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

Feature/pybind11 experimental lift entry watchdog #96

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

tongkiat
Copy link
Contributor

Added some python bindings

Added python binding for C++ function rmf_fleet_adapter::agv::RobotUpdateHandle::Unstable::set_lift_entry_watchdog.

Added python binding for C++ enum rmf_fleet_adapter::agv::RobotUpdateHandle::Unstable::Decision.

@tongkiat tongkiat force-pushed the feature/pybind11-experimental-lift-entry-watchdog branch from e118271 to 355777b Compare August 11, 2021 09:13
@tongkiat
Copy link
Contributor Author

tongkiat commented Aug 11, 2021

I know these are unstable APIs. I'll amend them if/when the C++ function and/or enum changes.

Also, I'm wondering whether it's OK if I contribute the python version of the full_control fleet_adapter. In addition, I'm thinking of writing an accompanying article or note to help more people implement their own fleet_adapters. I hope this will pique the interest of more causal tinkerers (like me) and, perhaps, encourage/nudge these people to experiment or adopt RMF for their own just-for-fun projects. 😄

@codecov
Copy link

codecov bot commented Aug 11, 2021

Codecov Report

Merging #96 (8cc0610) into main (87ce942) will decrease coverage by 0.09%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main      #96      +/-   ##
==========================================
- Coverage   21.40%   21.30%   -0.10%     
==========================================
  Files        1260      210    -1050     
  Lines      101676    16957   -84719     
  Branches    48378     8073   -40305     
==========================================
- Hits        21760     3613   -18147     
+ Misses      56505     9441   -47064     
+ Partials    23411     3903   -19508     
Flag Coverage Δ
tests 21.30% <ø> (-0.10%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
..._adapter/src/rmf_fleet_adapter/phases/DoorOpen.cpp
.../rmf_task_ros2/src/rmf_task_ros2/action/Server.cpp
...adapter/src/rmf_fleet_adapter/agv/RobotContext.cpp
...eet_adapter/rmf_rxcpp/include/rmf_rxcpp/RxJobs.hpp
...ffic_ros2/src/rmf_traffic_ros2/geometry/Circle.cpp
..._fleet_adapter/test/adapters/test_TrafficLight.cpp
...mf_rxcpp/RxCpp-4.1.0/Rx/v2/src/rxcpp/rx-predef.hpp
...raffic_ros2/src/rmf_traffic_ros2/convert_Route.cpp
...lude/rmf_fleet_adapter_python/geometry/PyShape.hpp
...rmf_fleet_adapter/test/services/test_Negotiate.cpp
... and 1460 more

Copy link
Member

@youliangtan youliangtan 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 your PR. Overall this looks good, just dropped in one small comment.
For sure we absolutely welcome any external contributions and encourage people to try out RMF! On a side note, you can also refer to our mir fleet adapter: https://github.com/osrf/fleet_adapter_mir/. Also we will soon (in this few weeks) release another py-based fleet adapter example template for easy integration, hopefully this can be a good reference for you. Will drop in the link once it's ready.

auto m_robot_update_handle = m.def_submodule("robot_update_handle");

py::enum_<agv::RobotUpdateHandle::Unstable::Decision>(
m_robot_update_handle, "Decision")
Copy link
Member

Choose a reason for hiding this comment

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

Maybe we can explicitly mention this as unstable, or create an unstable def_submodule.

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 this pull request may close these issues.

2 participants