Expose internal errors and convergence status #265
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
[#676] FAILED on melodicTest failed[#676] FAILED on kineticTest failed |
[#677] PASSED on kineticAll tests passed[#677] PASSED on melodicAll tests passed |
Codecov Report
@@ Coverage Diff @@
## master #265 +/- ##
==========================================
+ Coverage 94.28% 94.28% +<.01%
==========================================
Files 28 28
Lines 1661 1662 +1
==========================================
+ Hits 1566 1567 +1
Misses 95 95
Continue to review full report at Codecov.
|
at-wat
reviewed
Dec 16, 2019
src/mcl_3dl.cpp
Outdated
| const float float_max = std::numeric_limits<float>::max(); | ||
| pnh_.param("std_warn_thresh_x", std_warn_thresh_[0], float_max); | ||
| pnh_.param("std_warn_thresh_y", std_warn_thresh_[1], float_max); | ||
| pnh_.param("std_warn_thresh_z", std_warn_thresh_[2], float_max); |
There was a problem hiding this comment.
As 3D stddev can't represent orientation of the error ellipse (excepting 0 and 90 deg), it would better be 1D threshold. (or 2D (xy and z) may have meaning for mobile robot.)
I would like to recommend to use amcl_pose.covariance if the user want to check convergency in 3D or 6DOF.
[#682] PASSED on melodicAll tests passed[#682] PASSED on kineticAll tests passed |
at-wat
reviewed
Dec 23, 2019
src/mcl_3dl.cpp
Outdated
| @@ -1289,6 +1344,11 @@ class MCL3dlNode | |||
| pnh_.param("publish_tf", publish_tf_, true); | |||
| pnh_.param("output_pcd", output_pcd_, false); | |||
|
|
|||
| const float float_max = std::numeric_limits<float>::max(); | |||
| pnh_.param("std_warn_thresh_x", std_warn_thresh_[0], float_max); | |||
| pnh_.param("std_warn_thresh_y", std_warn_thresh_[1], float_max); | |||
There was a problem hiding this comment.
IMO, having x and y separately doesn't make sense as the error ellipse represented by (x, y) parameter can't represent the major axis orientation of the ellipse.
For mobile robot purpose, separating z may make sense due to the constraint of the floor.
There was a problem hiding this comment.
@at-wat Addressed your comment.
[#705] PASSED on kineticAll tests passed[#705] PASSED on melodicAll tests passed |
[#706] PASSED on kineticAll tests passed[#706] PASSED on melodicAll tests passed |
at-wat
reviewed
Jan 10, 2020
src/mcl_3dl.cpp
Outdated
| @@ -28,6 +28,7 @@ | |||
| */ | |||
|
|
|||
| #include <algorithm> | |||
| #include <limits> | |||
src/mcl_3dl.cpp
Outdated
|
|
||
| bool fake_imu_, fake_odom_; | ||
| ros::Time match_output_last_; | ||
| ros::Time odom_last_; | ||
| bool has_map_; | ||
| bool has_odom_; | ||
| bool has_imu_; | ||
| bool has_points_; |
[#710] PASSED on kineticAll tests passed[#710] PASSED on melodicAll tests passed |
Co-Authored-By: Atsushi Watanabe <[email protected]>
[#711] PASSED on kineticAll tests passed[#711] PASSED on melodicAll tests passed |
[#712] PASSED on melodicAll tests passed[#712] PASSED on kineticAll tests passed |
at-wat
reviewed
Jan 15, 2020
Co-Authored-By: Atsushi Watanabe <[email protected]>
[#713] PASSED on melodicAll tests passed[#713] PASSED on kineticAll tests passed |
at-wat
reviewed
Jan 16, 2020
src/mcl_3dl.cpp
Outdated
| * * Neither the name of the copyright holder nor the names of its | ||
| * contributors may be used to endorse or promote products derived from | ||
| * * Neither the name of the copyright holder nor the names of its | ||
| * contributors may be used to endorse or promote products derived from |
at-wat
approved these changes
Jan 16, 2020
[#714] PASSED on kineticAll tests passed[#714] PASSED on melodicAll tests passed |
at-wat
changed the title
Closed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
closes #264