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

Add rosplan_planner_interfaces with ffha_planning_interface example launch file #33

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

tomcreutz
Copy link

Added a demo package for planning interfaces. It contains an example launch file to use the ff_planner_interface with ffha planner. To test the changes please have a look on the changes at KCL-Planning/ROSPlan#234

@@ -0,0 +1,3 @@
cmake_minimum_required(VERSION 2.8.3)
project(rosplan_planner_interfaces)
find_package(catkin REQUIRED)
Copy link
Contributor

Choose a reason for hiding this comment

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

no new line at end of file

Copy link
Contributor

Choose a reason for hiding this comment

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

Please additionally rename the name of the ROS pkg from rosplan_planner_interfaces -> rosplan_planner_interfaces_demo


</node>

</launch>
Copy link
Contributor

Choose a reason for hiding this comment

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

no new line at end of file

<maintainer email="[email protected]">Tom Creutz</maintainer>

<license>BSD</license>

Copy link
Contributor

Choose a reason for hiding this comment

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

remove extra line

Comment on lines +14 to +16
<export>

</export>
Copy link
Contributor

Choose a reason for hiding this comment

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

if export is empty, then remove

<package format="2">
<name>rosplan_planner_interfaces</name>
<version>1.0.0</version>
<description>rosplan_planner_interfaces repository with example launch files to use planner_interfaces</description>
Copy link
Contributor

Choose a reason for hiding this comment

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

rosplan_planner_interfaces is not a repository, the repo is rosplan_demos, it is a ROS pkg ; )

Comment on lines +34 to +35
<!-- use this param to use either ff_planner_interface with ff planner ("use_ffha" = "false" or ffha planner ("use_ffha" = "true" -->
<param name="use_ffha" value="$(arg use_ffha)" />
Copy link
Contributor

Choose a reason for hiding this comment

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

please indent to match previous params

<arg name="data_path" default="$(find rosplan_planning_system)/test/pddl/turtlebot/" />
<arg name="planner_command" default="timeout 10 $(find rosplan_planning_system)/common/bin/popf DOMAIN PROBLEM" />
<arg name="planner_interface" default="ff_planner_interface" />
<arg name="use_ffha" default="true" />
Copy link
Contributor

Choose a reason for hiding this comment

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

please indent to match previous args

@oscar-lima oscar-lima self-requested a review March 12, 2020 13:01
<arg name="use_problem_topic" default="true" />
<arg name="problem_topic" default="/rosplan_problem_interface/problem_instance" />
<arg name="planner_topic" default="planner_output" />
<arg name="domain_path" default="$(find rosplan_planning_system)/test/pddl/turtlebot/domain.pddl" />
Copy link
Contributor

@oscar-lima oscar-lima Mar 12, 2020

Choose a reason for hiding this comment

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

this domain has durative actions, which are not supported neither by ff nor ffha. I suggest to change it e.g. for ROSPlan/rosplan_planning_system/test/pddl/amazon

Copy link
Contributor

@oscar-lima oscar-lima left a comment

Choose a reason for hiding this comment

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

Please apply the feedback left in comments, thanks!


<!-- directory for files -->
<param name="domain_path" value="$(arg domain_path)" />
<param name="problem_path" value="$(arg problem_path)" />
Copy link
Contributor

Choose a reason for hiding this comment

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

please change from problem_path -> autom_gen_problem_path (consistency w/ ROSPlan launch files reasons...)

<arg name="domain_path" default="$(find rosplan_planning_system)/test/pddl/turtlebot/domain.pddl" />
<arg name="problem_path" default="$(find rosplan_planning_system)/test/pddl/turtlebot/problem.pddl" />
<arg name="data_path" default="$(find rosplan_planning_system)/test/pddl/turtlebot/" />
<arg name="planner_command" default="timeout 10 $(find rosplan_planning_system)/common/bin/popf DOMAIN PROBLEM" />
Copy link
Contributor

Choose a reason for hiding this comment

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

popf? planner command should be something like rosrun ffha ffha foo bar ... etc

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