Software Training #16
Conversation
| <build_depend>actionlib</build_depend> | ||
| <exec_depend>actionlib</exec_depend> | ||
| <build_depend>actionlib_msgs</build_depend> | ||
| <exec_depend>actionlib_msgs</exec_depend> |
There was a problem hiding this comment.
if its build_depend and exec_depend you can just put the depend tag, its cleaner
| { | ||
| public: | ||
| ros::NodeHandle node; | ||
| GotoAction(std::string name) : as_(node, name, boost::bind(&GotoAction::executeCB, this, _1), false) |
There was a problem hiding this comment.
instead of having NodeHandle as public you can pass the NodeHandle into the constructor. Nobody should be modifying the NodeHandle so it should be private
| void executeCB(const software_training_assignment::GotoGoal::ConstPtr &goal) | ||
| { | ||
|
|
||
| ROS_INFO("executeCB called"); |
There was a problem hiding this comment.
this formatting is all off, make sure to format properly so that it's easy to read the code
|
|
||
| ROS_INFO("executeCB called"); | ||
| double relativeX = goal->x - x, | ||
| relativeY = goal->y - y, |
There was a problem hiding this comment.
separate each double declaration, it's cleaner and easier to read
|
|
||
| pub.publish(rotation); | ||
|
|
||
| ros::Duration(1).sleep(); |
|
|
||
| ros::Duration(1).sleep(); | ||
|
|
||
| double gotoCount = (int)relativeDist / 2, gotoRe = relativeDist - (gotoCount * 2); |
There was a problem hiding this comment.
same as above for declaring variables
| @@ -0,0 +1,8 @@ | |||
| #ifndef KILLTURTLES_H | |||
There was a problem hiding this comment.
should just use #pragma once instead of header guards, its standard for most compilers
|
|
||
| #include <ros/ros.h> | ||
|
|
||
| bool killTurtles(ros::NodeHandle n); |
There was a problem hiding this comment.
should always pass NodeHandle by reference, we don't want create another copy
|
|
||
| namespace P | ||
| { | ||
| class Publisher |
There was a problem hiding this comment.
shouldn't use Publisher as a name, it will be confusing. Instead be more descriptive - PositionPublisher
| bool resetPos(std_srvs::Empty::Request &req, | ||
| std_srvs::Empty::Response &res) | ||
| { | ||
| ros::NodeHandle n; |
There was a problem hiding this comment.
should pass the NodeHandle by reference, you ideally should only need 1 NodeHandle per node you write
| y = msg->y; | ||
| } | ||
|
|
||
| void executeCB(const software_training_assignment::GotoGoal::ConstPtr &goal) |
There was a problem hiding this comment.
the easiest way to write this function would be to determine distance and angle from the point and send the twist msg until you've reached the point. the feedback would be given as distance from the goal and once you have reached the succeed flag could be set. this is something to think about
| @@ -0,0 +1,9 @@ | |||
| #ifndef RESETPOS_H | |||
There was a problem hiding this comment.
see comment above about header guards
| #include "spawnTurtles.h" | ||
|
|
||
|
|
||
| bool spawnTurtles(ros::NodeHandle n, std::string name, int x, int y) |
| posPublisher.sub = n.subscribe("moving_turtle/pose", 1000, &Publisher::subCallback, &posPublisher); | ||
|
|
||
| GotoAction goTo("goto"); | ||
| goTo.node = n; |
There was a problem hiding this comment.
same with my comment above, instead of setting it equal, it should be passed by reference through the constructor so it is immutable by other processes
|
|
||
| software_training_assignment::GotoGoal goal; | ||
|
|
||
| goal.x = atoll(argv[1]); |
There was a problem hiding this comment.
we don't use command line arguments for nodes usually. most of the time the data is passed through subscriber or publisher callbacks
No description provided.