-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
553 feature document behaviour tree and behaviours #566
553 feature document behaviour tree and behaviours #566
Conversation
added new behaviour tree doc assets
…stuck_Behavior.md
Updated comments and log messages for better understanding/debugging. No logic changes.
Refined comments and log messages, this does not change any logic
Improved comments and logging messages, no logic changes
WalkthroughThe pull request introduces significant modifications across several files related to vehicle behavior, particularly focusing on intersection, lane change, and overtaking maneuvers. Enhancements include improved logging, refined control flow, and detailed documentation for the Changes
Possibly related PRs
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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 14
🧹 Outside diff range and nitpick comments (22)
doc/planning/behaviours/LeaveParkingSpace.md (3)
1-7
: Fix formatting issues in the table of contents sectionThe table of contents structure has a trailing space on line 7 that should be removed.
- - [General](#general) + - [General](#general)🧰 Tools
🪛 Markdownlint (0.37.0)
7-7: Expected: 0 or 2; Actual: 1
Trailing spaces(MD009, no-trailing-spaces)
10-13
: Enhance readability with proper punctuation and formattingThe description has some grammatical and formatting issues that should be addressed for better clarity.
-The leave parking space behaviour is only executed at the beginning of the simulation to leave the parking space. +The Leave Parking Space behavior is only executed at the beginning of the simulation to leave the parking space. -The behaviour calculates the euclidian distance between the starting position and the current position to determine whether the car has fully left the parking space. If that is the case a "called" -flag is set to true so that this behaviour is never executed again and FAILURE is returned to end the behaviour. Otherwise it stays in RUNNING. +The behavior calculates the Euclidean distance between the starting position and the current position to determine whether the car has fully left the parking space. If that is the case, a "called" +flag is set to true so that this behavior is never executed again and FAILURE is returned to end the behavior. Otherwise, it stays in RUNNING.🧰 Tools
🪛 LanguageTool
[uncategorized] ~12-~12: A comma might be missing here.
Context: ... left the parking space. If that is the case a "called" flag is set to true so that ...(AI_EN_LECTOR_MISSING_PUNCTUATION_COMMA)
[uncategorized] ~13-~13: A comma may be missing after the conjunctive/linking adverb ‘Otherwise’.
Context: ...ILURE is returned to end the behaviour. Otherwise it stays in RUNNING.(SENT_START_CONJUNCTIVE_LINKING_ADVERB_COMMA)
1-13
: Consider enhancing documentation with additional sectionsThe current documentation provides a good overview but could benefit from additional sections to make it more comprehensive.
Consider adding the following sections:
- Parameters - Document any configurable parameters (e.g., distance threshold)
- Return Values - Clearly document the FAILURE and RUNNING states
- Example Usage - Provide a simple example of how this behavior is used
- Dependencies - List any dependencies or prerequisites
- Edge Cases - Document how the behavior handles edge cases
Would you like me to help generate a template for these additional sections?
🧰 Tools
🪛 LanguageTool
[uncategorized] ~12-~12: A comma might be missing here.
Context: ... left the parking space. If that is the case a "called" flag is set to true so that ...(AI_EN_LECTOR_MISSING_PUNCTUATION_COMMA)
[uncategorized] ~13-~13: A comma may be missing after the conjunctive/linking adverb ‘Otherwise’.
Context: ...ILURE is returned to end the behaviour. Otherwise it stays in RUNNING.(SENT_START_CONJUNCTIVE_LINKING_ADVERB_COMMA)
🪛 Markdownlint (0.37.0)
7-7: Expected: 0 or 2; Actual: 1
Trailing spaces(MD009, no-trailing-spaces)
doc/planning/behaviours/LaneChange.md (2)
1-12
: Enhance the summary section for better clarity.The summary could be more descriptive by including:
- The purpose of lane changes
- When lane changes are triggered
- Key components or prerequisites
Also, remove the trailing space on line 12 to fix the markdown linting issue.
🧰 Tools
🪛 Markdownlint (0.37.0)
12-12: Expected: 0 or 2; Actual: 1
Trailing spaces(MD009, no-trailing-spaces)
1-39
: Consider adding essential documentation sections.To make the documentation more comprehensive, consider adding sections about:
Error Handling & Recovery
- Common failure scenarios
- Recovery procedures
- Error messages and troubleshooting
Edge Cases
- Emergency vehicles
- Road work zones
- Weather conditions
- System failures
Testing & Validation
- Integration test scenarios
- Validation procedures
- Performance metrics
🧰 Tools
🪛 LanguageTool
[uncategorized] ~27-~27: A comma might be missing here.
Context: ...stance of the virtual stop line and not blocked it returns SUCCESS. SUCCESS is also ret...(AI_EN_LECTOR_MISSING_PUNCTUATION_COMMA)
[uncategorized] ~35-~35: The preposition “at” seems more likely in this position.
Context: .... ## Enter Inititates the lane change with 20 km/h and continues driving on the ne...(AI_EN_LECTOR_REPLACEMENT_PREPOSITION)
[uncategorized] ~35-~35: The preposition “in” seems more likely in this position than the preposition “on”.
Context: ...ange with 20 km/h and continues driving on the next lane until the lane change way...(AI_EN_LECTOR_REPLACEMENT_PREPOSITION_ON_IN)
🪛 Markdownlint (0.37.0)
12-12: Expected: 0 or 2; Actual: 1
Trailing spaces(MD009, no-trailing-spaces)
code/planning/src/behavior_agent/behaviours/overtake.py (4)
78-80
: Improve docstring formatting and clarityThe docstring formatting could be improved to better follow Python conventions.
- Gets the current distance to overtake, the current oncoming lane status and the - distance to collsion object. Slows down while oncoming blocked until stopped - or oncoming clear. + Gets the current distance to overtake, the current oncoming lane status and the + distance to collision object. Slows down when oncoming lane is blocked until + either stopped or oncoming lane is clear.
114-114
: Enhance logging messages with more contextThe logging messages could be more informative by including specific values.
- rospy.loginfo("Overtake Approach: oncoming blocked slowing down") + rospy.loginfo(f"Overtake Approach: oncoming blocked (distance: {distance_oncoming}m), slowing down") - rospy.loginfo( - f"Overtake Approach: still approaching obstacle, " - f"distance: {self.ot_distance}" - ) + rospy.loginfo(f"Overtake Approach: approaching obstacle at {self.ot_distance:.1f}m") - rospy.loginfo("Overtake Approach: stopped behind obstacle") + rospy.loginfo(f"Overtake Approach: stopped behind obstacle at {self.ot_distance:.1f}m")Also applies to: 127-130, 134-134
321-324
: Enhance docstring and logging with more detailsThe docstring lacks specifics about return values, and logging messages could be more informative.
- Waits for motion_planner to finish the new trajectory. - :return: py_trees.common.Status.RUNNING, - py_trees.common.Status.SUCCESS, - py_trees.common.Status.FAILURE, + Waits for motion_planner to finish planning the overtaking trajectory. + :return: py_trees.common.Status.RUNNING: Still waiting for trajectory planning + py_trees.common.Status.SUCCESS: Trajectory successfully planned + py_trees.common.Status.FAILURE: Trajectory planning failed - rospy.loginfo("Overtake Enter: Trajectory planned") + rospy.loginfo("Enter: Overtaking trajectory successfully planned") - rospy.loginfo("Overtake Enter: Slowing down") + rospy.loginfo("Enter: Slowing down while planning trajectory") - rospy.loginfo("Overtake Enter: Abort") + rospy.loginfo("Enter: Aborting overtake - trajectory planning failed") - rospy.loginfo("Overtake Enter: Waiting for status update") + rospy.loginfo("Enter: Waiting for trajectory planning status")Also applies to: 329-329, 333-333, 336-336, 339-339
Line range hint
1-441
: Consider architectural improvements for better maintainabilityWhile the documentation improvements are good, there are several architectural improvements that could be made:
- Consider creating a dedicated logging utility class to standardize log message formats and levels across all behaviors
- Move configuration values (like distances and speeds) to a central configuration file
- Consider using a state management pattern instead of global variables
- Add type hints to improve code maintainability
Would you like assistance in implementing any of these architectural improvements?
code/planning/src/behavior_agent/behaviours/lane_change.py (4)
95-103
: Consider implementing the traffic check featureThe TODO comment indicates a placeholder for traffic checking functionality. Currently, there's a hardcoded distance value (20) being used.
Would you like help implementing a proper traffic check feature using LIDAR data? This could include:
- Proper sensor data processing
- Vehicle detection logic
- Safe distance calculations based on current speed
Line range hint
226-241
: Refactor duplicated traffic check logicThe traffic check logic is duplicated between the
Approach
andWait
classes, using the same hardcoded distance values.Consider extracting this into a shared utility function:
+def is_lane_change_clear(distance_lidar, min_safe_distance=15.0): + """ + Checks if it's safe to perform a lane change based on LIDAR distance. + + Args: + distance_lidar: Distance reading from LIDAR + min_safe_distance: Minimum safe distance to proceed (default: 15.0) + + Returns: + bool: True if lane change is clear, False otherwise + """ + return distance_lidar is not None and distance_lidar > min_safe_distanceThen use this in both classes:
if is_lane_change_clear(distance_lidar): rospy.loginfo("Lane Change is free not slowing down!") # ... rest of the logic
389-392
: Consider adding more detailed exit loggingThe current logging message could be more informative about the lane change outcome.
Consider enhancing the logging to include relevant metrics:
- rospy.loginfo("Lane Change Finished") + rospy.loginfo("Lane Change Finished - Duration: %.2fs, Final Position: (%.2f, %.2f)", + (rospy.Time.now() - self._start_time).to_sec(), + self.blackboard.get("/carla/hero/odometry").pose.pose.position.x, + self.blackboard.get("/carla/hero/odometry").pose.pose.position.y)
Line range hint
1-424
: Consider architectural improvements for behavior implementationsThe current implementation shows several patterns that could be improved:
- Common behavior patterns (setup, initialization, termination) are duplicated across classes
- Hardcoded distance values are scattered throughout the code
- Logging patterns are inconsistent
Consider these improvements:
- Create a base behavior class that implements common patterns
- Move configuration values to a central configuration file
- Implement a structured logging format for better debugging and monitoring
Example base class:
class BaseBehavior(py_trees.behaviour.Behaviour): def __init__(self, name, config): super().__init__(name) self.config = config self.blackboard = None self._start_time = None def setup(self, timeout): self.curr_behavior_pub = rospy.Publisher( "/paf/hero/curr_behavior", String, queue_size=1 ) self.blackboard = py_trees.blackboard.Blackboard() return True def initialise(self): self._start_time = rospy.Time.now() self._log_state("Started") def terminate(self, new_status): self._log_state(f"Terminated [{self.status} -> {new_status}]") def _log_state(self, message): rospy.loginfo(f"{self.__class__.__name__}: {message}")doc/planning/behaviours/Overtake.md (2)
1-4
: Enhance the introduction with more contextThe summary could be more informative by including:
- The behavior's role in the overall system
- Prerequisites or dependencies
- Expected inputs and outputs
# Overtake Behavior -**Summary:** This file explains the Overtake behavior. +**Summary:** This file explains the Overtake behavior, which enables autonomous vehicles to safely pass slower or stationary objects ahead. The behavior is part of the vehicle's decision-making system and coordinates with the motion planner to execute lane changes safely. + +**Prerequisites:** +- Motion planner integration +- Object detection capability +- Lane detection system
1-45
: Consider adding technical implementation detailsTo make this documentation more valuable for developers:
- Add code examples showing typical usage
- Document message formats for motion planner communication
- Include sequence diagrams for state transitions
- Add error handling and edge cases
- Include configuration parameters and their effects
Would you like me to help generate these additional sections?
🧰 Tools
🪛 LanguageTool
[style] ~15-~15: This phrase is a bit redundant. Consider using just “proximity”.
Context: ...aviour is used to overtake an object in close proximity. This behaviour is currently not workin...(CLOSE_SCRUTINY)
[misspelling] ~15-~15: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ... is currently not working and more like a initial prototype. ## Overtake ahead ...(EN_A_VS_AN)
[misspelling] ~19-~19: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ...Overtake ahead Checks whether there is a object in front of the car that needs o...(EN_A_VS_AN)
[uncategorized] ~27-~27: A comma may be missing after the conjunctive/linking adverb ‘Otherwise’.
Context: ...the motion_planner and returns SUCCESS. Otherwise ot_app_blocked is published for the car...(SENT_START_CONJUNCTIVE_LINKING_ADVERB_COMMA)
[uncategorized] ~33-~33: The preposition ‘to’ seems more likely in this position.
Context: ...returned. ## Wait This handles wating for clear oncoming traffic if the car has s...(AI_HYDRA_LEO_REPLACE_FOR_TO)
[uncategorized] ~33-~33: A comma may be missing after the conjunctive/linking adverb ‘Otherwise’.
Context: ...gets published and SUCCESS is returned. Otherwise ot_wait_stopped gets published and the ...(SENT_START_CONJUNCTIVE_LINKING_ADVERB_COMMA)
[uncategorized] ~33-~33: Use a comma before “and” if it connects two independent clauses (unless they are closely connected and short).
Context: ...Otherwise ot_wait_stopped gets published and the behaviour stays in RUNNING. If the...(COMMA_COMPOUND_SENTENCE_2)
🪛 Markdownlint (0.37.0)
12-12: Expected: 0 or 2; Actual: 1
Trailing spaces(MD009, no-trailing-spaces)
19-19: Expected: 0 or 2; Actual: 1
Trailing spaces(MD009, no-trailing-spaces)
code/planning/src/behavior_agent/behaviours/intersection.py (2)
100-102
: Typo in docstring: Correct 'Calcualtes' to 'Calculates'In the docstring of the
update
method in theApproach
class, there is a typo on line 100. Please correct "Calcualtes" to "Calculates" for clarity and professionalism.
Line range hint
288-293
: Incomplete implementation: Intersection clearance check needs to be implementedIn the
Wait
class, the logic for checking if the intersection is clear is currently incomplete. The variablelidar_data
is set toNone
, so the conditionif lidar_data is not None:
will always evaluate toFalse
, andintersection_clear
remainsTrue
by default. This may lead to the vehicle proceeding through an obstructed intersection.Would you like assistance in implementing the intersection clearance check? I can help generate the necessary code or open a GitHub issue to track this task.
doc/planning/behaviours/Intersection.md (4)
12-12
: Remove trailing spaces on line 12Line 12 contains trailing spaces which should be removed to adhere to Markdown styling guidelines and maintain consistency in the document formatting.
🧰 Tools
🪛 Markdownlint (0.37.0)
12-12: Expected: 0 or 2; Actual: 1
Trailing spaces(MD009, no-trailing-spaces)
15-15
: Grammar correction: 'a intersection' should be 'an intersection'In line 15, the phrase "when it encounters a intersection" should be corrected to "when it encounters an intersection" to follow standard English grammar rules.
🧰 Tools
🪛 LanguageTool
[misspelling] ~15-~15: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ... control the vehicle when it encounters a intersection. It handles stop signs as ...(EN_A_VS_AN)
31-31
: Preposition correction: 'approached with 30 km/h' should be 'approached at 30 km/h'In line 31, "A green light is approached with 30 km/h" should be corrected to "A green light is approached at 30 km/h" for proper use of the preposition and clearer understanding.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~31-~31: The preposition “at” seems more likely in this position.
Context: ... velocity. A green light is approached with 30 km/h. ## Wait Handles wating at th...(AI_EN_LECTOR_REPLACEMENT_PREPOSITION)
35-35
: Spelling correction: 'wating' should be 'waiting'In line 35, there's a misspelling. Please correct "wating" to "waiting" in "Handles waiting at the stop line until the vehicle is allowed to drive."
doc/planning/Behavior_tree.md (1)
Line range hint
49-51
: Consistency: Use 'FAILURE' instead of 'Failure' for action return valuesIn the list of action return values under the "Behavior" section, the third item should be '
FAILURE
: The action couldn't be executed.' This ensures consistency with the capitalization used forSUCCESS
andRUNNING
.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
doc/assets/planning/behaviour_tree.PNG
is excluded by!**/*.png
📒 Files selected for processing (10)
code/planning/src/behavior_agent/behaviours/intersection.py
(8 hunks)code/planning/src/behavior_agent/behaviours/lane_change.py
(13 hunks)code/planning/src/behavior_agent/behaviours/overtake.py
(8 hunks)doc/assets/planning/behaviour_tree.drawio.xml
(1 hunks)doc/planning/Behavior_tree.md
(1 hunks)doc/planning/README.md
(1 hunks)doc/planning/behaviours/Intersection.md
(1 hunks)doc/planning/behaviours/LaneChange.md
(1 hunks)doc/planning/behaviours/LeaveParkingSpace.md
(1 hunks)doc/planning/behaviours/Overtake.md
(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- doc/planning/README.md
- doc/assets/planning/behaviour_tree.drawio.xml
🧰 Additional context used
🪛 LanguageTool
doc/planning/behaviours/Intersection.md
[misspelling] ~15-~15: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ... control the vehicle when it encounters a intersection. It handles stop signs as ...
(EN_A_VS_AN)
[uncategorized] ~31-~31: The preposition “at” seems more likely in this position.
Context: ... velocity. A green light is approached with 30 km/h. ## Wait Handles wating at th...
(AI_EN_LECTOR_REPLACEMENT_PREPOSITION)
doc/planning/behaviours/LaneChange.md
[uncategorized] ~27-~27: A comma might be missing here.
Context: ...stance of the virtual stop line and not blocked it returns SUCCESS. SUCCESS is also ret...
(AI_EN_LECTOR_MISSING_PUNCTUATION_COMMA)
[uncategorized] ~35-~35: The preposition “at” seems more likely in this position.
Context: .... ## Enter Inititates the lane change with 20 km/h and continues driving on the ne...
(AI_EN_LECTOR_REPLACEMENT_PREPOSITION)
[uncategorized] ~35-~35: The preposition “in” seems more likely in this position than the preposition “on”.
Context: ...ange with 20 km/h and continues driving on the next lane until the lane change way...
(AI_EN_LECTOR_REPLACEMENT_PREPOSITION_ON_IN)
doc/planning/behaviours/Overtake.md
[style] ~15-~15: This phrase is a bit redundant. Consider using just “proximity”.
Context: ...aviour is used to overtake an object in close proximity. This behaviour is currently not workin...
(CLOSE_SCRUTINY)
[misspelling] ~15-~15: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ... is currently not working and more like a initial prototype. ## Overtake ahead ...
(EN_A_VS_AN)
[misspelling] ~19-~19: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ...Overtake ahead Checks whether there is a object in front of the car that needs o...
(EN_A_VS_AN)
[uncategorized] ~27-~27: A comma may be missing after the conjunctive/linking adverb ‘Otherwise’.
Context: ...the motion_planner and returns SUCCESS. Otherwise ot_app_blocked is published for the car...
(SENT_START_CONJUNCTIVE_LINKING_ADVERB_COMMA)
[uncategorized] ~33-~33: The preposition ‘to’ seems more likely in this position.
Context: ...returned. ## Wait This handles wating for clear oncoming traffic if the car has s...
(AI_HYDRA_LEO_REPLACE_FOR_TO)
[uncategorized] ~33-~33: A comma may be missing after the conjunctive/linking adverb ‘Otherwise’.
Context: ...gets published and SUCCESS is returned. Otherwise ot_wait_stopped gets published and the ...
(SENT_START_CONJUNCTIVE_LINKING_ADVERB_COMMA)
[uncategorized] ~33-~33: Use a comma before “and” if it connects two independent clauses (unless they are closely connected and short).
Context: ...Otherwise ot_wait_stopped gets published and the behaviour stays in RUNNING. If the...
(COMMA_COMPOUND_SENTENCE_2)
doc/planning/behaviours/LeaveParkingSpace.md
[uncategorized] ~12-~12: A comma might be missing here.
Context: ... left the parking space. If that is the case a "called" flag is set to true so that ...
(AI_EN_LECTOR_MISSING_PUNCTUATION_COMMA)
[uncategorized] ~13-~13: A comma may be missing after the conjunctive/linking adverb ‘Otherwise’.
Context: ...ILURE is returned to end the behaviour. Otherwise it stays in RUNNING.
(SENT_START_CONJUNCTIVE_LINKING_ADVERB_COMMA)
🪛 Markdownlint (0.37.0)
doc/planning/behaviours/Intersection.md
12-12: Expected: 0 or 2; Actual: 1
Trailing spaces
(MD009, no-trailing-spaces)
doc/planning/behaviours/LaneChange.md
12-12: Expected: 0 or 2; Actual: 1
Trailing spaces
(MD009, no-trailing-spaces)
doc/planning/behaviours/Overtake.md
12-12: Expected: 0 or 2; Actual: 1
Trailing spaces
(MD009, no-trailing-spaces)
19-19: Expected: 0 or 2; Actual: 1
Trailing spaces
(MD009, no-trailing-spaces)
doc/planning/behaviours/LeaveParkingSpace.md
7-7: Expected: 0 or 2; Actual: 1
Trailing spaces
(MD009, no-trailing-spaces)
🔇 Additional comments (2)
doc/planning/behaviours/LeaveParkingSpace.md (1)
10-13
: Verify behavior implementation details
The documentation mentions a "called" flag and distance calculation that should be verified against the actual implementation.
✅ Verification successful
Documentation matches implementation with minor differences
The implementation in code/planning/src/behavior_agent/behaviours/maneuvers.py
aligns with the documentation with some minor variations:
- The "called" flag exists as documented and is set to True when the vehicle has moved far enough from its starting position
- The distance calculation is implemented using numpy's
linalg.norm
function to compute the Euclidean distance between start and current positions - The behavior returns FAILURE (not SUCCESS as documented) when setting called=True to end the behavior
- There's an additional speed check (
speed < 2
) along with the distance check (distance < 1
) to determine if the vehicle is still in the parking space
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for the LeaveParkingSpace behavior implementation
ast-grep --pattern 'class LeaveParkingSpace {
$$$
}'
# Look for the distance calculation implementation
rg -l "euclidean.*distance.*parking.*space"
Length of output: 102
Script:
#!/bin/bash
# Try different search patterns for the LeaveParkingSpace class
rg -l "class LeaveParkingSpace"
# Search for files containing "leave" and "parking" to find potential implementations
rg -l "leave.*parking|parking.*leave" --type cpp --type hpp
# Look for distance calculation with different patterns
rg "distance.*current.*position" --type cpp --type hpp
Length of output: 265
Script:
#!/bin/bash
# Check the implementation in the found Python file
cat code/planning/src/behavior_agent/behaviours/maneuvers.py
# Search for distance calculation without file type restrictions
rg "distance.*current.*position"
# Look for called flag implementation
rg "called.*=.*true" code/planning/src/behavior_agent/behaviours/maneuvers.py
Length of output: 26355
🧰 Tools
🪛 LanguageTool
[uncategorized] ~12-~12: A comma might be missing here.
Context: ... left the parking space. If that is the case a "called" flag is set to true so that ...
(AI_EN_LECTOR_MISSING_PUNCTUATION_COMMA)
[uncategorized] ~13-~13: A comma may be missing after the conjunctive/linking adverb ‘Otherwise’.
Context: ...ILURE is returned to end the behaviour. Otherwise it stays in RUNNING.
(SENT_START_CONJUNCTIVE_LINKING_ADVERB_COMMA)
code/planning/src/behavior_agent/behaviours/lane_change.py (1)
324-329
: Review the status logic in Enter.update()
The current implementation returns SUCCESS when distance ≥ 5 and RUNNING when distance < 5, which seems counterintuitive for a lane change maneuver.
Let's verify if this logic aligns with other lane change implementations:
✅ Verification successful
Let me gather more context about the lane change behavior implementation.
Let me try a different approach to understand the lane change behavior implementation.
Let me get the complete implementation of the update method to understand the status logic better.
Let me get one final piece of context to understand the complete lane change behavior.
The status logic in Enter.update() is correct
The current implementation is actually intuitive for a lane change maneuver:
- When distance < 5, it returns RUNNING because the vehicle is close to the lane change point and actively executing the maneuver
- When distance ≥ 5, it returns SUCCESS because the vehicle has successfully completed the lane change and moved away from the change point
- When no waypoint data is available, it fails safely
This aligns with the behavior's documented purpose: "This behavior initiates the lane change and waits until the lane change is finished."
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for similar distance checks in lane change implementations
rg -A 3 "lane_change.*distance" --type py
Length of output: 2395
Script:
#!/bin/bash
# Search for the Enter class implementation and its context
ast-grep --pattern 'class Enter($_) {
$$$
update($_) {
$$$
}
}' code/planning/src/behavior_agent/behaviours/lane_change.py
Length of output: 134
Script:
#!/bin/bash
# Get the context around the Enter class implementation
rg -B 10 -A 20 "class Enter" code/planning/src/behavior_agent/behaviours/lane_change.py
Length of output: 1154
Script:
#!/bin/bash
# Get the update method implementation in Enter class
rg -B 2 -A 10 "def update\(self\):" code/planning/src/behavior_agent/behaviours/lane_change.py
Length of output: 2206
Script:
#!/bin/bash
# Get the complete context of the lane change implementation including the distance check
rg -B 5 -A 5 "if next_waypoint_msg.distance" code/planning/src/behavior_agent/behaviours/lane_change.py
Length of output: 518
## Wait | ||
|
||
This handles wating for clear oncoming traffic if the car has stopped behind the obstacle. If the overtake is clear ot_wait_free gets published and SUCCESS is returned. Otherwise ot_wait_stopped gets published and the behaviour stays in RUNNING. | ||
|
||
If the obstacle in front is gone the behaviour is aborted with FAILURE. | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Fix spelling and improve state transition documentation
The section contains spelling errors and needs better organization of state transitions.
## Wait
-This handles wating for clear oncoming traffic if the car has stopped behind the obstacle. If the overtake is clear ot_wait_free gets published and SUCCESS is returned. Otherwise ot_wait_stopped gets published and the behaviour stays in RUNNING.
+This phase handles waiting for clear oncoming traffic after the vehicle has stopped behind an obstacle.
+
+State transitions:
+1. Clear path detected:
+ - Publishes `ot_wait_free`
+ - Returns SUCCESS
+2. Path blocked:
+ - Publishes `ot_wait_stopped`
+ - Returns RUNNING
-If the obstacle in front is gone the behaviour is aborted with FAILURE.
+Failure condition:
+- Returns FAILURE if the obstacle is no longer detected (may have moved away)
📝 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.
## Wait | |
This handles wating for clear oncoming traffic if the car has stopped behind the obstacle. If the overtake is clear ot_wait_free gets published and SUCCESS is returned. Otherwise ot_wait_stopped gets published and the behaviour stays in RUNNING. | |
If the obstacle in front is gone the behaviour is aborted with FAILURE. | |
## Wait | |
This phase handles waiting for clear oncoming traffic after the vehicle has stopped behind an obstacle. | |
State transitions: | |
1. Clear path detected: | |
- Publishes `ot_wait_free` | |
- Returns SUCCESS | |
2. Path blocked: | |
- Publishes `ot_wait_stopped` | |
- Returns RUNNING | |
Failure condition: | |
- Returns FAILURE if the obstacle is no longer detected (may have moved away) |
🧰 Tools
🪛 LanguageTool
[uncategorized] ~33-~33: The preposition ‘to’ seems more likely in this position.
Context: ...returned. ## Wait This handles wating for clear oncoming traffic if the car has s...
(AI_HYDRA_LEO_REPLACE_FOR_TO)
[uncategorized] ~33-~33: A comma may be missing after the conjunctive/linking adverb ‘Otherwise’.
Context: ...gets published and SUCCESS is returned. Otherwise ot_wait_stopped gets published and the ...
(SENT_START_CONJUNCTIVE_LINKING_ADVERB_COMMA)
[uncategorized] ~33-~33: Use a comma before “and” if it connects two independent clauses (unless they are closely connected and short).
Context: ...Otherwise ot_wait_stopped gets published and the behaviour stays in RUNNING. If the...
(COMMA_COMPOUND_SENTENCE_2)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good idea to change the logging as well
…ttps://github.com/una-auxme/paf into 553-feature-document-behaviour-tree-and-behaviours
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
🧹 Outside diff range and nitpick comments (8)
doc/planning/behaviours/Intersection.md (5)
15-15
: Fix grammatical errorUse "an" instead of "a" before "intersection" as it starts with a vowel sound.
-The Intersection behaviour is used to control the vehicle when it encounters a intersection. It handles stop signs as well as traffic lights. +The Intersection behaviour is used to control the vehicle when it encounters an intersection. It handles stop signs as well as traffic lights.🧰 Tools
🪛 LanguageTool
[misspelling] ~15-~15: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ... control the vehicle when it encounters a intersection. It handles stop signs as ...(EN_A_VS_AN)
26-26
: Improve readability with proper punctuationAdd a comma before "and" in the compound sentence.
-Handles approaching the intersection by slowing down the vehicle. Returns RUNNING while still far from the intersection, SUCCESS when the vehicle has stopped or has already entered the intersection and FAILURE when the path is faulty. +Handles approaching the intersection by slowing down the vehicle. Returns RUNNING while still far from the intersection, SUCCESS when the vehicle has stopped or has already entered the intersection, and FAILURE when the path is faulty.🧰 Tools
🪛 LanguageTool
[uncategorized] ~26-~26: Possible missing comma found.
Context: ... has stopped or has already entered the intersection and FAILURE when the path is faulty. C...(AI_HYDRA_LEO_MISSING_COMMA)
35-37
: Enhance behavior documentationThe Wait section should specify:
- What constitutes a failure condition
- How long the wait should be in case of stop signs
- Any timeout conditions
41-43
: Clarify waypoint terminologyThe Enter section should define what "next waypoint" means in this context. Consider adding:
- How waypoints are determined
- The specific conditions that indicate the intersection has been cleared
1-47
: Consider adding error handling and edge casesThe documentation would benefit from additional sections covering:
- Error handling for sensor failures (e.g., when traffic light detection fails)
- Edge cases (e.g., intersection with malfunctioning traffic lights)
- Interaction with other vehicles at the intersection
- Recovery behavior when the sequence fails
🧰 Tools
🪛 LanguageTool
[misspelling] ~15-~15: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ... control the vehicle when it encounters a intersection. It handles stop signs as ...(EN_A_VS_AN)
[uncategorized] ~26-~26: Possible missing comma found.
Context: ... has stopped or has already entered the intersection and FAILURE when the path is faulty. C...(AI_HYDRA_LEO_MISSING_COMMA)
doc/planning/behaviours/LaneChange.md (1)
1-39
: Consider adopting a consistent documentation template.To maintain consistency across behavior documentation, consider adding these standard sections to each behavior document:
- Prerequisites and Preconditions
- Configuration Parameters
- State Machine Diagram
- Error Handling
- Integration Points
- Examples
🧰 Tools
🪛 LanguageTool
[uncategorized] ~27-~27: Possible missing comma found.
Context: ...stance of the virtual stop line and not blocked it returns SUCCESS. SUCCESS is also ret...(AI_HYDRA_LEO_MISSING_COMMA)
doc/planning/behaviours/LeaveParkingSpace.md (1)
1-13
: Consider adding more technical details.The documentation could benefit from additional information:
- The specific distance threshold used to determine when the car has "fully left"
- Why FAILURE is returned instead of SUCCESS when the car successfully leaves
- Any preconditions or assumptions about the initial parking position
- Interaction with other behaviors or the behavior tree
Would you like me to propose an expanded version of the documentation that includes these details?
🧰 Tools
🪛 LanguageTool
[uncategorized] ~12-~12: A comma might be missing here.
Context: ... left the parking space. If that is the case a "called" flag is set to true so that ...(AI_EN_LECTOR_MISSING_PUNCTUATION_COMMA)
[uncategorized] ~13-~13: A comma may be missing after the conjunctive/linking adverb ‘Otherwise’.
Context: ...ILURE is returned to end the behaviour. Otherwise it stays in RUNNING.(SENT_START_CONJUNCTIVE_LINKING_ADVERB_COMMA)
doc/planning/behaviours/Overtake.md (1)
3-3
: Enhance the summary with more contextThe current summary is too brief. Consider expanding it to include the behavior's purpose and current status.
-**Summary:** This file explains the Overtake behavior. +**Summary:** This file explains the Overtake behavior, which enables autonomous vehicles to safely maneuver around obstacles by temporarily switching lanes. Note: Currently in prototype stage.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
code/planning/src/behavior_agent/behaviours/lane_change.py
(13 hunks)doc/planning/behaviours/Intersection.md
(1 hunks)doc/planning/behaviours/LaneChange.md
(1 hunks)doc/planning/behaviours/LeaveParkingSpace.md
(1 hunks)doc/planning/behaviours/Overtake.md
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- code/planning/src/behavior_agent/behaviours/lane_change.py
🧰 Additional context used
📓 Learnings (1)
doc/planning/behaviours/Overtake.md (1)
Learnt from: Vroni27
PR: una-auxme/paf#566
File: doc/planning/behaviours/Overtake.md:13-16
Timestamp: 2024-12-13T13:58:49.022Z
Learning: When documenting behaviors, include information about whether the behavior is used or not.
🪛 LanguageTool
doc/planning/behaviours/Intersection.md
[misspelling] ~15-~15: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ... control the vehicle when it encounters a intersection. It handles stop signs as ...
(EN_A_VS_AN)
[uncategorized] ~26-~26: Possible missing comma found.
Context: ... has stopped or has already entered the intersection and FAILURE when the path is faulty. C...
(AI_HYDRA_LEO_MISSING_COMMA)
doc/planning/behaviours/LaneChange.md
[uncategorized] ~27-~27: Possible missing comma found.
Context: ...stance of the virtual stop line and not blocked it returns SUCCESS. SUCCESS is also ret...
(AI_HYDRA_LEO_MISSING_COMMA)
doc/planning/behaviours/LeaveParkingSpace.md
[uncategorized] ~12-~12: A comma might be missing here.
Context: ... left the parking space. If that is the case a "called" flag is set to true so that ...
(AI_EN_LECTOR_MISSING_PUNCTUATION_COMMA)
[uncategorized] ~13-~13: A comma may be missing after the conjunctive/linking adverb ‘Otherwise’.
Context: ...ILURE is returned to end the behaviour. Otherwise it stays in RUNNING.
(SENT_START_CONJUNCTIVE_LINKING_ADVERB_COMMA)
doc/planning/behaviours/Overtake.md
[style] ~14-~14: This phrase is a bit redundant. Consider using just “proximity”.
Context: ...aviour is used to overtake an object in close proximity. This behaviour is currently not workin...
(CLOSE_SCRUTINY)
[misspelling] ~14-~14: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ... is currently not working and more like a initial prototype. ## Overtake ahead ...
(EN_A_VS_AN)
[misspelling] ~18-~18: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ...Overtake ahead Checks whether there is a object in front of the car that needs o...
(EN_A_VS_AN)
[uncategorized] ~26-~26: A comma may be missing after the conjunctive/linking adverb ‘Otherwise’.
Context: ...the motion_planner and returns SUCCESS. Otherwise ot_app_blocked is published for the car...
(SENT_START_CONJUNCTIVE_LINKING_ADVERB_COMMA)
[uncategorized] ~28-~28: Possible missing comma found.
Context: ...slow down. If the car stops behind the obstacle SUCCESS is also returned. ## Wait Thi...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~32-~32: Possible missing comma found.
Context: ...behind the obstacle. If the overtake is clear ot_wait_free gets published and SUCCESS...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~32-~32: A comma may be missing after the conjunctive/linking adverb ‘Otherwise’.
Context: ...gets published and SUCCESS is returned. Otherwise ot_wait_stopped gets published and the ...
(SENT_START_CONJUNCTIVE_LINKING_ADVERB_COMMA)
[uncategorized] ~32-~32: Use a comma before “and” if it connects two independent clauses (unless they are closely connected and short).
Context: ...Otherwise ot_wait_stopped gets published and the behaviour stays in RUNNING. If the...
(COMMA_COMPOUND_SENTENCE_2)
🔇 Additional comments (13)
doc/planning/behaviours/Intersection.md (1)
1-12
: LGTM! Well-structured document layout
The document follows markdown best practices with a clear title and properly organized table of contents.
doc/planning/behaviours/LaneChange.md (4)
13-15
: Expand the General section with essential details.
The General section needs more comprehensive information about lane change behavior.
33-35
: Fix language issues and add technical details.
37-39
: Expand the Leave section with technical details.
21-27
: 🛠️ Refactor suggestion
Improve technical clarity and fix grammar.
- Add missing comma: "...of the virtual stop line, and not blocked..."
- Clarify the following technical aspects:
- The algorithm or method used to calculate the virtual stop line position
- The specific conditions that determine "too far away"
- The rationale for choosing 30 km/h as the speed threshold
-Once the car is within a set distance of the virtual stop line and not blocked it returns SUCCESS.
+Once the car is within a set distance of the virtual stop line, and not blocked, it returns SUCCESS.
Likely invalid or redundant comment.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~27-~27: Possible missing comma found.
Context: ...stance of the virtual stop line and not blocked it returns SUCCESS. SUCCESS is also ret...
(AI_HYDRA_LEO_MISSING_COMMA)
doc/planning/behaviours/LeaveParkingSpace.md (2)
1-7
: LGTM: Document structure follows markdown best practices.
The document has a clear title, summary, and navigation structure.
1-13
: Verify behavior implementation details.
Let's verify the implementation details mentioned in the documentation against the actual code.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~12-~12: A comma might be missing here.
Context: ... left the parking space. If that is the case a "called" flag is set to true so that ...
(AI_EN_LECTOR_MISSING_PUNCTUATION_COMMA)
[uncategorized] ~13-~13: A comma may be missing after the conjunctive/linking adverb ‘Otherwise’.
Context: ...ILURE is returned to end the behaviour. Otherwise it stays in RUNNING.
(SENT_START_CONJUNCTIVE_LINKING_ADVERB_COMMA)
doc/planning/behaviours/Overtake.md (6)
14-14
: 🛠️ Refactor suggestion
Improve General section with status and technical details
-This behaviour is used to overtake an object in close proximity. This behaviour is currently not working and more like a initial prototype.
+This behaviour enables autonomous vehicles to safely overtake stationary or slow-moving objects. The implementation includes:
+
+- Collision detection and prediction
+- Oncoming traffic monitoring
+- Lane change trajectory planning
+- Safe distance maintenance
+
+**Current Status:** Prototype phase - Not actively used in production
+
+**Known Limitations:**
+- Limited testing in real-world scenarios
+- Basic collision avoidance only
+- No support for multiple-lane roads
+- No handling of dynamic obstacles during overtaking
🧰 Tools
🪛 LanguageTool
[style] ~14-~14: This phrase is a bit redundant. Consider using just “proximity”.
Context: ...aviour is used to overtake an object in close proximity. This behaviour is currently not workin...
(CLOSE_SCRUTINY)
[misspelling] ~14-~14: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ... is currently not working and more like a initial prototype. ## Overtake ahead ...
(EN_A_VS_AN)
18-20
: 🛠️ Refactor suggestion
Enhance technical documentation of collision detection
-Checks whether there is a object in front of the car that needs overtaking.
-
-Estimates whether the car would collide with the object soon. If that is the case a counter gets incremented. When that counter reaches 4 SUCCESS is returned. If the object is not blocking the trajectory, FAILURE is returned.
+Determines if an object ahead requires overtaking through the following process:
+
+1. **Object Detection:**
+ - Continuously monitors the environment for obstacles
+ - Classifies objects as overtakeable (e.g., parked vehicles, slow-moving vehicles)
+
+2. **Collision Risk Assessment:**
+ - Calculates Time-To-Collision (TTC) with detected objects
+ - Maintains a rolling counter of high-risk detections
+ - Increments counter when TTC falls below safety threshold
+ - Resets counter when risk subsides
+
+3. **Decision Making:**
+ - Returns SUCCESS when counter reaches 4 consecutive detections
+ (threshold helps filter false positives)
+ - Returns FAILURE if:
+ - No object detected
+ - Object not in vehicle's planned path
+ - Object moving at acceptable speed
🧰 Tools
🪛 LanguageTool
[misspelling] ~18-~18: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ...Overtake ahead Checks whether there is a object in front of the car that needs o...
(EN_A_VS_AN)
24-28
: 🛠️ Refactor suggestion
Clarify approach phase with technical details
-This is running while the obstacle is still in front of the car.
-
-Checks whether the oncoming traffic is far away or clear, if that is the case then ot_app_free is published as the current behaviour for the motion_planner and returns SUCCESS. Otherwise ot_app_blocked is published for the car to slow down.
-
-If the car stops behind the obstacle SUCCESS is also returned.
+The approach phase manages the vehicle's behavior while approaching the obstacle:
+
+1. **Continuous Monitoring:**
+ - Tracks distance to obstacle
+ - Monitors speed differential
+ - Scans opposing lane for oncoming traffic
+
+2. **Traffic Assessment:**
+ - Evaluates safe overtaking gap in opposing lane
+ - Considers:
+ - Distance to oncoming vehicles
+ - Relative speeds
+ - Required overtaking distance
+
+3. **Vehicle Control:**
+ - Clear path:
+ - Publishes `ot_app_free` to motion planner
+ - Returns SUCCESS
+ - Blocked path:
+ - Publishes `ot_app_blocked`
+ - Initiates controlled deceleration
+ - Returns SUCCESS when vehicle stops safely
🧰 Tools
🪛 LanguageTool
[uncategorized] ~26-~26: A comma may be missing after the conjunctive/linking adverb ‘Otherwise’.
Context: ...the motion_planner and returns SUCCESS. Otherwise ot_app_blocked is published for the car...
(SENT_START_CONJUNCTIVE_LINKING_ADVERB_COMMA)
[uncategorized] ~28-~28: Possible missing comma found.
Context: ...slow down. If the car stops behind the obstacle SUCCESS is also returned. ## Wait Thi...
(AI_HYDRA_LEO_MISSING_COMMA)
32-34
: 🛠️ Refactor suggestion
Document state machine behavior and transitions
-This handles wating for clear oncoming traffic if the car has stopped behind the obstacle. If the overtake is clear ot_wait_free gets published and SUCCESS is returned. Otherwise ot_wait_stopped gets published and the behaviour stays in RUNNING.
-
-If the obstacle in front is gone the behaviour is aborted with FAILURE.
+The wait phase implements a state machine for managing the vehicle while stopped:
+
+1. **Entry Conditions:**
+ - Vehicle completely stopped
+ - Obstacle detected ahead
+ - Safe distance maintained
+
+2. **State Transitions:**
+ - Path becomes clear:
+ - Publishes `ot_wait_free`
+ - Returns SUCCESS
+ - Triggers transition to Enter phase
+ - Path remains blocked:
+ - Publishes `ot_wait_stopped`
+ - Returns RUNNING
+ - Continues monitoring
+
+3. **Failure Conditions:**
+ - Obstacle no longer detected:
+ - Returns FAILURE
+ - Aborts overtaking sequence
+ - Emergency vehicle detected:
+ - Returns FAILURE
+ - Yields right of way
🧰 Tools
🪛 LanguageTool
[uncategorized] ~32-~32: Possible missing comma found.
Context: ...behind the obstacle. If the overtake is clear ot_wait_free gets published and SUCCESS...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~32-~32: A comma may be missing after the conjunctive/linking adverb ‘Otherwise’.
Context: ...gets published and SUCCESS is returned. Otherwise ot_wait_stopped gets published and the ...
(SENT_START_CONJUNCTIVE_LINKING_ADVERB_COMMA)
[uncategorized] ~32-~32: Use a comma before “and” if it connects two independent clauses (unless they are closely connected and short).
Context: ...Otherwise ot_wait_stopped gets published and the behaviour stays in RUNNING. If the...
(COMMA_COMPOUND_SENTENCE_2)
38-40
: 🛠️ Refactor suggestion
Document trajectory planning and execution
-Handles switching the lane for overtaking.
-
-Waits for motion planner to finish the trajectory changes and for it to set the overtake_success flag.
+The enter phase manages the lane change trajectory:
+
+1. **Trajectory Planning:**
+ - Calculates optimal lane change path
+ - Considers:
+ - Vehicle dynamics
+ - Safety margins
+ - Road geometry
+ - Current speed
+
+2. **Execution Monitoring:**
+ - Tracks trajectory progress
+ - Monitors:
+ - Lateral position
+ - Heading angle
+ - Lane alignment
+ - Safety envelope
+
+3. **Completion Criteria:**
+ - Motion planner sets `overtake_success`
+ - Vehicle stabilized in target lane
+ - Safe distance from obstacle maintained
+ - Original speed resumed
44-44
: 🛠️ Refactor suggestion
Document completion criteria and safety checks
-Runs until the overtake is fully finished and then leaves the behaviour.
+The leave phase ensures safe completion of the overtaking maneuver:
+
+1. **Safety Checks:**
+ - Minimum passing distance achieved
+ - No sudden obstacles detected
+ - Safe gap in target lane maintained
+ - Vehicle stability confirmed
+
+2. **Return Maneuver:**
+ - Initiates return to original lane
+ - Maintains safe distance from overtaken object
+ - Ensures smooth trajectory
+
+3. **Completion Criteria:**
+ - Vehicle aligned in original lane
+ - Normal driving speed resumed
+ - Safe following distance established
+ - All safety checks passed
+
+The behavior terminates with SUCCESS when all criteria are met,
+allowing the system to resume normal operation.
## Lane Change ahead | ||
|
||
Checks whether the next waypoint is a lane change and inititates the lane change sequence accordingly. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Add technical details about lane change point detection.
Please include information about:
- How the lane change waypoint is identified in the path
- What parameters or attributes define a lane change point
- The specific criteria that trigger the lane change sequence
## Wait | ||
|
||
Waits at the lane change point until the lane change is not blocked (not implemented). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Add safety considerations for the Wait state.
Please document:
- Safety checks performed during the waiting period
- Emergency handling procedures
- Maximum wait time before aborting the lane change
- Conditions for reverting to the original lane
## Enter | ||
|
||
Inititates the lane change with 20 km/h and continues driving on the next lane until the lane change waypoint is far enough away. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Document trajectory planning details.
Please add information about:
- The trajectory generation algorithm used during lane change
- Safety envelope considerations
- How the target position in the new lane is calculated
- Handling of dynamic obstacles during the maneuver
The leave parking space behaviour is only executed at the beginning of the simulation to leave the parking space. | ||
|
||
The behaviour calculates the euclidian distance between the starting position and the current position to determine whether the car has fully left the parking space. If that is the case a "called" | ||
flag is set to true so that this behaviour is never executed again and FAILURE is returned to end the behaviour. Otherwise it stays in RUNNING. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Fix grammar and improve technical clarity.
The content has several issues that need to be addressed:
- Spelling: "euclidian" should be "Euclidean"
- Missing commas and formatting issues
- Technical details could be more specific
Apply these changes to improve the documentation:
-The leave parking space behaviour is only executed at the beginning of the simulation to leave the parking space.
+The Leave Parking Space behavior is only executed at the beginning of the simulation to move the vehicle out of its initial parking position.
-The behaviour calculates the euclidian distance between the starting position and the current position to determine whether the car has fully left the parking space. If that is the case a "called"
-flag is set to true so that this behaviour is never executed again and FAILURE is returned to end the behaviour. Otherwise it stays in RUNNING.
+The behavior calculates the Euclidean distance between the starting position and the current position to determine whether the car has fully left the parking space. If that is the case, a "called"
+flag is set to true so that this behavior is never executed again, and FAILURE is returned to end the behavior. Otherwise, it stays in RUNNING state.
📝 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.
The leave parking space behaviour is only executed at the beginning of the simulation to leave the parking space. | |
The behaviour calculates the euclidian distance between the starting position and the current position to determine whether the car has fully left the parking space. If that is the case a "called" | |
flag is set to true so that this behaviour is never executed again and FAILURE is returned to end the behaviour. Otherwise it stays in RUNNING. | |
The Leave Parking Space behavior is only executed at the beginning of the simulation to move the vehicle out of its initial parking position. | |
The behavior calculates the Euclidean distance between the starting position and the current position to determine whether the car has fully left the parking space. If that is the case, a "called" | |
flag is set to true so that this behavior is never executed again, and FAILURE is returned to end the behavior. Otherwise, it stays in RUNNING state. |
🧰 Tools
🪛 LanguageTool
[uncategorized] ~12-~12: A comma might be missing here.
Context: ... left the parking space. If that is the case a "called" flag is set to true so that ...
(AI_EN_LECTOR_MISSING_PUNCTUATION_COMMA)
[uncategorized] ~13-~13: A comma may be missing after the conjunctive/linking adverb ‘Otherwise’.
Context: ...ILURE is returned to end the behaviour. Otherwise it stays in RUNNING.
(SENT_START_CONJUNCTIVE_LINKING_ADVERB_COMMA)
Description
Added documentation for behaviours.
Updated the behaviour tree graph and added a draw.io xml file for future updates.
Overhauled comments and logging messages inside behaviour files.
Fixes # 553
Type of change
Does this PR introduce a breaking change?
No
Checklist:
Summary by CodeRabbit
New Features
Bug Fixes
Documentation