-
Notifications
You must be signed in to change notification settings - Fork 60
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Merge pull request #1185 from tier4/feature/new_release_flow
Feature/new release flow
- Loading branch information
Showing
26 changed files
with
389 additions
and
151 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file was deleted.
Oops, something went wrong.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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. |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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. |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
# Description | ||
|
||
## Abstract | ||
|
||
Corrected "Vehicle" incorrectly spelled as "Vehicle". |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file was deleted.
Oops, something went wrong.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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 |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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 |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,4 @@ | ||
name: DocumentationLinkCheck | ||
name: Documentation link check | ||
|
||
on: | ||
repository_dispatch: | ||
|
@@ -13,7 +13,7 @@ jobs: | |
steps: | ||
- uses: actions/checkout@v3 | ||
|
||
- name: Link Checker | ||
- name: Check documentation link | ||
id: lychee | ||
uses: lycheeverse/[email protected] | ||
with: | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,4 @@ | ||
name: InterfaceUpdateNotification | ||
name: Interface update notification | ||
on: | ||
pull_request: | ||
branches: | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,4 @@ | ||
name: LineLint | ||
name: Line lint | ||
on: | ||
workflow_dispatch: | ||
push: | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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 }} |
Oops, something went wrong.