Skip to content

Commit

Permalink
Merge remote-tracking branch 'origin/master' into feature/slope_vehic…
Browse files Browse the repository at this point in the history
…le_model
  • Loading branch information
HansRobo committed Feb 15, 2024
2 parents 184ee8e + 17626af commit 679199c
Show file tree
Hide file tree
Showing 77 changed files with 823 additions and 176 deletions.
4 changes: 1 addition & 3 deletions .github/ISSUE_TEMPLATE/bug_report.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,6 @@ A clear and concise description of what you expected to happen.
If applicable, add screenshots to help explain your problem.

**Desktop (please complete the following information):**
- OS: [e.g. iOS]
- Browser [e.g. chrome, safari]
- Version [e.g. 22]
- Ubuntu Version [e.g. 22.04]
- ROS 2 version
- DDS
13 changes: 0 additions & 13 deletions .github/ISSUE_TEMPLATE/release.md

This file was deleted.

39 changes: 31 additions & 8 deletions .github/PULL_REQUEST_TEMPLATE.md
Original file line number Diff line number Diff line change
@@ -1,13 +1,36 @@
## Types of PR
# Description

- [ ] New Features
- [ ] Upgrade of existing features
- [ ] Bugfix
> Please check examples and comment out this sentence. Minimal example is [here](pull_request_samples/example_simple.md) and detailed example is [here](pull_request_samples/example_detail.md)
## Link to the issue
## Abstract

## Description
> [Required] This section is required, keep it short, clear and to the point.
> Delete this sentence and explain this pull request shortly.
## How to review this PR.
## Background

## Others
> [Optional] If there is no background information that needs explanation (e.g., just a typo correction, etc.), you can skip this section.
> Delete this sentence and explain the circumstances that led to this pull request being sent.
## Details

> [Optional] If there are only differences whose effects are so obvious that no explanation is needed, or if there are no differences in the code (e.g., documentation additions), you can skip this section.
> Delete this sentence and describe this pull request.
> For example, it is desirable to describe the specifications of added functions, and detailed explanations of bugs that have been fixed.
## References

> [Optional] If the referenced material does not exist, you can skip this section.
> Describe any standards, algorithms, books, articles, etc. that you referenced when creating the pull request. If there is any novelty, mention it.
# Destructive Changes

