Add overview of the localization of the car #427
Conversation
WalkthroughThe changes involve updates to the documentation of the perception component, specifically the addition of a new section titled "Overview Localization" in Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (6)
doc/perception/README.md (2)
22-23: Improve clarity and grammar.The sentence structure could be enhanced for better readability.
Consider this revision:
-An overview over the different nodes working together to localize the vehicle is provided in the [localization](./localization.md) file. +An overview of the different nodes working together to localize the vehicle is provided in the [localization](./localization.md) file.🧰 Tools
🪛 LanguageTool
[uncategorized] ~22-~22: The preposition “of” seems more likely in this position.
Context: ... ## Overview Localization An overview over the different nodes working together to...(AI_EN_LECTOR_REPLACEMENT_PREPOSITION)
20-23: Consider expanding the overview section.Based on the PR objectives regarding GPS enhancement (issue #370), it would be valuable to briefly mention:
- The current reliance on GPS
- The potential for multi-sensor integration
- The planned improvements to the localization system
This context would help readers understand the current state and future direction of the localization system. Would you like assistance in drafting this additional content?
🧰 Tools
🪛 LanguageTool
[uncategorized] ~22-~22: The preposition “of” seems more likely in this position.
Context: ... ## Overview Localization An overview over the different nodes working together to...(AI_EN_LECTOR_REPLACEMENT_PREPOSITION)
doc/perception/localization.md (4)
1-8: Add a high-level overview of the localization system.Consider adding a brief introduction explaining the system's purpose, requirements, and overall architecture before diving into the individual nodes. This would help readers better understand the context.
# Localization + +## Overview + +The localization system determines the vehicle's position and heading by fusing data from multiple sensors including GPS, IMU, and map information. The system is designed to provide accurate and reliable position estimates even in noisy conditions through the use of filtering techniques. +Also, fix the grammar in the debug node description:
-The [position_heading_filter_debug_node](./position_heading_filter_debug_node.md) node is another useful node but it does not actively localize the vehicle. Instead it makes it possible to compare and tune different filters and the [viz.py](../../code/perception/src/experiments/Position_Heading_Datasets/viz.py) file is the recommended way to visualizes the results. +The [position_heading_filter_debug_node](./position_heading_filter_debug_node.md) node is another useful node, but it does not actively localize the vehicle. Instead, it makes it possible to compare and tune different filters. The [viz.py](../../code/perception/src/experiments/Position_Heading_Datasets/viz.py) file is the recommended way to visualize the results.🧰 Tools
🪛 LanguageTool
[uncategorized] ~8-~8: Use a comma before ‘but’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...bug_node.md) node is another useful node but it does not actively localize the vehic...(COMMA_COMPOUND_SENTENCE)
[uncategorized] ~8-~8: A comma may be missing after the conjunctive/linking adverb ‘Instead’.
Context: ...does not actively localize the vehicle. Instead it makes it possible to compare and tun...(SENT_START_CONJUNCTIVE_LINKING_ADVERB_COMMA)
🪛 Markdownlint
8-8: Expected: 300; Actual: 367
Line length(MD013, line-length)
4-4: null
Lists should be surrounded by blank lines(MD032, blanks-around-lists)
16-27: Add message types for published and subscribed topics.The topic descriptions should include their message types to help developers understand the data format.
The following topics are therefore published by this node: -# unfiltered_pos (raw data, subscribed to by filter nodes e.g. kalman_filter) -# unfiltered_heading (raw data, subscribed to by filter nodes e.g. kalman_filter) -# current_pos (filtered data, position of the car) -# current_heading (filtered data, orientation of the car around the z-axis) +- `unfiltered_pos` (geometry_msgs/PoseStamped - raw data, subscribed to by filter nodes e.g. kalman_filter) +- `unfiltered_heading` (std_msgs/Float64 - raw data, subscribed to by filter nodes e.g. kalman_filter) +- `current_pos` (geometry_msgs/PoseStamped - filtered data, position of the car) +- `current_heading` (std_msgs/Float64 - filtered data, orientation of the car around the z-axis)🧰 Tools
🪛 Markdownlint
17-17: null
Lists should be surrounded by blank lines(MD032, blanks-around-lists)
23-23: null
Lists should be surrounded by blank lines(MD032, blanks-around-lists)
66-81: Add filter parameters and tuning guidelines.The state vector description is good, but information about filter parameters (process and measurement noise matrices) and tuning guidelines would be valuable for maintainers.
The variables to be estimated are put together in the state vector. It consists of the following elements: - `x` (position on the x-axis) - `y` (position on the y-axis) - `v_x` (velocity in the x-direction) - `v_y` (velocity in the y-direction) - `yaw` (orientation, rotation around z-axis) - `omega_z` (angular velocity) + +### Filter Parameters + +The Kalman filter's performance depends on proper tuning of the following parameters: +- Process noise covariance matrix (Q): Models the uncertainty in the system's dynamics +- Measurement noise covariance matrix (R): Models the uncertainty in sensor measurements + +Guidelines for tuning: +1. Start with diagonal matrices +2. Adjust values based on sensor specifications and empirical testing +3. Use the debug node to validate parameter changes🧰 Tools
🪛 LanguageTool
[uncategorized] ~80-~80: Possible missing comma found.
Context: ...orientation and angular velocity of the vehicle the data provided by the IMU (Inertial ...(AI_HYDRA_LEO_MISSING_COMMA)
🪛 Markdownlint
67-67: null
Lists should be surrounded by blank lines(MD032, blanks-around-lists)
92-96: Restructure the map-based improvement suggestion.The suggestion to use map data for improving localization is valuable but should be presented more clearly.
-The localization of the vehicle could further be improved by combining the current estimate of the position with data generated by image processing. Using the vision node and the lidar distance node it is possible to calculate the distance to detected objects (for details see [distance_to_objects.md](./distance_to_objects.md). Objects such as signals (traffic signs, traffic lights, ...) have a specified position in the OpenDrive map (for details see [source_1](https://www.asam.net/standards/detail/opendrive/) and [source_2](https://www.asam.net/index.php?eID=dumpFile&t=f&f=4422&token=e590561f3c39aa2260e5442e29e93f6693d1cccd#top-016f925e-bfe2-481d-b603-da4524d7491f) (menu point "12. Signals"), disclaimer: the second source belongs to a previous version of OpenDrive but is probably still applicable). +### Map-Based Localization Enhancement + +The localization accuracy can be improved by incorporating map data: + +1. Object Detection: + - Use vision and lidar nodes to detect and measure distance to objects + - See [distance_to_objects.md](./distance_to_objects.md) for implementation details + +2. Map Integration: + - OpenDrive map contains known positions of objects (traffic signs, lights, etc.) + - References: + - [OpenDRIVE Standard](https://www.asam.net/standards/detail/opendrive/) + - [Signal Specifications](https://www.asam.net/index.php?eID=dumpFile&t=f&f=4422&token=e590561f3c39aa2260e5442e29e93f6693d1cccd#top-016f925e-bfe2-481d-b603-da4524d7491f) (Section 12) + +3. Position Refinement: + - Match detected objects with map data + - Use relative distances to refine position estimates🧰 Tools
🪛 LanguageTool
[uncategorized] ~92-~92: Possible missing comma found.
Context: ... the vision node and the lidar distance node it is possible to calculate the distanc...(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~94-~94: Possible missing comma found.
Context: ...ance to a detected object. For a better understanding look at the following example: The car ...(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~95-~95: Possible missing comma found.
Context: ... next to the road with a distance of 20 meters this suggests that the vehicle is 30 me...(AI_HYDRA_LEO_MISSING_COMMA)
🪛 Markdownlint
92-92: Expected: 300; Actual: 809
Line length(MD013, line-length)
95-95: Expected: 300; Actual: 334
Line length(MD013, line-length)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
doc/perception/README.md(1 hunks)doc/perception/localization.md(1 hunks)
🧰 Additional context used
🪛 LanguageTool
doc/perception/README.md
[uncategorized] ~22-~22: The preposition “of” seems more likely in this position.
Context: ... ## Overview Localization An overview over the different nodes working together to...
(AI_EN_LECTOR_REPLACEMENT_PREPOSITION)
doc/perception/localization.md
[uncategorized] ~8-~8: Use a comma before ‘but’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...bug_node.md) node is another useful node but it does not actively localize the vehic...
(COMMA_COMPOUND_SENTENCE)
[uncategorized] ~8-~8: A comma may be missing after the conjunctive/linking adverb ‘Instead’.
Context: ...does not actively localize the vehicle. Instead it makes it possible to compare and tun...
(SENT_START_CONJUNCTIVE_LINKING_ADVERB_COMMA)
[typographical] ~28-~28: Consider adding a comma here.
Context: ...t is used (e.g. kalman_pos) As you can see the node first subscribes the filtered ...
(AS_YOU_CAN_SEE_COMMA)
[uncategorized] ~30-~30: Possible missing comma found.
Context: ...ossible for multiple filter nodes to be running and only the data produced by one filte...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~30-~30: A comma may be missing after the conjunctive/linking adverb ‘Otherwise’.
Context: ...shed as the current position / heading. Otherwise different filters would publish to the ...
(SENT_START_CONJUNCTIVE_LINKING_ADVERB_COMMA)
[uncategorized] ~46-~46: Possible missing comma found.
Context: ... purposes only) To use a certain / new filter two files need to be updated: - First ...
(AI_HYDRA_LEO_MISSING_COMMA)
[duplication] ~50-~50: Possible typo: you repeated a word
Context: ... code of the position_heading_publisher_node node - Then the according subscriber and pu...
(ENGLISH_WORD_REPEAT_RULE)
[duplication] ~54-~54: Possible typo: you repeated a word
Context: ...tails on the position_heading_publisher_node node click [here](./position_heading_publish...
(ENGLISH_WORD_REPEAT_RULE)
[uncategorized] ~58-~58: Use a comma before ‘so’ if it connects two independent clauses (unless they are closely connected and short).
Context: ... filtering the location and heading data so the noise can be eliminated / reduced. ...
(COMMA_COMPOUND_SENTENCE_2)
[uncategorized] ~61-~61: A comma may be missing after the conjunctive/linking adverb ‘Therefore’.
Context: ...ed. ### Published / subscribed topics Therefore the published topics are: - `kalman_pos...
(SENT_START_CONJUNCTIVE_LINKING_ADVERB_COMMA)
[uncategorized] ~80-~80: Possible missing comma found.
Context: ...orientation and angular velocity of the vehicle the data provided by the IMU (Inertial ...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~84-~84: Possible missing comma found.
Context: ... ### Possible improvements In earlier experiments it was shown that the Kalman Filter per...
(AI_HYDRA_LEO_MISSING_COMMA)
[style] ~84-~84: Consider an alternative to strengthen your wording.
Context: ...filtered data. But it seems likely that further improvements can be made. For further details on th...
(IMPROVEMENTS_REFINEMENTS)
[uncategorized] ~86-~86: Possible missing comma found.
Context: ...ent implementation of the kalman_filter node click here. Curr...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~87-~87: A comma may be missing after the conjunctive/linking adverb ‘Currently’.
Context: ...node click here. Currently the model assumes that the vehicle driv...
(SENT_START_CONJUNCTIVE_LINKING_ADVERB_COMMA)
[uncategorized] ~90-~90: Use a comma before ‘but’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...sume a normal distribution of the system but it is also the most complex Kalman Filt...
(COMMA_COMPOUND_SENTENCE)
[uncategorized] ~92-~92: Possible missing comma found.
Context: ... the vision node and the lidar distance node it is possible to calculate the distanc...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~94-~94: Possible missing comma found.
Context: ...ance to a detected object. For a better understanding look at the following example: The car ...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~95-~95: Possible missing comma found.
Context: ... next to the road with a distance of 20 meters this suggests that the vehicle is 30 me...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~99-~99: Possible missing comma found.
Context: ...useful helper functions such as quat_to_heading which transforms a given quaternion int...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~101-~101: Possible missing comma found.
Context: ...ode](./position_heading_publisher_node) node as well as the [kalman_filter](./kalman...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~107-~107: Use a comma before “and” if it connects two independent clauses (unless they are closely connected and short).
Context: ...performance of a filter can be evaluated and the parameters used by the filter can b...
(COMMA_COMPOUND_SENTENCE_2)
[uncategorized] ~109-~109: Possible missing comma found.
Context: ...ments/Position_Heading_Datasets/viz.py) file even though they can also be shown via ...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~111-~111: Possible missing comma found.
Context: ...Carla API and therefore uses the ground truth it should only be used for combaring an...
(AI_HYDRA_LEO_MISSING_COMMA)
[style] ~111-~111: Consider replacing ‘only’ with a different word to let your writing stand out.
Context: ...erefore uses the ground truth it should only be used for combaring and tuning filter...
(ONLY_EXCLUSIVELY_STYLE)
🪛 Markdownlint
doc/perception/localization.md
38-38: Expected: 2; Actual: 4
Unordered list indentation
(MD007, ul-indent)
39-39: Expected: 2; Actual: 4
Unordered list indentation
(MD007, ul-indent)
40-40: Expected: 2; Actual: 4
Unordered list indentation
(MD007, ul-indent)
42-42: Expected: 2; Actual: 4
Unordered list indentation
(MD007, ul-indent)
43-43: Expected: 2; Actual: 4
Unordered list indentation
(MD007, ul-indent)
44-44: Expected: 2; Actual: 4
Unordered list indentation
(MD007, ul-indent)
49-49: Expected: 2; Actual: 4
Unordered list indentation
(MD007, ul-indent)
50-50: Expected: 2; Actual: 4
Unordered list indentation
(MD007, ul-indent)
8-8: Expected: 300; Actual: 367
Line length
(MD013, line-length)
92-92: Expected: 300; Actual: 809
Line length
(MD013, line-length)
95-95: Expected: 300; Actual: 334
Line length
(MD013, line-length)
107-107: Expected: 300; Actual: 425
Line length
(MD013, line-length)
60-60: null
Multiple headings with the same content
(MD024, no-duplicate-heading)
4-4: null
Lists should be surrounded by blank lines
(MD032, blanks-around-lists)
17-17: null
Lists should be surrounded by blank lines
(MD032, blanks-around-lists)
23-23: null
Lists should be surrounded by blank lines
(MD032, blanks-around-lists)
37-37: null
Lists should be surrounded by blank lines
(MD032, blanks-around-lists)
63-63: null
Lists should be surrounded by blank lines
(MD032, blanks-around-lists)
67-67: null
Lists should be surrounded by blank lines
(MD032, blanks-around-lists)
113-113: null
Files should end with a single newline character
(MD047, single-trailing-newline)
🔇 Additional comments (1)
doc/perception/README.md (1)
20-21: LGTM! Section placement is appropriate.
The new "Overview Localization" section is well-placed in the documentation structure, following the individual component descriptions and before the experiments section.
| ## coordinate_transformation | ||
|
|
||
| The [coordinate_transformation](./coordinate_transformation) node provides useful helper functions such as quat_to_heading which transforms a given quaternion into the heading of the car. | ||
|
|
||
| This node is used by the [position_heading_publisher_node](./position_heading_publisher_node) node as well as the [kalman_filter](./kalman_filter) node. Both nodes use the node for its quat_to_heading function and its CoordinateTransformer class. | ||
|
|
||
| The node is not fully documented yet but for further details on the quat_to_heading function click [here](./coordinate_transformation.md). | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Expand coordinate transformation documentation.
The coordinate transformation section needs more details about:
- Coordinate systems used (e.g., local vs. global)
- Transformation equations or algorithms
- Quaternion to heading conversion method
## coordinate_transformation
+### Coordinate Systems
+
+The system uses the following coordinate frames:
+- Global frame (map coordinates)
+- Local frame (vehicle coordinates)
+- Sensor frames (GPS, IMU)
+
+### Transformations
+
+#### Quaternion to Heading
The [coordinate_transformation](./coordinate_transformation) node provides useful helper functions such as quat_to_heading which transforms a given quaternion into the heading of the car.
+
+The heading is calculated as follows:
+1. Extract Euler angles from quaternion
+2. Use yaw angle (rotation around z-axis) as heading
+3. Normalize to range [-π, π]📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ## coordinate_transformation | |
| The [coordinate_transformation](./coordinate_transformation) node provides useful helper functions such as quat_to_heading which transforms a given quaternion into the heading of the car. | |
| This node is used by the [position_heading_publisher_node](./position_heading_publisher_node) node as well as the [kalman_filter](./kalman_filter) node. Both nodes use the node for its quat_to_heading function and its CoordinateTransformer class. | |
| The node is not fully documented yet but for further details on the quat_to_heading function click [here](./coordinate_transformation.md). | |
| ## coordinate_transformation | |
| ### Coordinate Systems | |
| The system uses the following coordinate frames: | |
| - Global frame (map coordinates) | |
| - Local frame (vehicle coordinates) | |
| - Sensor frames (GPS, IMU) | |
| ### Transformations | |
| #### Quaternion to Heading | |
| The [coordinate_transformation](./coordinate_transformation) node provides useful helper functions such as quat_to_heading which transforms a given quaternion into the heading of the car. | |
| The heading is calculated as follows: | |
| 1. Extract Euler angles from quaternion | |
| 2. Use yaw angle (rotation around z-axis) as heading | |
| 3. Normalize to range [-π, π] | |
| This node is used by the [position_heading_publisher_node](./position_heading_publisher_node) node as well as the [kalman_filter](./kalman_filter) node. Both nodes use the node for its quat_to_heading function and its CoordinateTransformer class. | |
| The node is not fully documented yet but for further details on the quat_to_heading function click [here](./coordinate_transformation.md). |
🧰 Tools
🪛 LanguageTool
[uncategorized] ~99-~99: Possible missing comma found.
Context: ...useful helper functions such as quat_to_heading which transforms a given quaternion int...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~101-~101: Possible missing comma found.
Context: ...ode](./position_heading_publisher_node) node as well as the [kalman_filter](./kalman...
(AI_HYDRA_LEO_MISSING_COMMA)
doc/perception/localization.md
Outdated
| ## position_heading_filter_debug_node | ||
|
|
||
| This node processes the data provided by the IMU and GNSS so the errors between the is-state and the measured state can be seen. To get the is-state the Carla API is used to get the real position and heading of the car. Comparing the real position / heading with the position / heading estimated by a filter (e.g. Kalman Filter) the performance of a filter can be evaluated and the parameters used by the filter can be tuned. | ||
|
|
||
| The recommended way to look at the results is using the mathplotlib plots provided by the [viz.py](../../code/perception/src/experiments/Position_Heading_Datasets/viz.py) file even though they can also be shown via rqt_plots. | ||
|
|
||
| Because the node uses the Carla API and therefore uses the ground truth it should only be used for combaring and tuning filters and not for any other purposes. It might be best to remove this node before submitting to the official leaderboard because otherwise the project could get disqualified. | ||
|
|
||
| For more details on the node see [position_heading_filter_debug_node](./position_heading_filter_debug_node.md) and [viz.py](../../code/perception/src/experiments/Position_Heading_Datasets/viz.py). |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Make competition warning more prominent and fix typo.
The warning about using ground truth in competition should be highlighted, and there's a typo in "combaring".
## position_heading_filter_debug_node
+⚠️ **Competition Warning**: This node uses ground truth data from the Carla API and should NOT be used in competition submissions to avoid disqualification.
+
This node processes the data provided by the IMU and GNSS so the errors between the is-state and the measured state can be seen. To get the is-state the Carla API is used to get the real position and heading of the car. Comparing the real position / heading with the position / heading estimated by a filter (e.g. Kalman Filter) the performance of a filter can be evaluated and the parameters used by the filter can be tuned.
The recommended way to look at the results is using the mathplotlib plots provided by the [viz.py](../../code/perception/src/experiments/Position_Heading_Datasets/viz.py) file even though they can also be shown via rqt_plots.
-Because the node uses the Carla API and therefore uses the ground truth it should only be used for combaring and tuning filters and not for any other purposes. It might be best to remove this node before submitting to the official leaderboard because otherwise the project could get disqualified.
+Because the node uses the Carla API and therefore uses the ground truth, it should only be used for comparing and tuning filters and not for any other purposes. Remove this node before submitting to the official leaderboard to avoid disqualification.
For more details on the node see [position_heading_filter_debug_node](./position_heading_filter_debug_node.md) and [viz.py](../../code/perception/src/experiments/Position_Heading_Datasets/viz.py).📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ## position_heading_filter_debug_node | |
| This node processes the data provided by the IMU and GNSS so the errors between the is-state and the measured state can be seen. To get the is-state the Carla API is used to get the real position and heading of the car. Comparing the real position / heading with the position / heading estimated by a filter (e.g. Kalman Filter) the performance of a filter can be evaluated and the parameters used by the filter can be tuned. | |
| The recommended way to look at the results is using the mathplotlib plots provided by the [viz.py](../../code/perception/src/experiments/Position_Heading_Datasets/viz.py) file even though they can also be shown via rqt_plots. | |
| Because the node uses the Carla API and therefore uses the ground truth it should only be used for combaring and tuning filters and not for any other purposes. It might be best to remove this node before submitting to the official leaderboard because otherwise the project could get disqualified. | |
| For more details on the node see [position_heading_filter_debug_node](./position_heading_filter_debug_node.md) and [viz.py](../../code/perception/src/experiments/Position_Heading_Datasets/viz.py). | |
| ## position_heading_filter_debug_node | |
| ⚠️ **Competition Warning**: This node uses ground truth data from the Carla API and should NOT be used in competition submissions to avoid disqualification. | |
| This node processes the data provided by the IMU and GNSS so the errors between the is-state and the measured state can be seen. To get the is-state the Carla API is used to get the real position and heading of the car. Comparing the real position / heading with the position / heading estimated by a filter (e.g. Kalman Filter) the performance of a filter can be evaluated and the parameters used by the filter can be tuned. | |
| The recommended way to look at the results is using the mathplotlib plots provided by the [viz.py](../../code/perception/src/experiments/Position_Heading_Datasets/viz.py) file even though they can also be shown via rqt_plots. | |
| Because the node uses the Carla API and therefore uses the ground truth, it should only be used for comparing and tuning filters and not for any other purposes. Remove this node before submitting to the official leaderboard to avoid disqualification. | |
| For more details on the node see [position_heading_filter_debug_node](./position_heading_filter_debug_node.md) and [viz.py](../../code/perception/src/experiments/Position_Heading_Datasets/viz.py). |
🧰 Tools
🪛 LanguageTool
[uncategorized] ~107-~107: Use a comma before “and” if it connects two independent clauses (unless they are closely connected and short).
Context: ...performance of a filter can be evaluated and the parameters used by the filter can b...
(COMMA_COMPOUND_SENTENCE_2)
[uncategorized] ~109-~109: Possible missing comma found.
Context: ...ments/Position_Heading_Datasets/viz.py) file even though they can also be shown via ...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~111-~111: Possible missing comma found.
Context: ...Carla API and therefore uses the ground truth it should only be used for combaring an...
(AI_HYDRA_LEO_MISSING_COMMA)
[style] ~111-~111: Consider replacing ‘only’ with a different word to let your writing stand out.
Context: ...erefore uses the ground truth it should only be used for combaring and tuning filter...
(ONLY_EXCLUSIVELY_STYLE)
🪛 Markdownlint
107-107: Expected: 300; Actual: 425
Line length
(MD013, line-length)
113-113: null
Files should end with a single newline character
(MD047, single-trailing-newline)
Toni2go
force-pushed
the
370-localization-with-gps
branch
from
30c3e2b
to
63a47c5
Compare
Toni2go
force-pushed
the
370-localization-with-gps
branch
from
63a47c5
to
46c96d2
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (3)
doc/perception/localization.md (3)
51-58: Enhance filter configuration documentation.The filter configuration process needs additional information:
- Error handling for misconfigured filters
- Impact of filter selection on localization accuracy
- Validation steps after configuration changes
🧰 Tools
🪛 LanguageTool
[uncategorized] ~51-~51: Possible missing comma found.
Context: ... purposes only) To use a certain / new filter two files need to be updated: - First ...(AI_HYDRA_LEO_MISSING_COMMA)
[duplication] ~55-~55: Possible typo: you repeated a word
Context: ... code of the position_heading_publisher_node node - Then the according subscriber and pu...(ENGLISH_WORD_REPEAT_RULE)
81-82: Document z-position calculation details.The documentation states that z-position uses rolling average but doesn't explain:
- Why Kalman filter isn't used for z-position
- The rolling average window size
- Accuracy implications of this approach
100-111: Expand vision-based improvement proposal.The vision-based localization improvement suggestion needs:
- Implementation approach
- Required changes to existing nodes
- Expected accuracy improvements
- Challenges and limitations
🧰 Tools
🪛 LanguageTool
[uncategorized] ~101-~101: Possible missing comma found.
Context: ... the vision node and the lidar distance node it is possible to calculate the distanc...(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~108-~108: Possible missing comma found.
Context: ...ance to a detected object. For a better understanding look at the following example: The car ...(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~110-~110: Possible missing comma found.
Context: ... next to the road with a distance of 20 meters this suggests that the vehicle is 30 me...(AI_HYDRA_LEO_MISSING_COMMA)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
doc/perception/README.md(1 hunks)doc/perception/localization.md(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- doc/perception/README.md
🧰 Additional context used
🪛 LanguageTool
doc/perception/localization.md
[uncategorized] ~9-~9: Use a comma before ‘but’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...bug_node.md) node is another useful node but it does not actively localize the vehic...
(COMMA_COMPOUND_SENTENCE)
[uncategorized] ~9-~9: A comma may be missing after the conjunctive/linking adverb ‘Instead’.
Context: ...does not actively localize the vehicle. Instead it makes it possible to compare and tun...
(SENT_START_CONJUNCTIVE_LINKING_ADVERB_COMMA)
[uncategorized] ~25-~25: Possible missing comma found.
Context: ...he necessary information for the topics above the node subscribes the following topic...
(AI_HYDRA_LEO_MISSING_COMMA)
[typographical] ~32-~32: Consider adding a comma here.
Context: ...t is used (e.g. kalman_pos) As you can see the node first subscribes the filtered ...
(AS_YOU_CAN_SEE_COMMA)
[uncategorized] ~34-~34: Possible missing comma found.
Context: ...ossible for multiple filter nodes to be running and only the data produced by one filte...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~34-~34: A comma may be missing after the conjunctive/linking adverb ‘Otherwise’.
Context: ...shed as the current position / heading. Otherwise different filters would publish to the ...
(SENT_START_CONJUNCTIVE_LINKING_ADVERB_COMMA)
[uncategorized] ~51-~51: Possible missing comma found.
Context: ... purposes only) To use a certain / new filter two files need to be updated: - First ...
(AI_HYDRA_LEO_MISSING_COMMA)
[duplication] ~55-~55: Possible typo: you repeated a word
Context: ... code of the position_heading_publisher_node node - Then the according subscriber and pu...
(ENGLISH_WORD_REPEAT_RULE)
[duplication] ~59-~59: Possible typo: you repeated a word
Context: ...tails on the position_heading_publisher_node node click [here](./position_heading_publish...
(ENGLISH_WORD_REPEAT_RULE)
[uncategorized] ~63-~63: Use a comma before ‘so’ if it connects two independent clauses (unless they are closely connected and short).
Context: ... filtering the location and heading data so the noise can be eliminated / reduced. ...
(COMMA_COMPOUND_SENTENCE_2)
[uncategorized] ~66-~66: A comma may be missing after the conjunctive/linking adverb ‘Therefore’.
Context: ...ed. ### Published / subscribed topics Therefore the published topics are: - `kalman_po...
(SENT_START_CONJUNCTIVE_LINKING_ADVERB_COMMA)
[uncategorized] ~91-~91: Possible missing comma found.
Context: ... ### Possible improvements In earlier experiments it was shown that the Kalman Filter per...
(AI_HYDRA_LEO_MISSING_COMMA)
[style] ~91-~91: Consider an alternative to strengthen your wording.
Context: ...filtered data. But it seems likely that further improvements can be made. For further details on th...
(IMPROVEMENTS_REFINEMENTS)
[uncategorized] ~93-~93: Possible missing comma found.
Context: ...ent implementation of the kalman_filter node click here. Curr...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~94-~94: A comma may be missing after the conjunctive/linking adverb ‘Currently’.
Context: ...node click here. Currently the model assumes that the vehicle driv...
(SENT_START_CONJUNCTIVE_LINKING_ADVERB_COMMA)
[uncategorized] ~98-~98: Use a comma before ‘but’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...sume a normal distribution of the system but it is also the most complex Kalman Filt...
(COMMA_COMPOUND_SENTENCE)
[uncategorized] ~101-~101: Possible missing comma found.
Context: ... the vision node and the lidar distance node it is possible to calculate the distanc...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~108-~108: Possible missing comma found.
Context: ...ance to a detected object. For a better understanding look at the following example: The car ...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~110-~110: Possible missing comma found.
Context: ... next to the road with a distance of 20 meters this suggests that the vehicle is 30 me...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~115-~115: Possible missing comma found.
Context: ...useful helper functions such as quat_to_heading which transforms a given quaternion int...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~117-~117: Possible missing comma found.
Context: ...ode](./position_heading_publisher_node) node as well as the [kalman_filter](./kalman...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~125-~125: Use a comma before “and” if it connects two independent clauses (unless they are closely connected and short).
Context: ...performance of a filter can be evaluated and the parameters used by the filter can b...
(COMMA_COMPOUND_SENTENCE_2)
[uncategorized] ~127-~127: Possible missing comma found.
Context: ...ments/Position_Heading_Datasets/viz.py) file even though they can also be shown via ...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~129-~129: Possible missing comma found.
Context: ...Carla API and therefore uses the ground truth it should only be used for combaring an...
(AI_HYDRA_LEO_MISSING_COMMA)
[style] ~129-~129: Consider replacing ‘only’ with a different word to let your writing stand out.
Context: ...erefore uses the ground truth it should only be used for combaring and tuning filter...
(ONLY_EXCLUSIVELY_STYLE)
🪛 Markdownlint
doc/perception/localization.md
65-65: null
Multiple headings with the same content
(MD024, no-duplicate-heading)
| # Localization | ||
|
|
||
| There are currently three nodes working together to localize the vehicle: | ||
|
|
||
| - [position_heading_publisher_node](./position_heading_publisher_node.md) | ||
| - [kalman_filter](./kalman_filter.md) | ||
| - [coordinate_transformation](./coordinate_transformation.md) | ||
|
|
||
| The [position_heading_filter_debug_node](./position_heading_filter_debug_node.md) node is another useful node but it does not actively localize the vehicle. | ||
| Instead it makes it possible to compare and tune different filters and the [viz.py](../../code/perception/src/experiments/Position_Heading_Datasets/viz.py) file is the recommended way to visualizes the results. | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add context about the overall localization strategy.
Given that this PR addresses questions about GPS reliance (Issue #370), consider adding:
- An overview of the current localization strategy
- Rationale for using these specific nodes
- Limitations of the current approach
- Future plans for sensor fusion
🧰 Tools
🪛 LanguageTool
[uncategorized] ~9-~9: Use a comma before ‘but’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...bug_node.md) node is another useful node but it does not actively localize the vehic...
(COMMA_COMPOUND_SENTENCE)
[uncategorized] ~9-~9: A comma may be missing after the conjunctive/linking adverb ‘Instead’.
Context: ...does not actively localize the vehicle. Instead it makes it possible to compare and tun...
(SENT_START_CONJUNCTIVE_LINKING_ADVERB_COMMA)
| - Kalman (Kalman Filter) | ||
| - None (No Filter) | ||
| - Old (heading is calculated the WRONG way, for demonstration purposes only) | ||
|
|
There was a problem hiding this comment.
Clarify the "Old" heading filter warning.
The documentation marks this filter as "WRONG" but doesn't explain:
- Why it's wrong
- What issues it causes
- Why it's still included
Consider removing this option to prevent accidental usage.
Description
An overview of the nodes that work together to localize the vehicle is added. There are also some suggestions on how to improve the currently used filter.
Fixes #370
Does this PR introduce a breaking change?
no
Checklist:
Summary by CodeRabbit
New Features
Documentation