-
Notifications
You must be signed in to change notification settings - Fork 30
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
ros_control/RosTrajectoryExecutor #149
Changes from 1 commit
43ea8bf
e3f783b
4391bf9
338f1b7
aafa1cf
b013bfd
793a23a
a2e0ed8
22ba7b4
9e09781
6714011
785871b
c98e8ea
5a655e5
d4bdaa3
300e050
d0d29dc
f750b32
3a10630
9b9ba1e
064aa8a
8f1a94b
fc70347
4fd63b7
65292c0
6ad1d9c
5db6d4e
6b71cb1
98e8440
5f0f1ae
2537bfd
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,79 @@ | ||
#ifndef AIKIDO_CONTROL_ROS_ROSTRAJECTORYEXECUTOR_HPP_ | ||
#define AIKIDO_CONTROL_ROS_ROSTRAJECTORYEXECUTOR_HPP_ | ||
#include <chrono> | ||
#include <future> | ||
#include <mutex> | ||
#include <ros/ros.h> | ||
#include <ros/callback_queue.h> | ||
#include <control_msgs/FollowJointTrajectoryAction.h> | ||
#include <dart/dart.hpp> | ||
#include <aikido/control/TrajectoryExecutor.hpp> | ||
#include <aikido/trajectory/Trajectory.hpp> | ||
|
||
// actionlib and DART both #define this macro. | ||
#undef DEPRECATED | ||
#include <actionlib/client/action_client.h> | ||
#undef DEPRECATED | ||
|
||
namespace aikido { | ||
namespace control { | ||
namespace ros { | ||
|
||
class RosTrajectoryExecutor : public aikido::control::TrajectoryExecutor | ||
{ | ||
public: | ||
RosTrajectoryExecutor( | ||
::dart::dynamics::MetaSkeletonPtr skeleton, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why do we need to tie each There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Great point! It seems like we could replace There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This probably comes from my lack of knowledge in ROS. My question was, can't a |
||
::ros::NodeHandle node, | ||
const std::string& serverName, | ||
double timestep, | ||
double goalTimeTolerance, | ||
std::chrono::milliseconds connectionTimeout | ||
= std::chrono::milliseconds{1000}, | ||
std::chrono::milliseconds connectionPollingRate | ||
= std::chrono::milliseconds{20} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit: Rename this to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would like to suggest templatizing this constructor for the each time-related types so that we could pass any of time units. Here is a similar example. In this case, however, we would like to templatize for two possibly different duration types like: template <typename DurationA, typename DurationB>
RosTrajectoryExecutor(...,
const DurationA& connectionTimeout = std::chrono::milliseconds{1000},
const DurationB& connectionPollingPeriod = std::chrono::milliseconds{20}); There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am generally supportive of this, but I am not sure how to implement it. We store these There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We actually don't need to templatize the whole class but only this constructor. The member variable can be any specific duration type. Here is an example what I did: member variable, templated constructor. |
||
); | ||
|
||
virtual ~RosTrajectoryExecutor(); | ||
|
||
std::future<void> execute(trajectory::TrajectoryPtr _traj) override; | ||
|
||
std::future<void> execute( | ||
trajectory::TrajectoryPtr _traj, const ::ros::Time& _startTime); | ||
|
||
/// Simulates mTraj. To be executed on a separate thread. | ||
void spin(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should refactor this function into a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nevermind! It turns out that Maybe we should rename this to something more obvious? Maybe |
||
|
||
private: | ||
using TrajectoryActionClient | ||
= actionlib::ActionClient<control_msgs::FollowJointTrajectoryAction>; | ||
using GoalHandle = TrajectoryActionClient::GoalHandle; | ||
|
||
bool waitForServer(); | ||
|
||
void transitionCallback(GoalHandle _handle); | ||
|
||
::ros::NodeHandle mNode; | ||
::ros::CallbackQueue mCallbackQueue; | ||
TrajectoryActionClient mClient; | ||
TrajectoryActionClient::GoalHandle mGoalHandle; | ||
|
||
::dart::dynamics::MetaSkeletonPtr mSkeleton; | ||
double mTimestep; | ||
double mGoalTimeTolerance; | ||
|
||
std::chrono::milliseconds mConnectionTimeout; | ||
std::chrono::milliseconds mConnectionPollingRate; | ||
|
||
bool mInProgress; | ||
std::promise<void> mPromise; | ||
trajectory::TrajectoryPtr mTrajectory; | ||
|
||
std::mutex mMutex; | ||
}; | ||
|
||
} // namespace ros | ||
} // namespace control | ||
} // namespace aikido | ||
|
||
#endif // ifndef AIKIDO_CONTROL_ROS_ROSTRAJECTORYEXECUTOR_HPP_ |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,41 @@ | ||
#============================================================================== | ||
# Dependencies | ||
# | ||
find_package(actionlib QUIET) | ||
aikido_check_package(actionlib "aikido::control::ros" "actionlib") | ||
|
||
find_package(control_msgs QUIET) | ||
aikido_check_package(control_msgs "aikido::control::ros" "control_msgs") | ||
|
||
find_package(roscpp QUIET) | ||
aikido_check_package(roscpp "aikido::control::ros" "roscpp") | ||
|
||
#============================================================================== | ||
# Libraries | ||
# | ||
set(sources | ||
RosTrajectoryExecutor.cpp | ||
) | ||
|
||
add_library("${PROJECT_NAME}_control_ros" SHARED ${sources}) | ||
target_include_directories("${PROJECT_NAME}_control_ros" SYSTEM | ||
PUBLIC | ||
${actionlib_INCLUDE_DIRS} | ||
${control_msgs_INCLUDE_DIRS} | ||
${roscpp_INCLUDE_DIRS} | ||
) | ||
target_link_libraries("${PROJECT_NAME}_control_ros" | ||
"${PROJECT_NAME}_control" | ||
"${PROJECT_NAME}_statespace" | ||
"${PROJECT_NAME}_trajectory" | ||
${DART_LIBRARIES} | ||
${actionlib_LIBRARIES} | ||
${control_msgs_LIBRARIES} | ||
${roscpp_LIBRARIES} | ||
) | ||
|
||
add_component(${PROJECT_NAME} control_ros) | ||
add_component_targets(${PROJECT_NAME} control_ros "${PROJECT_NAME}_control_ros") | ||
add_component_dependencies(${PROJECT_NAME} control_ros control statespace trajectory) | ||
|
||
# coveralls_add_sources(${sources}) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Unless there is a strong reason not to, we should uncomment this. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jslee02 Is this still necessary? I believe DART switched to the
DART_DEPRECATED
macro to avoid a conflict just like this.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not necessary. DART switched to the
DART_DEPRECATED
since version 6.0.1.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@gilwoolee Can you switch from
DEPRECATED
toDART_GENERATED
and remove this#undef
.