> [Optional] If no destructive change exists, you can skip this section.
> Include a description of the changes and a migration guide and send the pull request with a bump major label. (Example : https://github.com/tier4/scenario_simulator_v2/pull/1139)
> Otherwise, skip the "Destructive Changes" section and make sure this pull request is labeled `bump minor` or `bump patch`.
# Known Limitations

> [Optional] If there are no known limitations that you can think of, you can skip this section. If there are any limitations on the features or fixes added by this pull request, delete this sentence and clearly describe them.
> For example, the lane matching algorithm currently (1/25/2024) employed is unable to match Entity that is heavily tilted with respect to the lane, and it is difficult to throw an exception.
> If the developer is aware of the existence of such problems or limitations, describe them in detail. The problems or limitation should be listed as an Issue on GitHub and a link to it should be provided in this section.
85 changes: 85 additions & 0 deletions .github/pull_request_samples/example_detail.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
# Description

## Abstract

Fixed bags below.

- Removed equal operators for geometry_msgs::msg::Point and geometry_msgs::msg::Vector3, because they were ambiguous.
- Fixed the bug which caused the intersection functions using vector to look past the last element of the vector and return wrong results.
- Fixed a bug where the LineSegment class could be constructed with a geometry_msgs::msg::Vector3 of size = 0. This lead to initialization of end_point with nan values.
- Fixed the getMinValue and getMaxValue bug where when an empty vector was passed the function tried to de-reference vector.end() and the whole program crashed.
- Fixed a getPolygon bug which caused the function to generate incorrect number of points.
- Added support for negative curvature values which were already supported by HermiteCurve. This incompatibility lead to errors.
- Fixed spline collision implementation error which caused spline to add normalized length to absolute lengths which is incorrect.
- Fixed CatmullRomSubspline collision detection by enabling the HermiteCurve and CatmullRomSpline to have multiple collisions detected and then choosing in the subspline the collisions that occur inside the subspline.

## Background

Fixed several residual problems in the geometric calculation library that were causing incorrect scenario execution results.

## Details

This PR fixes how the length of the curve is computed

| Before fix | After fix |
| --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
| Lengths of the first two Curves (A and B) are calculated correctly (because these are full Curve lengths), but the distance in the third Curve (C) is calculated as half of the normalized length of the Curve (0.5). This value is added and the result and distance to the collision in the spline is equal to 20.5 | Lengths of the first two Curves (A and B) are calculated correctly (because these are full Curve lengths) and the distance in the third Curve (C) is also calculated correctly because the collision distance is being denormalized (multiplied by the full length of the Curve) which is equal to 5 (0.5 * 10).This value is added and the result and distance to the collision in the spline is equal to 25 |
| ![RJD-753_base](https://github.com/tier4/scenario_simulator_v2/assets/87643052/18b0f1a5-5370-4cf3-a60a-c1af05448d50) | ![RJD-753_eval](https://github.com/tier4/scenario_simulator_v2/assets/87643052/87570089-bf77-4be8-b950-e3f1fb8499a9) |

## References

- [determine-arc-length-of-a-catmull-rom-spline-to-move-at-a-constant-speed](https://gamedev.stackexchange.com/questions/14985/determine-arc-length-of-a-catmull-rom-spline-to-move-at-a-constant-speed)
- This link is an example and is not directly related to this sample.

# Destructive Changes

| traffic_simulator/OpenSCENARIO features | Severity | Fix |
|----------------------------------------------------------|-------------|--------------|
| FollowFrontEntityAction | low - distance kept to the front entity might be a bit larger which might influence scenarios which are relying on specific behavior in this action but it does not seem to be a lot of scenarios | Conditions which check this distance should be increased accordingly - trial and error seem to be the best way to find how much change the condition without changing the essence of the scenario|
| Computing the distance to stop_line or other "obstacles" | low - it will increase this distance making it a bit safer but if the scenario conditions rely on this value it might cause issues. No scenario, however, was noticed to be influenced by that to the extent that caused them to fail | Conditions that check this distance should be increased accordingly - trial and error seem to be the best way to find how much change the condition without changing the essence of the scenario |
| Distance-based lane change | Medium - regression is known which was caused by the fix. Cut-in scenarios which are written in a rather strict way might be influenced by this change | Decreasing the distance on which the lane change should occur or change the speed of the vehicles taking part in the scenario - example in the section below |

This scenario usually passes without the fix but always fails after the fix is added. The scenario is based on [shinjuku_map](https://github.com/tier4/AWSIM/releases/download/v1.2.0/shinjuku_map.zip).

[scenario.yml.txt](https://github.com/tier4/scenario_simulator_v2/files/13707779/scenario.yml.txt)

An issue in this scenario is lane change action:

```
LaneChangeAction:
LaneChangeActionDynamics:
dynamicsDimension: distance
value: 10
dynamicsShape: cubic
LaneChangeTarget:
AbsoluteTargetLane:
value: '37'
```

To fix it:
- The speed of Ego vehicle can be decreased slightly
- The speed of NPC motorcycle can be increased slightly
- The lane change distance can be decreased slightly

The exact amount of the change of the values above is hard to estimate because it is very dependent on the specific scenario - like the vehicle position within the lanelet might influence how much the normalized lanelet length was different than not normalized length.

Here is the scenario that uses the third possible fix - decreasing the distance on which the lane change action takes place. One meter decrease makes the scenario pass again.

```
LaneChangeAction:
LaneChangeActionDynamics:
dynamicsDimension: distance
value: 9
dynamicsShape: cubic
LaneChangeTarget:
AbsoluteTargetLane:
value: '37'
```

Full fixed scenario
[scenario.yml.txt](https://github.com/tier4/scenario_simulator_v2/files/13707839/scenario.yml.txt)

# Known Limitations

- If the curvature is very large, the calculation may fail.
- This link is an example and is not directly related to this sample.
5 changes: 5 additions & 0 deletions .github/pull_request_samples/example_simple.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
# Description

## Abstract

Corrected "Vehicle" incorrectly spelled as "Vehicle".
6 changes: 3 additions & 3 deletions .github/workflows/BuildAndRun.yaml
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
name: BuildAndRun
name: Build and run

on:
workflow_dispatch:
Expand All @@ -19,7 +19,7 @@ on:
- master
jobs:
job1:
name: BuildAndRun
name: Build and run
runs-on: ubuntu-22.04
timeout-minutes: 180
container: ros:${{ matrix.rosdistro }}
Expand All @@ -30,7 +30,7 @@ jobs:
matrix:
rosdistro: [humble]
steps:
- name: suppress warnings
- name: Suppress warnings
run: |
git config --global --add safe.directory '*'
Expand Down
44 changes: 0 additions & 44 deletions .github/workflows/CheckAndCommentAction.yaml

This file was deleted.

29 changes: 29 additions & 0 deletions .github/workflows/CheckBranchUpToDate.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
name: Check branch up to date
on:
pull_request:
branches:
- master
merge_group:
types: [checks_requested]

jobs:
check_branch_up_to_date:
name: Check branch up to date
timeout-minutes: 10
runs-on: ubuntu-22.04
steps:
- name: checkout
uses: actions/checkout@v2
with:
fetch-depth: 0
fetch-tags: true
- name: Check if branches are up to date
shell: bash
run: |
BASE=$(git merge-base origin/master HEAD)
if [ $BASE = $(git rev-parse origin/master) ]; then
echo "Branches are up to date, skipping merge."
else
echo "Branches are not up to date, merge is required."
exit 1
fi
23 changes: 23 additions & 0 deletions .github/workflows/CheckLabel.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
name: Check label
on:
pull_request:
branches:
- master
label:
types:
- created
- edited
- labeled
- unlabeled

jobs:
check_label:
name: Check label
runs-on: ubuntu-22.04
timeout-minutes: 10
steps:
- name: Check required label
if: (!contains(github.event.pull_request.labels.*.name, 'bump patch')) &&
(!contains(github.event.pull_request.labels.*.name, 'bump minor')) &&
(!contains(github.event.pull_request.labels.*.name, 'bump major'))
run: exit 1
4 changes: 2 additions & 2 deletions .github/workflows/Docker.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ on:
types: [published]
jobs:
job1:
name: Build Docker Image
name: Build Docker image
runs-on: ubuntu-20.04
timeout-minutes: 180
strategy:
Expand All @@ -28,7 +28,7 @@ jobs:
steps:
- uses: actions/checkout@v3

- name: Setup Docker Buildx
- name: Setup Docker buildx
uses: docker/setup-buildx-action@v2

- name: Login to GitHub Container Registry
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/Documentation.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ on:

jobs:
generate:
name: Generate Documentation
name: Generate documentation
runs-on: ubuntu-22.04
steps:
- uses: actions/checkout@v2-beta
Expand Down
4 changes: 2 additions & 2 deletions .github/workflows/DocumentationLinkCheck.yaml
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
name: DocumentationLinkCheck
name: Documentation link check

on:
repository_dispatch:
Expand All @@ -13,7 +13,7 @@ jobs:
steps:
- uses: actions/checkout@v3

- name: Link Checker
- name: Check documentation link
id: lychee
uses: lycheeverse/[email protected]
with:
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/InterfaceUpdateNotification.yaml
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
name: InterfaceUpdateNotification
name: Interface update notification
on:
pull_request:
branches:
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/LineLint.yaml
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
name: LineLint
name: Line lint
on:
workflow_dispatch:
push:
Expand Down
36 changes: 36 additions & 0 deletions .github/workflows/PostCheckList.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
name: Post check list
on:
pull_request:
branches:
- master
types: [opened, reopened]

jobs:
post_check_list:
name: Post check list
timeout-minutes: 10
runs-on: ubuntu-22.04
permissions:
actions: write
checks: write
contents: write
deployments: write
issues: write
packages: write
pull-requests: write
repository-projects: write
security-events: write
statuses: write
steps:
- uses: alawiii521/[email protected]
with:
comment: |
# Checklist for reviewers ☑️
All references to "You" in the following text refer to the code reviewer.
- [ ] Is this pull request written in a way that is easy to read from a third-party perspective?
- [ ] Is there sufficient information (background, purpose, specification, algorithm description, list of disruptive changes, and migration guide) in the description of this pull request?
- [ ] If this pull request contains a destructive change, does this pull request contain the migration guide?
- [ ] Labels of this pull request are valid?
- [ ] All unit tests/integration tests are included in this pull request? If you think adding test cases is unnecessary, please describe why and cross out this line.
- [ ] The documentation for this pull request is enough? If you think adding documents for this pull request is unnecessary, please describe why and cross out this line.
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
Loading

0 comments on commit 679199c

Please sign in to comment.