jsk-ros-pkg / jsk_3rdparty

42 stars 60 forks source link

[dialogflow_task_executive] adding some args in dialogflow_ros.launch #361

Closed mqcmd196 closed 2 years ago

mqcmd196 commented 2 years ago

TODO

knorth55 commented 2 years ago

can you add roslaunch_add_file_check to prevent missing args? https://github.com/jsk-ros-pkg/jsk_robot/blob/master/jsk_fetch_robot/jsk_fetch_startup/CMakeLists.txt#L60-L81

mqcmd196 commented 2 years ago

Oh I didn't know the test. I'll try adding it.

mqcmd196 commented 2 years ago

I confirmed this PR works well in real robot. I added roslaunch test and found missing topic_tools dependency.

k-okada commented 2 years ago

that is test_depend ?

◉ Kei Okada

2022年7月25日(月) 14:29 Yoshiki Obinata @.***>:

@.**** commented on this pull request.

In dialogflow_task_executive/package.xml https://github.com/jsk-ros-pkg/jsk_3rdparty/pull/361#discussion_r928481812 :

@@ -19,9 +19,11 @@

catkin_virtualenv message_generation std_msgs
  • roslaunch

I followed this instruction

https://wiki.ros.org/roslaunch#Catkin

— Reply to this email directly, view it on GitHub https://github.com/jsk-ros-pkg/jsk_3rdparty/pull/361#discussion_r928481812, or unsubscribe https://github.com/notifications/unsubscribe-auth/AADYNXDA3OD7RWCNXOX7H2TVVYQXXANCNFSM54G56INA . You are receiving this because you commented.Message ID: @.***>

mqcmd196 commented 2 years ago

@k-okada I see, I checked http://docs.ros.org/en/jade/api/catkin/html/howto/format2/migrating_from_format_1.html and it might be test_depend

mqcmd196 commented 2 years ago

@k-okada Could you merge this? The original dialogflow_ros.launch doesn't works without this PR.