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

360 document and review systems architecture of autonomous driving platform #408

Conversation

SirMDA
Copy link
Collaborator

@SirMDA SirMDA commented Nov 1, 2024

Description

This adds an updated architecture documentation. It was checked which nodes exist and which connections are between them. Issues for checking the architecture of the subteams were created (#392 #393 #394 #395) These will be done the next sprint to extend the documentation further.

Fixes #360

Type of change

Please delete options that are not relevant.

  • Documentation update

Does this PR introduce a breaking change?

No

Most important changes

Added a new image which holds the information of the current rosgraphs nodes and connections between them.

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works (might be obsolete with CI later on)
  • New and existing unit tests pass locally with my changes (might be obsolete with CI later on)

Summary by CodeRabbit

  • Documentation
    • Updated architecture documentation to enhance understanding of component interactions.
    • Added links to detailed architecture design documents and a Miro board for visual representation.
    • Included an updated visual representation of the current ROS graph, highlighting node interactions.
    • Provided links to extended information on perception, planning, and acting architectures for deeper insights.

@SirMDA SirMDA linked an issue Nov 1, 2024 that may be closed by this pull request
4 tasks
Copy link
Contributor

coderabbitai bot commented Nov 1, 2024

Warning

Rate limit exceeded

@SirMDA has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 21 minutes and 8 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

📥 Commits

Reviewing files that changed from the base of the PR and between 3766310 and 5c8a602.

Walkthrough

The changes in the architecture_documentation.md file enhance the documentation of the existing architecture for the autonomous driving platform. The updates include an overview of component interactions, links to detailed design documents, and a visual representation of the ROS graph. The document categorizes nodes into perception, planning, and acting, while highlighting the need for further detail on node logic and interconnections. Additionally, it provides a Miro board link for visual reference, noting potential gaps in alignment with the current ROS graph.

Changes

File Path Change Summary
doc/research/paf24/system/.../architecture_documentation.md Added an overview of architecture documentation, links to detailed documents, and updated ROS graph representation. Included Miro board link and noted potential gaps in completeness.

Assessment against linked issues

Objective Addressed Explanation
A deeper understanding of the repository is gained
A high-level architectural diagram is created, detailing the interaction between various modules.
Documentation includes data flow between critical components.
Issues for architecture reviews with the new teams are created. No issues for architecture reviews were created.

Possibly related PRs

  • get current state and document it #363 #399: The changes in current_state.md provide a comprehensive overview of the agent's performance, which may relate to the architectural understanding presented in architecture_documentation.md, particularly in the context of how the agent's behavior interacts with the overall system architecture.

Poem

In the land of nodes and beams,
Our architecture flows like dreams.
With links and graphs, we pave the way,
For perception, planning, come what may.
A Miro board to guide our sight,
In the world of code, we craft the light! 🐇✨


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

@SirMDA SirMDA added this to the Sprint 00 2024-11-04 milestone Nov 1, 2024
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: 3

🧹 Outside diff range and nitpick comments (3)
doc/research/paf24/system/architecture_documentation.md (3)

1-5: Enhance the introduction's clarity and professionalism.

Consider restructuring the introduction to be more concise and professional:

-# Research about the existing architecture documentation
-The repository already holds various documents about how the architecture was planned and how the architecture should look or is
-looking. As it is a crucial part of the project to understand the component interactions, especially the up-to-date
-version of it, in this document I will give a brief overview of the existing documentation and some links to the up-to-date versions.
+# Architecture Documentation Overview
+
+This document provides a comprehensive overview of the autonomous driving platform's architecture documentation. It consolidates 
+existing architectural documentation, component interactions, and provides references to the most current documentation versions.
🧰 Tools
🪛 Markdownlint

1-1: Expected: 1; Actual: 0; Below
Headings should be surrounded by blank lines

(MD022, blanks-around-headings)


18-19: Remove commented out image reference.

Remove the commented out graph reference since an active reference is already present.

-[//]: # "![Up to date ros graph](/doc/assets/research_assets/rosgraph.svg)"

27-32: Enhance component architecture sections.

The current sections lack context about each component's role in the system. Consider:

  1. Adding brief descriptions of each component's responsibilities
  2. Including key interfaces and dependencies
  3. Fixing the markdown formatting issues with proper list indentation and spacing
 ## Perception architecture
+
+The perception component processes sensor data to understand the vehicle's environment.
+
   - Extended information of how the perception works can be found [here](/doc/perception/README.md)
+
 ## Planning architecture
+
+The planning component determines the vehicle's path and behavior based on perception data.
+
   - Extended information of how the planning works can be found [here](/doc/planning/README.md)
+
 ## Acting architecture
+
+The acting component translates planning decisions into vehicle control commands.
+
   - Extended information of how the acting works can be found [here](/doc/acting/architecture_documentation.md)
🧰 Tools
🪛 Markdownlint

28-28: Expected: 0; Actual: 2
Unordered list indentation

(MD007, ul-indent)


30-30: Expected: 0; Actual: 2
Unordered list indentation

(MD007, ul-indent)


32-32: Expected: 0; Actual: 2
Unordered list indentation

(MD007, ul-indent)


27-27: Expected: 1; Actual: 0; Below
Headings should be surrounded by blank lines

(MD022, blanks-around-headings)


29-29: Expected: 1; Actual: 0; Above
Headings should be surrounded by blank lines

(MD022, blanks-around-headings)


29-29: Expected: 1; Actual: 0; Below
Headings should be surrounded by blank lines

(MD022, blanks-around-headings)


31-31: Expected: 1; Actual: 0; Above
Headings should be surrounded by blank lines

(MD022, blanks-around-headings)


31-31: Expected: 1; Actual: 0; Below
Headings should be surrounded by blank lines

(MD022, blanks-around-headings)


28-28: null
Lists should be surrounded by blank lines

(MD032, blanks-around-lists)


28-28: null
Lists should be surrounded by blank lines

(MD032, blanks-around-lists)


30-30: null
Lists should be surrounded by blank lines

(MD032, blanks-around-lists)


30-30: null
Lists should be surrounded by blank lines

(MD032, blanks-around-lists)


32-32: null
Lists should be surrounded by blank lines

(MD032, blanks-around-lists)

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between 8c968fb and 3766310.

⛔ Files ignored due to path filters (3)
  • doc/assets/research_assets/node_path_ros.png is excluded by !**/*.png
  • doc/assets/research_assets/rosgraph.svg is excluded by !**/*.svg
  • doc/assets/research_assets/rosgraph_leaf_topics.svg is excluded by !**/*.svg
📒 Files selected for processing (1)
  • doc/research/paf24/system/architecture_documentation.md (1 hunks)
🧰 Additional context used
🪛 LanguageTool
doc/research/paf24/system/architecture_documentation.md

[uncategorized] ~8-~8: The preposition “about” seems more likely in this position.
Context: ...chitecture.md). It contains information to most nodes and what they subscribe / pu...

(AI_EN_LECTOR_REPLACEMENT_PREPOSITION)


[uncategorized] ~14-~14: Possible missing comma found.
Context: ...tecture details and flows of information but when comparing it to the rosgraph of t...

(AI_HYDRA_LEO_MISSING_COMMA)


[style] ~15-~15: Consider using “incomplete” to avoid wordiness.
Context: ...and topics it seems like the diagram is not complete. ### Current Rosgraph of the nodes and...

(NOT_ABLE_PREMIUM)

🪛 Markdownlint
doc/research/paf24/system/architecture_documentation.md

28-28: Expected: 0; Actual: 2
Unordered list indentation

(MD007, ul-indent)


30-30: Expected: 0; Actual: 2
Unordered list indentation

(MD007, ul-indent)


32-32: Expected: 0; Actual: 2
Unordered list indentation

(MD007, ul-indent)


14-14: Expected: 0 or 2; Actual: 1
Trailing spaces

(MD009, no-trailing-spaces)


1-1: Expected: 1; Actual: 0; Below
Headings should be surrounded by blank lines

(MD022, blanks-around-headings)


6-6: Expected: 1; Actual: 0; Below
Headings should be surrounded by blank lines

(MD022, blanks-around-headings)


17-17: Expected: 1; Actual: 0; Below
Headings should be surrounded by blank lines

(MD022, blanks-around-headings)


21-21: Expected: 1; Actual: 0; Below
Headings should be surrounded by blank lines

(MD022, blanks-around-headings)


27-27: Expected: 1; Actual: 0; Below
Headings should be surrounded by blank lines

(MD022, blanks-around-headings)


29-29: Expected: 1; Actual: 0; Above
Headings should be surrounded by blank lines

(MD022, blanks-around-headings)


29-29: Expected: 1; Actual: 0; Below
Headings should be surrounded by blank lines

(MD022, blanks-around-headings)


31-31: Expected: 1; Actual: 0; Above
Headings should be surrounded by blank lines

(MD022, blanks-around-headings)


31-31: Expected: 1; Actual: 0; Below
Headings should be surrounded by blank lines

(MD022, blanks-around-headings)


17-17: Punctuation: ':'
Trailing punctuation in heading

(MD026, no-trailing-punctuation)


21-21: Punctuation: ':'
Trailing punctuation in heading

(MD026, no-trailing-punctuation)


28-28: null
Lists should be surrounded by blank lines

(MD032, blanks-around-lists)


28-28: null
Lists should be surrounded by blank lines

(MD032, blanks-around-lists)


30-30: null
Lists should be surrounded by blank lines

(MD032, blanks-around-lists)


30-30: null
Lists should be surrounded by blank lines

(MD032, blanks-around-lists)


32-32: null
Lists should be surrounded by blank lines

(MD032, blanks-around-lists)

🔇 Additional comments (1)
doc/research/paf24/system/architecture_documentation.md (1)

14-15: Specify the gaps between Miro board and actual implementation.

The document mentions that the Miro board diagram is incomplete compared to the ROS graph, but doesn't specify which aspects are missing. Consider adding details about:

  • Which nodes or connections are missing
  • What discrepancies exist between the documented and actual architecture
  • Any planned updates to align the documentation
#!/bin/bash
# Verify if there's additional context about the architectural gaps in other documentation
rg -l "architecture.*gap" --type md
🧰 Tools
🪛 LanguageTool

[uncategorized] ~14-~14: Possible missing comma found.
Context: ...tecture details and flows of information but when comparing it to the rosgraph of t...

(AI_HYDRA_LEO_MISSING_COMMA)


[style] ~15-~15: Consider using “incomplete” to avoid wordiness.
Context: ...and topics it seems like the diagram is not complete. ### Current Rosgraph of the nodes and...

(NOT_ABLE_PREMIUM)

🪛 Markdownlint

14-14: Expected: 0 or 2; Actual: 1
Trailing spaces

(MD009, no-trailing-spaces)

@victor-42 victor-42 self-requested a review November 1, 2024 10:20
Copy link
Collaborator

@victor-42 victor-42 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Checks all out! 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Document and Review Systems Architecture of Autonomous Driving Platform
2 participants