Merge commit '253fa785573217ad3a6bde882724a9e35a0c99ed' into feature/…

…synchronized_action

.github/ISSUE_TEMPLATE/bug_report.md

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

.github/PULL_REQUEST_TEMPLATE.md

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

.github/msgs_checker.py

@@ -0,0 +1,150 @@
+#!/usr/bin/python3
+
+from glob import glob
+import re
+from dataclasses import dataclass
+from typing import Set, Dict
+from catkin_pkg.packages import find_packages
+from catkin_pkg.package import Package
+from subprocess import check_output
+from pathlib import Path
+from itertools import chain
+from json import dumps
+import requests
+import os
+
+
+# cspell: ignore srvs
+@dataclass
+class UsedPackage:
+    msgs: Set[str]
+    srvs: Set[str]
+
+
+# u_int8 -> UInt8
+def to_camel_case(snake_str: str):
+    return "".join(x.capitalize() for x in snake_str.lower().split("_"))
+
+
+def either_glob(str: str):
+    return "".join([f"[{c.lower()}{c.upper()}]" if c.isalpha() else c for c in str])
+
+
+def main():
+    # Glob all *.cpp / *.hpp files
+    files = glob("**/*.cpp", recursive=True) + glob("**/*.hpp", recursive=True)
+
+    # Message include regex
+    msg_include_regex = re.compile(r"#include\s*[<\"](.+_msgs)/msg/(.+)\.hpp[>\"]")
+    srv_include_regex = re.compile(r"#include\s*[<\"](.+_srv)/srv/(.+)\.hpp[>\"]")
+
+    # Check for `#include` statements
+    used_packages: Dict[str, UsedPackage] = {}
+    for file in files:
+        with open(file, "r") as f:
+            lines = f.readlines()
+            for line in lines:
+                msg_match = msg_include_regex.match(line)
+                srv_match = srv_include_regex.match(line)
+                if msg_match:
+                    package = msg_match.group(1)
+                    msg = to_camel_case(msg_match.group(2))
+
+                    if package not in used_packages:
+                        used_packages[package] = UsedPackage(set(), set())
+                    used_packages[package].msgs.add(msg)
+
+                if srv_match:
+                    package = srv_match.group(1)
+                    srv = to_camel_case(srv_match.group(2))
+
+                    if package not in used_packages:
+                        used_packages[package] = UsedPackage(set(), set())
+                    used_packages[package].srvs.add(srv)
+
+    # Expect external packages to be in the workspace...
+    pkgs: Dict[str, Package] = find_packages(".")
+    known_packages = [pkg.name for pkg in pkgs.values()]
+    unknown_packages = set(used_packages.keys()) - set(known_packages)
+    if len(unknown_packages) > 0:
+        print(f"Unknown packages: {unknown_packages}")
+
+    updated_patches = []
+
+    for path, pkg in pkgs.items():
+        if pkg.name not in used_packages:
+            continue
+
+        used_msgs = used_packages[pkg.name].msgs
+        used_srvs = used_packages[pkg.name].srvs
+        used_names = used_msgs | used_srvs
+
+        base_path = Path(path)
+        check_paths = chain(
+            *(
+                [base_path.rglob(f"**/{either_glob(msg)}.msg") for msg in used_names]
+                + [base_path.rglob(f"**/{either_glob(msg)}.srv") for msg in used_names]
+                + [base_path.rglob(f"**/{either_glob(msg)}.idl") for msg in used_names]
+            )
+        )
+        existing_files = [path for path in check_paths if path.exists()]
+        existing_files = [str(path.relative_to(base_path)) for path in existing_files]
+
+        if len(existing_files) == 0:
+            print(
+                f"Package {pkg.name} has no messages or services {used_msgs} {used_srvs}"
+            )
+            continue
+
+        # Call `git log` for 7 days history
+        # cspell: ignore oneline
+        log = (
+            check_output(
+                [
+                    "git",
+                    "log",
+                    "--since",
+                    "7.days",
+                    "-p",
+                    "--minimal",
+                    "--pretty=oneline",
+                    "--",
+                    *existing_files,
+                ],
+                cwd=base_path,
+            )
+            .strip()
+            .decode("utf-8")
+        )
+
+        if len(log) == 0:
+            print(f"Package {pkg.name} has no recent changes")
+            continue
+
+        updated_patches.append(log)
+
+    if len(updated_patches) == 0:
+        print("No patches to notify")
+        return
+
+    patches = "\n".join(updated_patches)
+
+    # Post to GitHub issues comment
+
+    body = dumps(
+        {
+            "body": f"```diff\n{patches}\n```",
+        }
+    )
+    requests.post(
+        f"https://api.github.com/repos/{os.environ['GITHUB_REPOSITORY']}/issues/{os.environ['ISSUE_NUMBER']}/comments",
+        headers={
+            "Authorization": f"token {os.environ['GITHUB_TOKEN']}",
+            "Content-Type": "application/json",
+        },
+        data=body,
+    )
+
+
+if __name__ == "__main__":
+    main()

.github/pull_request_samples/example_detail.md

@@ -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
+
+See also [this document.](https://tier4.github.io/scenario_simulator_v2-docs/)  
+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.

.github/pull_request_samples/example_simple.md

@@ -0,0 +1,5 @@
+# Description
+
+## Abstract
+
+Corrected "Vehicle" incorrectly spelled as "Vehicle".