490 remodel acc concept #516
Conversation
added information about current implementation, new concept and next steps
WalkthroughThe pull request introduces updates to the Adaptive Cruise Control (ACC) system documentation and implementation. It provides an overview of ACC systems, detailing their objectives, methodologies, and challenges. Key control techniques such as PID Control, Model Predictive Control (MPC), and Fuzzy Logic Control (FLC) are discussed. The changes include the addition of a new enumeration for behaviors and a new trajectory message type to enhance trajectory management. The document emphasizes the need for restructuring the current implementation to address existing issues. Changes
Assessment against linked issues
Possibly related PRs
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (4)
doc/research/paf24/planning/ACC.md (4)
4-6: Consider adding specific velocity thresholds for ACC systems.The distinction between classic ACC and Stop & Go systems could be more precise. Consider adding typical industry-standard velocity thresholds and citing sources for these values.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~5-~5: Use a comma before ‘and’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...n our case, both systems might be needed and it might be reasonable to develop two d...(COMMA_COMPOUND_SENTENCE)
17-17: Fix typo in MPC procedure.There's a typo in "repitition" which should be "repetition".
65-65: Define clear criteria for overtaking decisions.The statement about initiating overtaking needs more specific criteria:
- What speed difference triggers overtaking?
- How do we ensure safe overtaking conditions?
- How does this integrate with the lane change system?
84-85: Enhance documentation with additional sources and proper citations.Consider:
- Adding more academic and industry sources
- Using proper citation format instead of bare URLs
- Including specific references for safety distance calculations and control algorithms
🧰 Tools
🪛 Markdownlint (0.35.0)
84-84: Expected: 1; Actual: 0; Below
Headings should be surrounded by blank lines(MD022, blanks-around-headings)
85-85: null
Bare URL used(MD034, no-bare-urls)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
doc/assets/research_assets/ACC_FLC_Example_1.PNGis excluded by!**/*.png
📒 Files selected for processing (1)
doc/research/paf24/planning/ACC.md(1 hunks)
🧰 Additional context used
🪛 LanguageTool
doc/research/paf24/planning/ACC.md
[uncategorized] ~5-~5: Use a comma before ‘and’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...n our case, both systems might be needed and it might be reasonable to develop two d...
(COMMA_COMPOUND_SENTENCE)
[uncategorized] ~55-~55: Possible missing comma found.
Context: ...just accordingly in a local manner. For this a new trajectory message type was imple...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~63-~63: Possible missing comma found.
Context: ...stem needs to be discussed. For safety distance the old approach can simply be modified...
(AI_HYDRA_LEO_MISSING_COMMA)
[style] ~63-~63: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...he old approach can simply be modified. For example, (speed / 10)*3 + (speed / 10)²...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[uncategorized] ~63-~63: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...le, (speed / 10)*3 + (speed / 10)² is a well known formula for calculating the braking dis...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
[style] ~64-~64: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ... for calculating the braking distance. For a general speed target we need to take ...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[uncategorized] ~67-~67: Possible missing comma found.
Context: ...oint in the trajectory up to the car in front for example by interpolation. Points fu...
(AI_HYDRA_LEO_MISSING_COMMA)
🪛 Markdownlint (0.35.0)
doc/research/paf24/planning/ACC.md
22-22: Expected: 1; Actual: 2
Multiple consecutive blank lines
(MD012, no-multiple-blanks)
82-82: Expected: 1; Actual: 2
Multiple consecutive blank lines
(MD012, no-multiple-blanks)
83-83: Expected: 1; Actual: 3
Multiple consecutive blank lines
(MD012, no-multiple-blanks)
3-3: Expected: 1; Actual: 0; Below
Headings should be surrounded by blank lines
(MD022, blanks-around-headings)
10-10: Expected: 1; Actual: 0; Below
Headings should be surrounded by blank lines
(MD022, blanks-around-headings)
19-19: Expected: 1; Actual: 0; Below
Headings should be surrounded by blank lines
(MD022, blanks-around-headings)
69-69: Expected: 1; Actual: 0; Below
Headings should be surrounded by blank lines
(MD022, blanks-around-headings)
78-78: Expected: 1; Actual: 0; Below
Headings should be surrounded by blank lines
(MD022, blanks-around-headings)
84-84: Expected: 1; Actual: 0; Below
Headings should be surrounded by blank lines
(MD022, blanks-around-headings)
78-78: Punctuation: ':'
Trailing punctuation in heading
(MD026, no-trailing-punctuation)
13-13: null
Lists should be surrounded by blank lines
(MD032, blanks-around-lists)
79-79: null
Lists should be surrounded by blank lines
(MD032, blanks-around-lists)
85-85: null
Bare URL used
(MD034, no-bare-urls)
🔇 Additional comments (1)
doc/research/paf24/planning/ACC.md (1)
44-44:
Critical: Safety distance calculation needs immediate attention.
The current safety distance formula speed + (speed * 0.36)² is identified as incorrect. This is a critical safety concern that should be addressed before any other improvements.
doc/research/paf24/planning/ACC.md
Outdated
| ### Requirements: | ||
| - obstacle speed | ||
| - obstacle distance | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Expand requirements section with specific criteria.
The requirements section needs significant expansion to include:
- Safety requirements (minimum safe distances, maximum deceleration rates)
- Performance requirements (response times, accuracy)
- System constraints (sensor limitations, processing delays)
- Integration requirements with other systems
🧰 Tools
🪛 Markdownlint (0.35.0)
78-78: Expected: 1; Actual: 0; Below
Headings should be surrounded by blank lines
(MD022, blanks-around-headings)
78-78: Punctuation: ':'
Trailing punctuation in heading
(MD026, no-trailing-punctuation)
79-79: null
Lists should be surrounded by blank lines
(MD032, blanks-around-lists)
doc/research/paf24/planning/ACC.md
Outdated
|
|
||
| The general idea for speeds above the 40 km/h mark is to calculate a proper safety distance and a general target velocity. For speeds lower than that a stop and go system needs to be discussed. | ||
|
|
||
| For safety distance the old approach can simply be modified. For example, (speed / 10)*3 + (speed / 10)² is a well known formula for calculating the braking distance. |
There was a problem hiding this comment.
💡 Codebase verification
The proposed safety distance formula needs revision based on existing implementation
The proposed formula (speed / 10)*3 + (speed / 10)² differs significantly from the current validated implementation in the codebase. The existing implementation in utils.py uses:
- Reaction distance = speed
- Braking distance = (speed * 0.36)²
- Total safety distance = reaction_distance + braking_distance
The proposed formula would result in less conservative safety margins compared to the existing implementation, which could compromise safety. Stick with the current implementation as it:
- Separates reaction distance and braking distance components
- Has been tested in the CARLA simulator environment
- Includes an emergency braking mode with adjusted calculations
🔗 Analysis chain
Validate the proposed safety distance formula.
The proposed formula (speed / 10)*3 + (speed / 10)² for braking distance should be:
- Validated against industry standards
- Include additional safety margin
- Consider road conditions and vehicle capabilities
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any existing safety distance calculations in the codebase
rg "safety.*distance|braking.*distance" --type cpp --type python
Length of output: 97
Script:
#!/bin/bash
# Search for any existing safety distance calculations in the codebase with correct file extensions
rg "safety.*distance|braking.*distance" -t cpp -t py
# Also search for general distance calculations that might be safety-related
rg "calculateDistance|getDistance|distance.*calculation" -t cpp -t py
# Look for files that might contain safety-related calculations
fd -e cpp -e hpp -e py -e h | grep -i "safety\|distance\|brake"
Length of output: 1159
Script:
#!/bin/bash
# Let's examine the actual implementation of safety distance calculation
cat code/planning/src/local_planner/utils.py
Length of output: 7921
🧰 Tools
🪛 LanguageTool
[uncategorized] ~63-~63: Possible missing comma found.
Context: ...stem needs to be discussed. For safety distance the old approach can simply be modified...
(AI_HYDRA_LEO_MISSING_COMMA)
[style] ~63-~63: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...he old approach can simply be modified. For example, (speed / 10)*3 + (speed / 10)²...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[uncategorized] ~63-~63: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...le, (speed / 10)*3 + (speed / 10)² is a well known formula for calculating the braking dis...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
fix linter
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
doc/research/paf24/planning/ACC.md (2)
5-7: Enhance ACC system boundaries and requirementsThe distinction between ACC and Stop & Go systems needs more specific details:
- Define exact speed thresholds and transition logic between systems
- Specify how the systems will handle edge cases (e.g., transitioning between high and low speeds)
- Document the rationale for potentially implementing two separate systems
Would you like me to help draft a detailed specification for the system boundaries and transition logic?
🧰 Tools
🪛 LanguageTool
[uncategorized] ~6-~6: Use a comma before ‘and’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...n our case, both systems might be needed and it might be reasonable to develop two d...(COMMA_COMPOUND_SENTENCE)
87-89: Enhance documentation with additional sourcesConsider adding more references:
- Industry standards for ACC systems
- Research papers on safety distance calculations
- Implementation examples of trajectory-based ACC
Would you like assistance in finding relevant academic and industry sources?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
doc/research/paf24/planning/ACC.md(1 hunks)
🧰 Additional context used
🪛 LanguageTool
doc/research/paf24/planning/ACC.md
[uncategorized] ~6-~6: Use a comma before ‘and’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...n our case, both systems might be needed and it might be reasonable to develop two d...
(COMMA_COMPOUND_SENTENCE)
[style] ~66-~66: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...he old approach can simply be modified. For example, (speed / 10)*3 + (speed / 10)²...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[uncategorized] ~66-~66: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...le, (speed / 10)*3 + (speed / 10)² is a well known formula for calculating the braking dis...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
[style] ~67-~67: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ... for calculating the braking distance. For a general speed target we need to take ...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
🔇 Additional comments (4)
doc/research/paf24/planning/ACC.md (4)
47-51:
Prioritize critical safety and architectural issues
The identified issues need to be prioritized:
- Safety-critical: Incorrect safety distance calculation must be addressed first
- Architectural: Separate ACC from waypoint management and unstuck routine
- Behavioral: Speed limit handling needs clear policy
This aligns with the previous architectural review comment. The separation of concerns is essential for maintainability and safety.
66-66:
Validate and document safety distance calculations
The proposed formula (speed / 10)*3 + (speed / 10)² needs:
- Validation against industry standards
- Documentation of assumptions and limitations
- Consideration of different road conditions
This aligns with the previous safety distance review comment. Please provide empirical evidence or citations supporting this formula's safety margins.
🧰 Tools
🪛 LanguageTool
[style] ~66-~66: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...he old approach can simply be modified. For example, (speed / 10)*3 + (speed / 10)²...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[uncategorized] ~66-~66: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...le, (speed / 10)*3 + (speed / 10)² is a well known formula for calculating the braking dis...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
82-86:
Expand system requirements section
The requirements section needs significant expansion to include:
- Functional requirements:
- Speed control ranges and accuracy
- Response time requirements
- System modes (ACC, Stop & Go)
- Safety requirements:
- Minimum safe distances
- Maximum acceleration/deceleration rates
- Fail-safe behaviors
- Performance requirements:
- Sensor update rates
- Processing latency limits
- Integration requirements:
- Interface specifications
- Data flow documentation
This aligns with the previous requirements review comment. Would you like assistance in drafting a comprehensive requirements specification?
68-71: Specify trajectory calculation details
The trajectory-based speed control needs more specific details:
- How far ahead should the trajectory planning look?
- What interpolation method will be used for speed calculations?
- How will the system handle sudden changes in the leading vehicle's speed?
Would you like assistance in designing the trajectory calculation algorithm?
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (4)
doc/research/paf24/planning/ACC.md (4)
11-31: Enhance control techniques section with implementation specificsWhile the control techniques are well-described theoretically, consider adding:
- Specific advantages/disadvantages of each approach
- Implementation complexity comparison
- Resource requirements (CPU, memory)
- Real-time performance characteristics
🧰 Tools
🪛 LanguageTool
[duplication] ~15-~15: Possible typo: you repeated a word
Context: ... + k_p e (t-t_s) + k_d \dot{e}(t - t_s) $$ $$ e(t-t_s) = \Delta x(t - t_s) - t_{hw,d}...(ENGLISH_WORD_REPEAT_RULE)
41-44: Address open questions with specific proposalsPlease provide concrete answers to these questions:
- ACC usage: Define specific conditions (straight roads, curves < X degrees, etc.)
- Testing: Outline specific test scenarios (e.g., sudden braking, gradual slowdown)
- Acting component I/O: Define exact message format and fields
- Perception input: Specify required sensor data format and update frequency
67-69: Elaborate on trajectory-based implementation detailsThe new concept needs additional clarification on:
- How many trajectory points to consider (performance vs. accuracy tradeoff)
- Update frequency for trajectory points
- Handling of trajectory interruptions or replanning
- Smooth transitions between different behaviors
71-71: Improve documentation quality and maintainability
- Replace GitHub user attachment link with a relative path to a committed image file
- Fix formatting issues:
- Add missing commas in compound sentences
- Use hyphens in compound adjectives (e.g., "well-known")
- Ensure consistent spacing in links
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
doc/research/paf24/planning/ACC.md(1 hunks)
🧰 Additional context used
🪛 LanguageTool
doc/research/paf24/planning/ACC.md
[uncategorized] ~8-~8: Use a comma before ‘and’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...n our case, both systems might be needed and it might be reasonable to develop two d...
(COMMA_COMPOUND_SENTENCE)
[duplication] ~15-~15: Possible typo: you repeated a word
Context: ... + k_p e (t-t_s) + k_d \dot{e}(t - t_s) $$ $$ e(t-t_s) = \Delta x(t - t_s) - t_{hw,d}...
(ENGLISH_WORD_REPEAT_RULE)
[uncategorized] ~73-~73: Possible missing comma found.
Context: ...c9cb9) Since behaviour is passed as an ID a new enum for behaviours was implement...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~77-~77: Possible missing comma found.
Context: ...stem needs to be discussed. For safety distance the old approach can simply be modified...
(AI_HYDRA_LEO_MISSING_COMMA)
[style] ~77-~77: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...he old approach can simply be modified. For example, (speed / 10)*3 + (speed / 10)²...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[uncategorized] ~77-~77: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...le, (speed / 10)*3 + (speed / 10)² is a well known formula for calculating the braking dis...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
[style] ~78-~78: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ... for calculating the braking distance. For a general speed target we need to take ...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[uncategorized] ~79-~79: Possible missing comma found.
Context: ... braking distance. For a general speed target we need to take the speed and the speed...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~81-~81: Possible missing comma found.
Context: ...oint in the trajectory up to the car in front for example by interpolation. Points fu...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~91-~91: Possible missing article found.
Context: ...c. Subscriber logic could be taken from old implementation. ### Requirements - ob...
(AI_HYDRA_LEO_MISSING_AN)
🪛 Markdownlint (0.35.0)
doc/research/paf24/planning/ACC.md
100-100: null
Spaces inside link text
(MD039, no-space-in-links)
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Outside diff range and nitpick comments (1)
doc/research/paf24/planning/ACC.md (1)
98-101: Expand literature reviewConsider adding recent sources covering:
- Modern ACC implementations in production vehicles
- Stop & Go systems
- Safety distance calculation methodologies
- Trajectory-based control approaches
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
doc/research/paf24/planning/ACC.md(1 hunks)
🧰 Additional context used
🪛 LanguageTool
doc/research/paf24/planning/ACC.md
[uncategorized] ~8-~8: Use a comma before ‘and’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...n our case, both systems might be needed and it might be reasonable to develop two d...
(COMMA_COMPOUND_SENTENCE)
[duplication] ~15-~15: Possible typo: you repeated a word
Context: ... + k_p e (t-t_s) + k_d \dot{e}(t - t_s) $$ $$ e(t-t_s) = \Delta x(t - t_s) - t_{hw,d}...
(ENGLISH_WORD_REPEAT_RULE)
[style] ~77-~77: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...he old approach can simply be modified. For example, (speed / 10)*3 + (speed / 10)²...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[uncategorized] ~77-~77: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...le, (speed / 10)*3 + (speed / 10)² is a well known formula for calculating the braking dis...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
[style] ~78-~78: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ... for calculating the braking distance. For a general speed target we need to take ...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
🔇 Additional comments (1)
doc/research/paf24/planning/ACC.md (1)
8-8: Justify the velocity threshold for ACC vs Stop & Go systems
The 40 km/h threshold for distinguishing between ACC and Stop & Go systems needs justification. Consider:
- Citing industry standards or research that supports this threshold
- Explaining the technical limitations that drive this separation
- Discussing the implications for our implementation
🧰 Tools
🪛 LanguageTool
[uncategorized] ~8-~8: Use a comma before ‘and’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...n our case, both systems might be needed and it might be reasonable to develop two d...
(COMMA_COMPOUND_SENTENCE)
There was a problem hiding this comment.
In general I think this is a good overview of possible improvements to the ACC.
Maybe you should create a diagram for your own solution. Try to use the formula symbols consistent with the other solution candidates mentioned (PID, Fuzzy).
Then it is far easier to comprehend which changes you actually plan on implementing and especially how you want to do so.
doc/research/paf24/planning/ACC.md
Outdated
|
|
||
| Provides a unified control framework to offer both functions: ACC and Stop & Go. | ||
|
|
||
|  |
There was a problem hiding this comment.
For me this link does not work. This might be the github preview however.
Maybe this image should also be directly included.
Especially because here some formula symbols are explained.
There was a problem hiding this comment.
How can I embed an image directly?
made proposed concept more detailed
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (2)
doc/research/paf24/planning/ACC.md (2)
24-32: Enhance MPC implementation detailsThe MPC section would benefit from additional technical details:
- Define the cost function components (tracking error, control effort, etc.)
- Specify the prediction horizon length and its impact on performance
- Document state and control constraints (acceleration limits, jerk bounds, etc.)
81-84: Review sampling approach performance implicationsThe proposed approach of sampling every fifth trajectory point needs careful consideration:
- Memory overhead of storing velocities for all trajectory points
- Computational cost of interpolation
- Impact on real-time performance
Consider implementing a dynamic sampling rate based on vehicle speed and computational resources.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~81-~81: Possible missing comma found.
Context: ..., the way PID calculates velocity seems reasonable since we can treat the trajectory point...(AI_HYDRA_LEO_MISSING_COMMA)
[style] ~81-~81: This phrase is redundant. Consider writing “points” or “times”.
Context: ...reat the trajectory points as different points in time for the sampling time. For example let'...(MOMENT_IN_TIME)
[typographical] ~81-~81: After the expression ‘for example’ a comma is usually used.
Context: ...ints in time for the sampling time. For example let's say we sample every fifth point a...(COMMA_FOR_EXAMPLE)
[typographical] ~83-~83: Consider adding a comma after ‘Theoretically’ for more clarity.
Context: ...the fifth point before the current one. Theoretically this allows us to dynamically adjust th...(RB_LY_COMMA)
🪛 Markdownlint (0.35.0)
81-81: Expected: 0 or 2; Actual: 1
Trailing spaces(MD009, no-trailing-spaces)
81-81: Expected: 300; Actual: 364
Line length(MD013, line-length)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
doc/research/paf24/planning/ACC.md(1 hunks)
🧰 Additional context used
🪛 LanguageTool
doc/research/paf24/planning/ACC.md
[uncategorized] ~8-~8: Use a comma before ‘and’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...n our case, both systems might be needed and it might be reasonable to develop two d...
(COMMA_COMPOUND_SENTENCE)
[duplication] ~15-~15: Possible typo: you repeated a word
Context: ... + k_p e (t-t_s) + k_d \dot{e}(t - t_s) $$ $$ e(t-t_s) = \Delta x(t - t_s) - t_{hw,d}...
(ENGLISH_WORD_REPEAT_RULE)
[uncategorized] ~68-~68: Possible missing comma found.
Context: ...e current behaviour to each point. This way the Acting has more knowledge about wha...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~69-~69: Possible missing comma found.
Context: ...just accordingly in a local manner. For this a new trajectory message type was imple...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~73-~73: Possible missing comma found.
Context: ...c9cb9) Since behaviour is passed as an ID a new enum for behaviours was implement...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~75-~75: Possible missing comma found.
Context: ...ets based on PID. For speeds lower than that a stop and go system can be discussed i...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~77-~77: Possible missing comma found.
Context: ...sed if it is really needed. For safety distance we will use the concept from the FLC gr...
(AI_HYDRA_LEO_MISSING_COMMA)
[style] ~78-~78: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ... use the concept from the FLC graphic. For a general speed target we either take t...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[uncategorized] ~79-~79: Possible missing comma found.
Context: ...m the FLC graphic. For a general speed target we either take the speed of the car in ...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~81-~81: Possible missing comma found.
Context: ..., the way PID calculates velocity seems reasonable since we can treat the trajectory point...
(AI_HYDRA_LEO_MISSING_COMMA)
[style] ~81-~81: This phrase is redundant. Consider writing “points” or “times”.
Context: ...reat the trajectory points as different points in time for the sampling time. For example let'...
(MOMENT_IN_TIME)
[typographical] ~81-~81: After the expression ‘for example’ a comma is usually used.
Context: ...ints in time for the sampling time. For example let's say we sample every fifth point a...
(COMMA_FOR_EXAMPLE)
[typographical] ~83-~83: Consider adding a comma after ‘Theoretically’ for more clarity.
Context: ...the fifth point before the current one. Theoretically this allows us to dynamically adjust th...
(RB_LY_COMMA)
[uncategorized] ~85-~85: Possible missing comma found.
Context: ...me as well if needed. For the distance error we can use the safety distance as the d...
(AI_HYDRA_LEO_MISSING_COMMA)
🪛 Markdownlint (0.35.0)
doc/research/paf24/planning/ACC.md
81-81: Expected: 0 or 2; Actual: 1
Trailing spaces
(MD009, no-trailing-spaces)
85-85: Expected: 0 or 2; Actual: 1
Trailing spaces
(MD009, no-trailing-spaces)
81-81: Expected: 300; Actual: 364
Line length
(MD013, line-length)
🔇 Additional comments (2)
doc/research/paf24/planning/ACC.md (2)
67-70: Verify trajectory message implementation
Need to verify the trajectory message type implementation from PR #511 and its integration with the existing system.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~68-~68: Possible missing comma found.
Context: ...e current behaviour to each point. This way the Acting has more knowledge about wha...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~69-~69: Possible missing comma found.
Context: ...just accordingly in a local manner. For this a new trajectory message type was imple...
(AI_HYDRA_LEO_MISSING_COMMA)
79-80:
Define safety protocols for overtaking initiation
The suggestion that "ACC could initiate overtaking" requires careful consideration:
- Define clear criteria for when overtaking is safe
- Specify required sensor coverage and reliability
- Document interaction with other autonomous systems
- Include fail-safe mechanisms
This is a safety-critical feature that needs thorough design and validation before implementation.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~79-~79: Possible missing comma found.
Context: ...m the FLC graphic. For a general speed target we either take the speed of the car in ...
(AI_HYDRA_LEO_MISSING_COMMA)
doc/research/paf24/planning/ACC.md
Outdated
| For safety distance we will use the concept from the FLC graphic. | ||
|
|
||
| For a general speed target we either take the speed of the car in front or the speed limit, whichever is lower. In cases where the car in front is substantially slower than the speed limit ACC could inititate overtaking. |
There was a problem hiding this comment.
Document safety distance calculation details
The reference to "the concept from the FLC graphic" needs explicit documentation:
- Provide the actual formula to be used
- Define all variables and their units
- Include validation criteria for the safety distance calculation
This is critical for safety and should be thoroughly documented before implementation.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~77-~77: Possible missing comma found.
Context: ...sed if it is really needed. For safety distance we will use the concept from the FLC gr...
(AI_HYDRA_LEO_MISSING_COMMA)
[style] ~78-~78: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ... use the concept from the FLC graphic. For a general speed target we either take t...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[uncategorized] ~79-~79: Possible missing comma found.
Context: ...m the FLC graphic. For a general speed target we either take the speed of the car in ...
(AI_HYDRA_LEO_MISSING_COMMA)
linter
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (3)
doc/research/paf24/planning/ACC.md (3)
7-9: Enhance system requirements documentationThe velocity threshold (40 km/h) for distinguishing between ACC and Stop & Go systems needs justification:
- Add references to industry standards or research supporting this threshold
- Document hardware/sensor requirements for each system
- Specify the impact on control strategy selection
🧰 Tools
🪛 LanguageTool
[uncategorized] ~8-~8: Use a comma before ‘and’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...n our case, both systems might be needed and it might be reasonable to develop two d...(COMMA_COMPOUND_SENTENCE)
24-31: Enhance MPC implementation detailsThe MPC section needs practical implementation guidance:
- Specify the prediction horizon length and its impact
- Define the cost function components (tracking error, control effort, etc.)
- Add computational complexity considerations
- Include pseudo-code or example implementation
107-110: Expand literature reviewConsider adding:
- Recent papers (2020-2024) on ACC implementations
- Industry standards and safety regulations
- Real-world ACC system case studies
- Sources specifically addressing Stop & Go systems
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
doc/research/paf24/planning/ACC.md(1 hunks)
🧰 Additional context used
🪛 LanguageTool
doc/research/paf24/planning/ACC.md
[uncategorized] ~8-~8: Use a comma before ‘and’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...n our case, both systems might be needed and it might be reasonable to develop two d...
(COMMA_COMPOUND_SENTENCE)
[duplication] ~15-~15: Possible typo: you repeated a word
Context: ... + k_p e (t-t_s) + k_d \dot{e}(t - t_s) $$ $$ e(t-t_s) = \Delta x(t - t_s) - t_{hw,d}...
(ENGLISH_WORD_REPEAT_RULE)
[style] ~78-~78: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ... use the concept from the FLC graphic. For a general speed target we either take t...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~81-~81: This phrase is redundant. Consider writing “points” or “times”.
Context: ...reat the trajectory points as different points in time for the sampling time. For example let'...
(MOMENT_IN_TIME)
[typographical] ~82-~82: After the expression ‘for example’ a comma is usually used.
Context: ...ints in time for the sampling time. For example let's say we sample every fifth point a...
(COMMA_FOR_EXAMPLE)
[typographical] ~84-~84: Consider adding a comma after ‘Theoretically’ for more clarity.
Context: ...the fifth point before the current one. Theoretically this allows us to dynamically adjust th...
(RB_LY_COMMA)
🔇 Additional comments (1)
doc/research/paf24/planning/ACC.md (1)
81-85: Validate trajectory sampling approach
The proposed sampling of every fifth point needs validation:
- Justify the sampling interval choice
- Address potential issues with varying vehicle speeds
- Document the impact on control system stability
- Consider adaptive sampling based on speed/conditions
🧰 Tools
🪛 LanguageTool
[style] ~81-~81: This phrase is redundant. Consider writing “points” or “times”.
Context: ...reat the trajectory points as different points in time for the sampling time. For example let'...
(MOMENT_IN_TIME)
[typographical] ~82-~82: After the expression ‘for example’ a comma is usually used.
Context: ...ints in time for the sampling time. For example let's say we sample every fifth point a...
(COMMA_FOR_EXAMPLE)
[typographical] ~84-~84: Consider adding a comma after ‘Theoretically’ for more clarity.
Context: ...the fifth point before the current one. Theoretically this allows us to dynamically adjust th...
(RB_LY_COMMA)
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Outside diff range and nitpick comments (1)
doc/research/paf24/planning/ACC.md (1)
24-32: Enhance MPC section with implementation detailsThe MPC overview is good, but consider adding:
- Cost function formulation for ACC
- Prediction horizon selection guidelines
- Computational complexity considerations
Would you like me to help draft these additional implementation details?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
doc/research/paf24/planning/ACC.md(1 hunks)
🧰 Additional context used
🪛 LanguageTool
doc/research/paf24/planning/ACC.md
[uncategorized] ~8-~8: Use a comma before ‘and’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...n our case, both systems might be needed and it might be reasonable to develop two d...
(COMMA_COMPOUND_SENTENCE)
[duplication] ~15-~15: Possible typo: you repeated a word
Context: ... + k_p e (t-t_s) + k_d \dot{e}(t - t_s) $$ $$ e(t-t_s) = \Delta x(t - t_s) - t_{hw,d}...
(ENGLISH_WORD_REPEAT_RULE)
[style] ~82-~82: This phrase is redundant. Consider writing “points” or “times”.
Context: ...reat the trajectory points as different points in time for the sampling time. For example let'...
(MOMENT_IN_TIME)
[typographical] ~83-~83: After the expression ‘for example’ a comma is usually used.
Context: ...ints in time for the sampling time. For example let's say we sample every fifth point a...
(COMMA_FOR_EXAMPLE)
[typographical] ~85-~85: Consider adding a comma after ‘Theoretically’ for more clarity.
Context: ...the fifth point before the current one. Theoretically this allows us to dynamically adjust th...
(RB_LY_COMMA)
added diagram for presentation
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Outside diff range and nitpick comments (1)
doc/research/paf24/planning/ACC.md (1)
89-90: Define safety margins for velocity calculationsThe 5% threshold for optimal safety distance needs additional safety considerations:
- What is the minimum absolute safety margin regardless of percentage?
- How are sudden changes in the leading vehicle's velocity handled?
Consider adding a section on safety margins:
+Safety considerations for velocity calculations: +1. Minimum absolute safety margin: 2 meters regardless of percentage +2. Maximum acceleration/deceleration rates: ±2.5 m/s² +3. Hysteresis in distance threshold to prevent oscillation🧰 Tools
🪛 LanguageTool
[uncategorized] ~89-~89: Possible missing comma found.
Context: ...afety distance. For points further than that we simply use the desired general speed...(AI_HYDRA_LEO_MISSING_COMMA)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
doc/research/paf24/planning/ACC.md(1 hunks)
🧰 Additional context used
🪛 LanguageTool
doc/research/paf24/planning/ACC.md
[uncategorized] ~8-~8: Use a comma before ‘and’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...n our case, both systems might be needed and it might be reasonable to develop two d...
(COMMA_COMPOUND_SENTENCE)
[duplication] ~15-~15: Possible typo: you repeated a word
Context: ... + k_p e (t-t_s) + k_d \dot{e}(t - t_s) $$ $$ e(t-t_s) = \Delta x(t - t_s) - t_{hw,d}...
(ENGLISH_WORD_REPEAT_RULE)
[uncategorized] ~68-~68: Possible missing comma found.
Context: ...e current behaviour to each point. This way the Acting has more knowledge about wha...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~69-~69: Possible missing comma found.
Context: ...just accordingly in a local manner. For this a new trajectory message type was imple...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~73-~73: Possible missing comma found.
Context: ...c9cb9) Since behaviour is passed as an ID a new enum for behaviours was implement...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~75-~75: Possible missing comma found.
Context: ...ets based on PID. For speeds lower than that a stop and go system can be discussed i...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~80-~80: Possible missing comma found.
Context: ...imit in that case. For a general speed target we either take the speed of the car in ...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~80-~80: Possible missing comma found.
Context: ... is substantially slower than the speed limit ACC could inititate overtaking. Since ...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~82-~82: Possible missing comma found.
Context: ..., the way PID calculates velocity seems reasonable since we can treat the trajectory point...
(AI_HYDRA_LEO_MISSING_COMMA)
[style] ~82-~82: This phrase is redundant. Consider writing “points” or “times”.
Context: ...reat the trajectory points as different points in time for the sampling time. For example let'...
(MOMENT_IN_TIME)
[typographical] ~83-~83: After the expression ‘for example’ a comma is usually used.
Context: ...ints in time for the sampling time. For example let's say we sample every fifth point a...
(COMMA_FOR_EXAMPLE)
[typographical] ~85-~85: Consider adding a comma after ‘Theoretically’ for more clarity.
Context: ...the fifth point before the current one. Theoretically this allows us to dynamically adjust th...
(RB_LY_COMMA)
[uncategorized] ~87-~87: Possible missing comma found.
Context: ...me as well if needed. For the distance error we can use the safety distance as the d...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~89-~89: Possible missing comma found.
Context: ...afety distance. For points further than that we simply use the desired general speed...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~101-~101: Possible missing article found.
Context: ...c. Subscriber logic could be taken from old implementation. Implement PID logic #...
(AI_HYDRA_LEO_MISSING_AN)
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (1)
doc/research/paf24/planning/ACC.md (1)
8-9: Justify the speed threshold selectionThe 40 km/h threshold between ACC and Stop & Go systems needs justification. Consider adding:
- Why this specific threshold was chosen
- How it aligns with urban/highway driving scenarios
- Impact on control system design
-In general, classic ACC systems are designed for higher velocity (e.g. > 40 km/h). Apart from that, there are Stop & Go systems that support lower velocites (e.g. < 40 km/h). In our case, both systems might be needed and it might be reasonable to develop two different systems for ACC and Stop & Go. -The threshold to distinguish between the two systems has to be chosen reasonably. +In general, classic ACC systems are designed for higher velocity (v > 40 km/h), aligning with typical highway driving scenarios where steady-state following is more common. Stop & Go systems support lower velocities (v ≤ 40 km/h), optimized for urban traffic patterns with frequent stops. This threshold was chosen based on: +- Typical urban speed limits (30-50 km/h) +- Different control requirements for low-speed following +- Industry standard implementations + +In our case, both systems are needed due to the diverse operating environments our vehicle will encounter.🧰 Tools
🪛 LanguageTool
[uncategorized] ~8-~8: Use a comma before ‘and’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...n our case, both systems might be needed and it might be reasonable to develop two d...(COMMA_COMPOUND_SENTENCE)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
doc/research/paf24/planning/ACC.md(1 hunks)
🧰 Additional context used
🪛 LanguageTool
doc/research/paf24/planning/ACC.md
[uncategorized] ~8-~8: Use a comma before ‘and’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...n our case, both systems might be needed and it might be reasonable to develop two d...
(COMMA_COMPOUND_SENTENCE)
[duplication] ~16-~16: Possible typo: you repeated a word
Context: ...t} e(\tau) d\tau + k_d \dot{e}(t - t_s) $$ $$ e(t-t_s) = \Delta x(t - t_s) - t_{hw,d}...
(ENGLISH_WORD_REPEAT_RULE)
[uncategorized] ~65-~65: Possible missing comma found.
Context: ...just accordingly in a local manner. For this a new trajectory message type was imple...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~71-~71: Possible missing comma found.
Context: ...ets based on PID. For speeds lower than that a stop and go system can be discussed i...
(AI_HYDRA_LEO_MISSING_COMMA)
[style] ~78-~78: This phrase is redundant. Consider writing “points” or “times”.
Context: ...reat the trajectory points as different points in time for the sampling time. For example let'...
(MOMENT_IN_TIME)
[typographical] ~79-~79: After the expression ‘for example’ a comma is usually used.
Context: ...ints in time for the sampling time. For example let's say we sample every fifth point a...
(COMMA_FOR_EXAMPLE)
[typographical] ~81-~81: Consider adding a comma after ‘Theoretically’ for more clarity.
Context: ...the fifth point before the current one. Theoretically this allows us to dynamically adjust th...
(RB_LY_COMMA)
[uncategorized] ~83-~83: Possible missing comma found.
Context: ...me as well if needed. For the distance error we can use the safety distance as the d...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~85-~85: Possible missing comma found.
Context: ...afety distance. For points further than that we simply use the desired general speed...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~97-~97: Possible missing article found.
Context: ...c. Subscriber logic could be taken from old implementation. Implement PID logic #...
(AI_HYDRA_LEO_MISSING_AN)
🔇 Additional comments (2)
doc/research/paf24/planning/ACC.md (2)
101-105: 🛠️ Refactor suggestion
Expand system requirements section
The requirements section needs significant expansion to include:
- Safety requirements:
- Minimum safe following distance
- Maximum deceleration rates
- Emergency behavior specifications
- Performance requirements:
- Control loop frequency
- Response time limits
- Speed control accuracy
- System constraints:
- Sensor limitations
- Processing delays
- Communication latency
-### Requirements
-
-- obstacle speed
-- obstacle distance
+### System Requirements
+
+1. Safety Requirements:
+ - Minimum following distance: 2 seconds time gap
+ - Maximum deceleration: 3 m/s²
+ - Emergency behavior specifications
+ - Fail-safe modes
+
+2. Performance Requirements:
+ - Control loop frequency: ≥ 20 Hz
+ - Response time: ≤ 100 ms
+ - Speed control accuracy: ±2 km/h
+ - Distance measurement accuracy: ±0.5m
+
+3. Input Requirements:
+ - Obstacle speed (range: 0-120 km/h, accuracy: ±1 km/h)
+ - Obstacle distance (range: 0-150m, accuracy: ±0.5m)
+ - Host vehicle speed and acceleration
+
+4. System Constraints:
+ - Sensor update rate: ≥ 10 Hz
+ - Processing delay: ≤ 50 ms
+ - Communication latency: ≤ 20 msLikely invalid or redundant comment.
78-85: Add error handling for trajectory sampling
The PID-based velocity calculation using trajectory points needs robust error handling:
- What happens if there aren't enough points for the sampling interval?
- How are trajectory discontinuities handled?
- How is the interpolation validated?
🧰 Tools
🪛 LanguageTool
[style] ~78-~78: This phrase is redundant. Consider writing “points” or “times”.
Context: ...reat the trajectory points as different points in time for the sampling time. For example let'...
(MOMENT_IN_TIME)
[typographical] ~79-~79: After the expression ‘for example’ a comma is usually used.
Context: ...ints in time for the sampling time. For example let's say we sample every fifth point a...
(COMMA_FOR_EXAMPLE)
[typographical] ~81-~81: Consider adding a comma after ‘Theoretically’ for more clarity.
Context: ...the fifth point before the current one. Theoretically this allows us to dynamically adjust th...
(RB_LY_COMMA)
[uncategorized] ~83-~83: Possible missing comma found.
Context: ...me as well if needed. For the distance error we can use the safety distance as the d...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~85-~85: Possible missing comma found.
Context: ...afety distance. For points further than that we simply use the desired general speed...
(AI_HYDRA_LEO_MISSING_COMMA)
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
doc/research/paf24/planning/ACC.md(1 hunks)
🧰 Additional context used
🪛 LanguageTool
doc/research/paf24/planning/ACC.md
[uncategorized] ~8-~8: Use a comma before ‘and’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...n our case, both systems might be needed and it might be reasonable to develop two d...
(COMMA_COMPOUND_SENTENCE)
[duplication] ~16-~16: Possible typo: you repeated a word
Context: ...t} e(\tau) d\tau + k_d \dot{e}(t - t_s) $$ $$ e(t-t_s) = \Delta x(t - t_s) - t_{hw,d}...
(ENGLISH_WORD_REPEAT_RULE)
[uncategorized] ~65-~65: Possible missing comma found.
Context: ...just accordingly in a local manner. For this a new trajectory message type was imple...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~69-~69: Possible missing comma found.
Context: ...c9cb9) Since behaviour is passed as an ID a new enum for behaviours was implement...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~71-~71: Possible missing comma found.
Context: ...ets based on PID. For speeds lower than that a stop and go system can be discussed i...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~76-~76: Possible missing comma found.
Context: ...imit in that case. For a general speed target we either take the speed of the car in ...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~76-~76: Possible missing comma found.
Context: ... is substantially slower than the speed limit ACC could inititate overtaking. Since ...
(AI_HYDRA_LEO_MISSING_COMMA)
[style] ~78-~78: This phrase is redundant. Consider writing “points” or “times”.
Context: ...reat the trajectory points as different points in time for the sampling time. For example let'...
(MOMENT_IN_TIME)
[typographical] ~79-~79: After the expression ‘for example’ a comma is usually used.
Context: ...ints in time for the sampling time. For example let's say we sample every fifth point a...
(COMMA_FOR_EXAMPLE)
[typographical] ~81-~81: Consider adding a comma after ‘Theoretically’ for more clarity.
Context: ...the fifth point before the current one. Theoretically this allows us to dynamically adjust th...
(RB_LY_COMMA)
[uncategorized] ~83-~83: Possible missing comma found.
Context: ...me as well if needed. For the distance error we can use the safety distance as the d...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~85-~85: Possible missing comma found.
Context: ...afety distance. For points further than that we simply use the desired general speed...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~97-~97: Possible missing article found.
Context: ...c. Subscriber logic could be taken from old implementation. Implement PID logic #...
(AI_HYDRA_LEO_MISSING_AN)
🔇 Additional comments (2)
doc/research/paf24/planning/ACC.md (2)
8-9: 🛠️ Refactor suggestion
Specify and justify the speed threshold
The threshold between ACC and Stop & Go systems needs more precise specification and justification:
- Define exact threshold value instead of "e.g. > 40 km/h"
- Explain the rationale behind choosing 40 km/h
- Document different control requirements for each speed range
-In general, classic ACC systems are designed for higher velocity (e.g. > 40 km/h). Apart from that, there are Stop & Go systems that support lower velocites (e.g. < 40 km/h).
+In general, classic ACC systems are designed for higher velocity (v > 40 km/h), where aerodynamic effects and longer stopping distances require different control strategies. Stop & Go systems support lower velocities (v ≤ 40 km/h), where precise distance control and quick response times are crucial for urban traffic scenarios.Likely invalid or redundant comment.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~8-~8: Use a comma before ‘and’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...n our case, both systems might be needed and it might be reasonable to develop two d...
(COMMA_COMPOUND_SENTENCE)
78-85: Validate trajectory sampling approach and add error handling
The PID sampling strategy needs validation and error handling:
- Edge cases for trajectory sampling:
- Insufficient points for sampling interval
- Trajectory discontinuities
- Invalid velocity calculations
- Distance prediction accuracy:
- Validation of predicted distances
- Error accumulation prevention
🧰 Tools
🪛 LanguageTool
[style] ~78-~78: This phrase is redundant. Consider writing “points” or “times”.
Context: ...reat the trajectory points as different points in time for the sampling time. For example let'...
(MOMENT_IN_TIME)
[typographical] ~79-~79: After the expression ‘for example’ a comma is usually used.
Context: ...ints in time for the sampling time. For example let's say we sample every fifth point a...
(COMMA_FOR_EXAMPLE)
[typographical] ~81-~81: Consider adding a comma after ‘Theoretically’ for more clarity.
Context: ...the fifth point before the current one. Theoretically this allows us to dynamically adjust th...
(RB_LY_COMMA)
[uncategorized] ~83-~83: Possible missing comma found.
Context: ...me as well if needed. For the distance error we can use the safety distance as the d...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~85-~85: Possible missing comma found.
Context: ...afety distance. For points further than that we simply use the desired general speed...
(AI_HYDRA_LEO_MISSING_COMMA)
| It calculates the current control action by solving an online, iterative and finite-horizon optimization of the model. | ||
| Procedure: | ||
|
|
||
| 1. Prediction of future system states based on current states | ||
| 2. Computation of the cost function for a finite time horizon in the future | ||
| 3. Implementation of the first step of the solved control sequence | ||
| 4. Application of the feedback control loop to compensate for the predictive error and model inaccuracy | ||
| 5. Sampling of new current states and repitition of the process | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance MPC section with implementation specifics
The MPC section needs more detailed information to be implementable:
- Define the prediction model
- Specify the cost function components
- Document the optimization constraints
- Provide the control horizon parameters
Would you like me to help draft the detailed MPC implementation specifications?
| - safe distance calculation is currently not correct, uses speed + (speed * 0.36)² which results in wrong distances | ||
| - if car in front of us ignores speed limits we ignore them as well | ||
| - some parts of unstuck routine are in ACC and need to be refactored | ||
| - same goes for publishing of current waypoint, this should not be in ACC | ||
|
|
There was a problem hiding this comment.
Critical: Address safety distance calculation and architectural issues
Several critical issues require immediate attention:
- Safety distance calculation is incorrect and unsafe
- Component responsibilities are mixed:
- Unstuck routine in ACC
- Waypoint publishing in ACC
Proposed actions:
- Create a dedicated
SafetyDistanceclass with physics-based calculations - Move unstuck routine to a behavior module
- Implement waypoint management in planning component
- Add comprehensive unit tests
Would you like me to help draft the SafetyDistance class implementation?
| - obstacle speed | ||
| - obstacle distance |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Expand system requirements with specific metrics
The requirements section needs quantitative metrics:
- Obstacle Detection:
- Detection range
- Speed measurement accuracy
- Position accuracy
- Update rate
- System Performance:
- Control loop frequency
- Response time
- Speed control accuracy
- Safety Requirements:
- Minimum time gap
- Maximum deceleration
- Fail-safe behavior
Would you like me to help draft detailed requirements specifications?
Description
This includes research on general ACC concepts, an evaluation of the current implementation and a concept for remaking it.
Fixes #490
Type of change
Please delete options that are not relevant.
Does this PR introduce a breaking change?
No
Most important changes
MD file
Checklist:
Summary by CodeRabbit
New Features
Behaviourenumeration to enhance utility functions.trajectory messagetype to improve the Adaptive Cruise Control (ACC) system's functionality.Documentation