Conversation
src/amcl/.vscode/settings.json
Outdated
| @@ -0,0 +1,44 @@ | |||
| { | |||
| @@ -0,0 +1,84 @@ | |||
| cmake_minimum_required(VERSION 3.5) | |||
There was a problem hiding this comment.
There should be a "localization" directory under which we can put multiple implementations of localization. So, I'm thinking:/localization/amcl
| find_package(message_filters REQUIRED) | ||
| find_package(Boost REQUIRED) | ||
| find_package(tf2_geometry_msgs REQUIRED) | ||
| find_package(nav_msgs REQUIRED) |
There was a problem hiding this comment.
Can we eliminate a dependency on the Navigation1 package?
| @@ -0,0 +1,150 @@ | |||
| /* | |||
There was a problem hiding this comment.
We should consider the scope of the c++ classes and put the code in a corresponding location. For example, is this map class specific to amcl? Specific to localization? Generally useful to other modules as well? Depending on the answer, we would locate this class in a different location.
There was a problem hiding this comment.
Agreed. I'm planning to work on this on phase 2.
| @@ -0,0 +1,198 @@ | |||
| /* | |||
There was a problem hiding this comment.
Same comment as previous for these various libraries. We can have a /libs directory (or similar name) for those libraries that could be useful to various modules, beyond just localization.
| */ | ||
| /////////////////////////////////////////////////////////////////////////// | ||
| // | ||
| // Desc: LASER sensor model for AMCL |
There was a problem hiding this comment.
Similarly, the sensors should not be specific to localization and should be factored out and placed in an appropriate location.
There was a problem hiding this comment.
Yes, we will work on refactoring on phase 2.
src/amcl/package.xml
Outdated
| <build_depend>tf2</build_depend> | ||
| <build_depend>util</build_depend> | ||
|
|
||
| <!-- |
There was a problem hiding this comment.
Please remove commented-out sections.
|
|
||
|
|
||
| // Create a new map | ||
| map_t *map_alloc(void) |
There was a problem hiding this comment.
Why not simply make this a class instead of using a C interface?
There was a problem hiding this comment.
We can change the code structure on phase 2.
|
|
||
| //////////////////////////////////////////////////////////////////////////// | ||
| // Draw the occupancy map | ||
| void map_draw_occ(map_t *map, rtk_fig_t *fig) |
There was a problem hiding this comment.
These could be methods of the map class.
| // Extract a single range reading from the map. Unknown cells and/or | ||
| // out-of-bound cells are treated as occupied, which makes it easy to | ||
| // use Stage bitmap files. | ||
| double map_calc_range(map_t *map, double ox, double oy, double oa, double max_range) |
src/amcl/src/amcl_node.hpp
Outdated
| #ifndef LOCALIZATION__AMCLNODE_HPP_ | ||
| #define LOCALIZATION__AMCLNODE_HPP_ | ||
|
|
||
| #include <chrono> |
There was a problem hiding this comment.
These seems like an excessive amount of includes for this header. The header should only include files required by the interface itself. The implementation (.cpp) can include what's required to implement the interface.
src/amcl/src/main.cpp
Outdated
| return 0; | ||
| } | ||
|
|
||
| #if 0 |
mjeronimo
left a comment
There was a problem hiding this comment.
Let's create a localization subdirectory and remove the commented-out code before merging.
|
@mhpanah @mjeronimo - I thought we were going to wait on this until after the ros2_devel branch is merged? |
AUTO-1287 nav2 sync to solve MPPI CI build flag for NUC13
…os-navigation#26) * fix: handle transition failures in planner/controller/smoother servers * adding support for rest of servers + review comments * Replacing throws with error and failed lifecycle transitions * fix vel smoother unit tests * fixing docking server unit testing * fixing last bits --------- Signed-off-by: Kemal Bektas <kemal.bektas@node-robotics.com> Signed-off-by: Steve Macenski <stevenmacenski@gmail.com> Co-authored-by: Kemal Bektas <kemal.bektas@node-robotics.com>
…ation#26) * Action server interface for JointTrajectoryController * trajectory gives linear interpolation of points * update function checks joint tolerances and goals * action server now aborts on path violations * action server succeeds on goal achieved * add action server tests * publish_state publishes state data * implemented hold position on cancellation received * minor rework to trajectory api
Completed phase 1 of porting AMCL ROS1 package to ROS2. This phase has all the functionalities except dynamic reconfigure, rosbag, and laser scan subscriber which is equivalent to https://github.com/ros2/navigation/pull/9/files.