AMCL Port #26
AMCL Port #26
Conversation
mhpanah
requested review from
mjeronimo,
mkhansenbot,
bpwilcox,
crdelsey and
orduno
src/amcl/.vscode/settings.json
Outdated
| @@ -0,0 +1,44 @@ | |||
| { | |||
src/amcl/CMakeLists.txt
Outdated
| @@ -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
src/amcl/CMakeLists.txt
Outdated
| 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?
src/amcl/include/amcl/map/map.h
Outdated
| @@ -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.
src/amcl/include/amcl/pf/pf.h
Outdated
| @@ -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.
src/amcl/src/amcl/map/map.c
Outdated
|
|
||
|
|
||
| // 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.
src/amcl/src/amcl/map/map_draw.c
Outdated
|
|
||
| //////////////////////////////////////////////////////////////////////////// | ||
| // 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.
src/amcl/src/amcl/map/map_range.c
Outdated
| // 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 |
|
@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>
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.