Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(autoware_lanelet_map_validator): add dangling reference checker to non existing intersection_area #177

Merged
merged 7 commits into from
Dec 25, 2024

Conversation

soblin
Copy link
Contributor

@soblin soblin commented Dec 18, 2024

Description

This PR aims to add checker if any intersection lanelets (namely with turn_direction tag) has valid VALUE as the intersection_area key. If there are no intersection_area with corresponding VALUE id, it is dangling reference.

How was this PR tested?

Added a test map "intersection_area_with_dangling_reference.osm" with:

  • valid reference to existing intersection_area 10803 (lanelet 59)
  • dangling reference to non existing intersection_area 777 (lanelet 53)

Notes for reviewers

None.

Effects on system behavior

None.

Copy link

github-actions bot commented Dec 18, 2024

Thank you for contributing to the Autoware project!

🚧 If your pull request is in progress, switch it to draft mode.

Please ensure:

@soblin soblin force-pushed the feat/vm-03-08-dangling-reference-intersection-area branch from adae44c to 7e22a32 Compare December 18, 2024 04:34
…to non existing intersection_area

Signed-off-by: Mamoru Sobue <[email protected]>
@soblin soblin force-pushed the feat/vm-03-08-dangling-reference-intersection-area branch from 7e22a32 to 60ea165 Compare December 18, 2024 04:39
@soblin soblin marked this pull request as ready for review December 18, 2024 04:41
Copy link
Contributor

@TaikiYamada4 TaikiYamada4 left a comment

Choose a reason for hiding this comment

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

Thank you for the great work!
I wrote my opinions below so please check them out.

Aside those comments

  • Could you write a brief document to explain what this validator can do?
  • Could you add this to autoware_requirement_set.json like the following?
  {
    "id": "vm-03-01",
    "validators": [
      {
        "name": "mapping.intersection.intersection_area_dangling_reference"
      }
    ]
  },

Signed-off-by: Mamoru Sobue <[email protected]>
Signed-off-by: Mamoru Sobue <[email protected]>
Signed-off-by: Mamoru Sobue <[email protected]>
Copy link
Contributor

@TaikiYamada4 TaikiYamada4 left a comment

Choose a reason for hiding this comment

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

@soblin
Since the functionality is perfect, I'll approve this PR 👍
However I have some comments about the phrasing in the document so could you check them out? It also relates to the spelling check CI.


| Issue Code | Message | Severity | Primitive | Description | Approach |
| -------------------------------------------------- | --------------------------------------------------------------------------------------- | -------- | --------- | ---------------------------------------------------------------------------------- | -------------------------------------------------------------------- |
| Intersection.IntersectionAreaDanglingReference-001 | "Dangling reference to non-existing intersection area of ID \<LANELET ID\> is detected" | Error | Lanelet | Lookup to `intersection_area` from the reporeted lanelet will cause runtime error. | Go to the reported lanelet and delete "intersection_area" key entry. |
Copy link
Contributor

Choose a reason for hiding this comment

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

Lookup to intersection_area from the reporeted lanelet will cause runtime error.

"The reported lanelet will cause a runtime error when attempting to look up the non-existent intersection_area." might be more clear.


## Feature

This validator check whether each intersection lanelet(namely the lanelet with `turn_direction` property) has existing reference to `intersection_area` polygon. The countercase occurs when an existing intersection_area is deleted but its referrers are not updated.
Copy link
Contributor

Choose a reason for hiding this comment

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

  • check -> checks
  • I think the word countercase is not a popular word so how about circumstance or anomaly?

@soblin soblin enabled auto-merge (squash) December 25, 2024 02:52
@soblin soblin merged commit 1ccfdf1 into main Dec 25, 2024
17 of 18 checks passed
@soblin soblin deleted the feat/vm-03-08-dangling-reference-intersection-area branch December 25, 2024 03:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants