Skip to content
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

Merged
merged 21 commits into from
Dec 15, 2024

Conversation

seefelke
Copy link
Collaborator

@seefelke seefelke commented Dec 12, 2024

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

  • Documentation update

Does this PR introduce a breaking change?

No

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings

Summary by CodeRabbit

  • New Features

    • Introduced detailed documentation for various vehicle behaviors including Intersection, Lane Change, Leave Parking Space, and Overtake.
    • Added a graphical representation of the behavior tree for planning purposes.
  • Bug Fixes

    • Enhanced logging clarity across multiple behavior classes, improving context during vehicle operations.
  • Documentation

    • Updated existing documentation for the behavior tree and README files to reflect changes and improve clarity.
    • Added new markdown files for comprehensive behavior descriptions.

@seefelke seefelke added this to the Sprint 03 2024-12-16 milestone Dec 12, 2024
@seefelke seefelke self-assigned this Dec 12, 2024
@seefelke seefelke linked an issue Dec 12, 2024 that may be closed by this pull request
2 tasks
Copy link
Contributor

coderabbitai bot commented Dec 12, 2024

Walkthrough

The 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 Approach, Wait, Enter, and Leave classes. Additionally, new documentation files have been created to outline the behaviors associated with intersections, lane changes, parking space exits, and overtaking. A new XML file representing a behavior tree is also included, providing a visual reference for the decision-making process.

Changes

File Path Change Summary
code/planning/src/behavior_agent/behaviours/intersection.py Enhanced logic and control flow in Approach, Wait, and Enter classes; expanded comments and logging; added virtual stop line calculations; refined waiting logic based on traffic light status.
code/planning/src/behavior_agent/behaviours/lane_change.py Improved logging for lane change operations; updated class documentation for Enter and Leave classes; clarified comments regarding future enhancements.
code/planning/src/behavior_agent/behaviours/overtake.py Enhanced logging and behavior logic in Approach, Wait, Enter, and Leave classes; clarified lane status descriptions and refined logging for overtaking maneuvers.
doc/assets/planning/behaviour_tree.drawio.xml New file created containing a graphical representation of a behavior tree for planning purposes.
doc/planning/Behavior_tree.md Updated documentation to reference the new XML file and clarified behavior descriptions; corrected terminology for return values.
doc/planning/README.md Updated image reference and made minor grammatical corrections.
doc/planning/behaviours/Intersection.md New documentation file detailing intersection behavior and its sub-behaviors.
doc/planning/behaviours/LaneChange.md New documentation file outlining lane change behavior and its processes.
doc/planning/behaviours/LeaveParkingSpace.md New documentation file for the "Leave Parking Space" behavior, detailing its logic and execution.
doc/planning/behaviours/Overtake.md New documentation file for the Overtake behavior, outlining its functionality and control flow.

Possibly related PRs

  • 383 planning evaluate and document motion planning #418: The changes in the MotionPlanning class in motion_planning.py enhance trajectory generation, which is relevant to the vehicle's behavior at intersections, as detailed in the main PR.
  • 373 unstuck behavior #425: The documentation on the "Unstuck/Overtake Behavior" discusses the vehicle's behavior during overtaking, which relates to the intersection behavior modifications in the main PR.
  • 529 - Implement service for planning - acting communication #549: The implementation of the RequestBehaviourChange service facilitates communication between planning and acting components, which is directly relevant to the behavior changes in the intersection handling described in the main PR.

🐰 In the world of lanes and lights,
A rabbit hops with all its might.
With clearer paths and logs so bright,
It guides the cars both day and night.
With every turn and careful wait,
Our journey's smooth, it’s simply great! 🚗✨


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?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR. (Experiment)
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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 section

The 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 formatting

The 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 sections

The current documentation provides a good overview but could benefit from additional sections to make it more comprehensive.

Consider adding the following sections:

  1. Parameters - Document any configurable parameters (e.g., distance threshold)
  2. Return Values - Clearly document the FAILURE and RUNNING states
  3. Example Usage - Provide a simple example of how this behavior is used
  4. Dependencies - List any dependencies or prerequisites
  5. 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:

  1. Error Handling & Recovery

    • Common failure scenarios
    • Recovery procedures
    • Error messages and troubleshooting
  2. Edge Cases

    • Emergency vehicles
    • Road work zones
    • Weather conditions
    • System failures
  3. 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 clarity

The 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 context

The 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 details

The 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 maintainability

