Address code review comments

Signed-off-by: Nahuel Espinosa <[email protected]>
Ekumen-OS · May 14, 2023 · 881237d · 881237d
1 parent b39b7a2
commit 881237d
Show file tree

Hide file tree

Showing 4 changed files with 32 additions and 24 deletions.
diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md
@@ -103,7 +103,7 @@ This section guides you through contributing code to Beluga. Following these gui
 
 #### A Primer On Project Workflow
 
-This projects adopts a [feature branch workflow](https://about.gitlab.com/topics/version-control/what-is-git-workflow/#feature-branching-git-workflow) ([forking workflow](https://about.gitlab.com/topics/version-control/what-is-git-workflow/#forking-git-workflow) for contributors that are not maintainers), with pull requests for code integration. Every code contribution must be associated to a [feature request](#requesting-features) or [bug report](#reporting-bugs) with enough consensus and evidence to move forward, signaled with `needs-work` and `needs-fix` labels respectively. Contributors must sign-off each commit by adding a `Signed-off-by: ...` line to every commit message to certify that they have the right to submit the code they are contributing to the project according to the [Developer Certificate of Origin (DCO)](https://developercertificate.org/). Pull requests must have reviewed and approved at least once to be merged.
+This projects adopts a [feature branch workflow](https://about.gitlab.com/topics/version-control/what-is-git-workflow/#feature-branching-git-workflow) ([forking workflow](https://about.gitlab.com/topics/version-control/what-is-git-workflow/#forking-git-workflow) for contributors that are not maintainers), with pull requests for code integration. Every code contribution must be associated to a [feature request](#requesting-features) or [bug report](#reporting-bugs) with enough consensus and evidence to move forward, signaled with `needs-work` and `needs-fix` labels respectively. Contributors must sign-off each commit by adding a `Signed-off-by: ...` line to every commit message to certify that they have the right to submit the code they are contributing to the project according to the [Developer Certificate of Origin (DCO)](https://developercertificate.org/). Pull requests must have been reviewed and approved at least once to be merged.
 
 #### How Do I Submit a Good Code Contribution?
 

diff --git a/README.md b/README.md
@@ -22,7 +22,7 @@ This repository contains the following packages:
 | [`beluga`](beluga)                           | A ROS-agnostic extensible library to implement algorithms based on particle filters.                                    |
 | [`beluga_amcl`](beluga_amcl)                 | A ROS 2 wrapper, providing an executable node and a ROS 2 component.<br> It provides interface parity with `nav2_amcl`. |
 | [`beluga_example`](beluga_example)           | Example launch files, showing how to run Beluga-based nodes.                                                            |
-| [`beluga_benchmark`](beluga_benchmark)       | Scripts to benchmark, profile and also compare Beluga with other AMCL implementations.                                  |
+| [`beluga_benchmark`](beluga_benchmark)       | Scripts to benchmark, profile and also compare Beluga with other MCL implementations.                                   |
 | [`beluga_system_tests`](beluga_system_tests) | System integration tests for Beluga.                                                                                    |
 
 ## First Steps

diff --git a/beluga/README.md b/beluga/README.md
@@ -8,9 +8,9 @@ Its modularity allows users to compose solutions from reusable modules and to co
 The current set of features includes:
 
 - A configurable particle filter with support for:
-  - [Structure of arrays and array of structures](https://en.wikipedia.org/wiki/AoS_and_SoA) storage policies.
-  - Fixed resampling and adaptive KLD resampling policies to determine how many samples to take per iteration.
-  - Selective resampling, on-motion resampling, interval resampling policies to determine when to resample.
+  - [Structure of arrays and array of structures][aos_soa] storage policies.
+  - Fixed resampling and [adaptive KLD resampling][fox2001] policies to determine how many samples to take per iteration.
+  - [Selective resampling][grisetti2007], on-motion resampling, interval resampling policies to determine when to resample.
   - Sequential and parallel execution policies.
   - Weighted mean and covariance statistics for pose estimation.
 - Likelihood field and beam sensor models.
@@ -24,3 +24,7 @@ Beluga is built on top of the following open source libraries:
 - [Sophus](https://github.com/strasdat/Sophus): A C++ implementation of Lie groups using Eigen.
 - [Range](https://github.com/ericniebler/range-v3): The basis library for C++20's `std::ranges`.
 - [libciabatta](https://github.com/atomgalaxy/libciabatta): A composable mixin support library.
+
+[aos_soa]: https://en.wikipedia.org/wiki/AoS_and_SoA
+[fox2001]: https://dl.acm.org/doi/10.5555/2980539.2980632
+[grisetti2007]: https://doi.org/10.1109/TRO.2006.889486
diff --git a/beluga_amcl/README.md b/beluga_amcl/README.md
@@ -1,54 +1,54 @@
 # Beluga AMCL
 
 Beluga AMCL is a ROS 2 node based on [Beluga](../beluga) featuring interface parity with
-[nav2's AMCL](https://github.com/ros-planning/navigation2/tree/main/nav2_amcl).
+[nav2's AMCL][nav2_amcl].
 This package can be easily integrated with code that currently uses `nav2's AMCL`.
 
 ## ROS 2 Interface
 
 ### Parameters
 
-Beluga AMCL currently supports the majority of ROS parameters used in [nav2's AMCL](https://github.com/ros-planning/navigation2/tree/main/nav2_amcl).
-See the [nav2's configuration guide](https://navigation.ros.org/configuration/packages/configuring-amcl.html) and
+Beluga AMCL currently supports the majority of ROS parameters used in [nav2's AMCL][nav2_amcl].
+See the [nav2's configuration guide][nav2_configuration_guide] and
 [Beluga examples](../beluga_example/config/params.yaml) for reference.
 
 #### Extra Parameters
 
 Additionally, this node supports the following extra parameters:
 
-| Parameter                                                          | Description                                                                    |
-|--------------------------------------------------------------------|--------------------------------------------------------------------------------|
-| `spatial_resolution_[x, y, theta]`                                 | Resolution to divide the space in buckets for adaptive KLD resampling.         |
-| <nobr>`initial_pose.covariance_[x, y, yaw, xy, xyaw, yyaw]`</nobr> | Covariance to use with the initial pose when initializing the particle filter. |
-| `execution_policy`                                                 | Execution policy used to process particles [seq: sequential, par: parallel].   |
+| Parameter                                                          | Description                                                                       |
+|--------------------------------------------------------------------|-----------------------------------------------------------------------------------|
+| `spatial_resolution_[x, y, theta]`                                 | Resolution for [adaptive KLD resampling][fox2001].                                |
+| <nobr>`initial_pose.covariance_[x, y, yaw, xy, xyaw, yyaw]`</nobr> | Covariance to use with the initial pose when initializing the particle filter.    |
+| `execution_policy`                                                 | Execution policy used to process particles [seq: sequential, par: parallel].      |
 
 ### Subscribed Topics
 
 The subscribed topic names can be changed with the parameters `map_topic`, `scan_topic` and `initial_pose_topic`.
 
-| Topic            | Type                                      | Description                                                              |
-|------------------|-------------------------------------------|--------------------------------------------------------------------------|
-| `map`            | `nav_msgs/OccupancyGrid`                  | Input topic for map updates.                                             |
-| `scan`           | `sensor_msgs/LaserScan`                   | Input topic for laser scan updates.                                      |
-| `initial_pose`   | `geometry_msgs/PoseWithCovarianceStamped` | Input topic for pose and covariance to initialize the particle filter.   |
+| Topic            | Type                                      | Description                                                                 |
+|------------------|-------------------------------------------|-----------------------------------------------------------------------------|
+| `map`            | `nav_msgs/OccupancyGrid`                  | Input topic for map updates.                                                |
+| `scan`           | `sensor_msgs/LaserScan`                   | Input topic for laser scan updates.                                         |
+| `initial_pose`   | `geometry_msgs/PoseWithCovarianceStamped` | Input topic for pose mean and covariance to initialize the particle filter. |
 
 ### Published Topics
 
 | Topic            | Type                                      | Description                                                              |
 |------------------|-------------------------------------------|--------------------------------------------------------------------------|
 | `particle_cloud` | `nav2_msgs/ParticleCloud`                 | Output topic for particle cloud published at a fixed frequency.          |
-| `pose`           | `geometry_msgs/PoseWithCovarianceStamped` | Output topic for pose and covariance representing the filter estimation. |
+| `pose`           | `geometry_msgs/PoseWithCovarianceStamped` | Output topic for estimated pose mean and covariance in map frame.        |
 
 ### Transforms
 
 The frame names can be changed with the parameters `global_frame_id`, `odom_frame_id` and `base_frame_id`.
 Defaults are `map`, `odom` and `base`.
 
-| Transform         | Description                                                                                   |
-|-------------------|-----------------------------------------------------------------------------------------------|
-| `odom` to `base`  | Input transform used to update the motion model and detect movement to know when to resample. |
-| `base` to `laser` | Input transform used to convert laser scan points to base frame.                              |
-| `map` to `odom`   | Output transform computed from the particle filter estimate.                                  |
+| Transform         | Description                                                                                        |
+|-------------------|----------------------------------------------------------------------------------------------------|
+| `odom` to `base`  | Input transform used by motion models and resampling policies.                                     |
+| `base` to `laser` | Input transform used to convert laser scan points to base frame.                                   |
+| `map` to `odom`   | Output transform calculated from the estimated pose mean and the current _odom-to-base_ transform. |
 
 ### Exposed Services
 
@@ -60,3 +60,7 @@ Defaults are `map`, `odom` and `base`.
 
 - See [example launch files](../beluga_example) showing how to run Beluga-based nodes.
 - See [available benchmarks](../beluga_benchmark) for scripts and comparison with other AMCL implementations.
+
+[nav2_amcl]: https://github.com/ros-planning/navigation2/tree/main/nav2_amcl
+[nav2_configuration_guide]: https://navigation.ros.org/configuration/packages/configuring-amcl.html
+[fox2001]: https://dl.acm.org/doi/10.5555/2980539.2980632