While the documentation improvements are good, there are several architectural improvements that could be made:

  1. Consider creating a dedicated logging utility class to standardize log message formats and levels across all behaviors
  2. Move configuration values (like distances and speeds) to a central configuration file
  3. Consider using a state management pattern instead of global variables
  4. 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 feature

The 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 logic

The traffic check logic is duplicated between the Approach and Wait 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_distance

Then 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 logging

The 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 implementations

The current implementation shows several patterns that could be improved:

  1. Common behavior patterns (setup, initialization, termination) are duplicated across classes
  2. Hardcoded distance values are scattered throughout the code
  3. Logging patterns are inconsistent

Consider these improvements:

  1. Create a base behavior class that implements common patterns
  2. Move configuration values to a central configuration file
  3. 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 context

The 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 details

To make this documentation more valuable for developers:

  1. Add code examples showing typical usage
  2. Document message formats for motion planner communication
  3. Include sequence diagrams for state transitions
  4. Add error handling and edge cases
  5. 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 the Approach 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 implemented

In the Wait class, the logic for checking if the intersection is clear is currently incomplete. The variable lidar_data is set to None, so the condition if lidar_data is not None: will always evaluate to False, and intersection_clear remains True 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 12

Line 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 values

In 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 for SUCCESS and RUNNING.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 727f1d5 and 80aa8bb.

⛔ 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

doc/planning/behaviours/LaneChange.md Show resolved Hide resolved
doc/planning/behaviours/LaneChange.md Show resolved Hide resolved
doc/planning/behaviours/LaneChange.md Show resolved Hide resolved
doc/planning/behaviours/LaneChange.md Show resolved Hide resolved
doc/planning/behaviours/LaneChange.md Show resolved Hide resolved
doc/planning/behaviours/Overtake.md Show resolved Hide resolved
doc/planning/behaviours/Overtake.md Show resolved Hide resolved
Comment on lines +31 to +36
## 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.

Copy link
Contributor

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.

Suggested change
## 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)

doc/planning/behaviours/Overtake.md Show resolved Hide resolved
doc/planning/behaviours/Overtake.md Outdated Show resolved Hide resolved
Copy link
Collaborator

@Vroni27 Vroni27 left a 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

doc/planning/behaviours/Overtake.md Show resolved Hide resolved
Copy link
Contributor

@coderabbitai coderabbitai bot left a 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 error

Use "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 punctuation

Add 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 documentation

The Wait section should specify:

  1. What constitutes a failure condition
  2. How long the wait should be in case of stop signs
  3. Any timeout conditions

41-43: Clarify waypoint terminology

The Enter section should define what "next waypoint" means in this context. Consider adding:

  1. How waypoints are determined
  2. The specific conditions that indicate the intersection has been cleared

1-47: Consider adding error handling and edge cases

The documentation would benefit from additional sections covering:

  1. Error handling for sensor failures (e.g., when traffic light detection fails)
  2. Edge cases (e.g., intersection with malfunctioning traffic lights)
  3. Interaction with other vehicles at the intersection
  4. 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:

  1. The specific distance threshold used to determine when the car has "fully left"
  2. Why FAILURE is returned instead of SUCCESS when the car successfully leaves
  3. Any preconditions or assumptions about the initial parking position
  4. 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 context

The 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

📥 Commits

Reviewing files that changed from the base of the PR and between 80aa8bb and 1f4c753.

📒 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.

  1. Add missing comma: "...of the virtual stop line, and not blocked..."
  2. 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.

Comment on lines +17 to +19
## Lane Change ahead

Checks whether the next waypoint is a lane change and inititates the lane change sequence accordingly.
Copy link
Contributor

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

Comment on lines +29 to +31
## Wait

Waits at the lane change point until the lane change is not blocked (not implemented).
Copy link
Contributor

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

Comment on lines +33 to +35
## 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.
Copy link
Contributor

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

Comment on lines +10 to +13
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.
Copy link
Contributor

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:

  1. Spelling: "euclidian" should be "Euclidean"
  2. Missing commas and formatting issues
  3. 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.

Suggested change
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)

@Vroni27 Vroni27 merged commit eabc86e into main Dec 15, 2024
4 checks passed
@Vroni27 Vroni27 deleted the 553-feature-document-behaviour-tree-and-behaviours branch December 15, 2024 18:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: ✅ Done
Development

Successfully merging this pull request may close these issues.

[Feature]: Document behaviour tree and behaviours
2 participants