Skip to content

Add Role to VehicleClassification - #613

Merged
pmai merged 6 commits into
OpenSimulationInterface:masterfrom
adrianschultz:OSI-584-VehicleRole
May 30, 2022
Merged

Add Role to VehicleClassification#613
pmai merged 6 commits into
OpenSimulationInterface:masterfrom
adrianschultz:OSI-584-VehicleRole

Conversation

@adrianschultz

@adrianschultzadrianschultz commented Jan 26, 2022•
edited
Loading

Copy link
Copy Markdown
Contributor

Reference to a related issue in the repository

Issue

Add a description

Added Role to VehicleClassification in alignment with OSC 1.2

Take this checklist as orientation for yourself, if this PR is ready for the Change Control Board:

  • My suggestion follows the style and contributors guidelines.
  • I have taken care about the documentation.
  • I have done the DCO signoff.
  • My changes generate no errors when passing CI tests.
  • I have successfully implemented and tested my fix/feature locally.
  • Appropriate reviewer(s) are assigned.

If you can’t check all of them, please explain why.
If all boxes are checked or commented and you have achieved at least one positive review, you can assign the label ReadyForCCBReview!

@adrianschultzadrianschultz added the Harmonisation The issue/PR needs harmonisation with another ASAM (or other organisation) standard label Jan 26, 2022
This was referenced Jan 26, 2022
Closed
Closed
@adrianschultzadrianschultz self-assigned this Jan 27, 2022
@adrianschultzadrianschultz linked an issue Jan 27, 2022 that may be closed by this pull request
Add subType to VehicleClassification in alignment with OpenSCENARIO 1.2 #584
Closed
@thempenthempen added the ReadyForCCBReview Indicates that this PR is ready for a final review and merge by the CCB. label Feb 15, 2022
kmeids
kmeids reviewed Feb 16, 2022
View reviewed changes
Comment threadosi_object.proto Outdated
@kmeids

kmeids commented Feb 16, 2022

Copy link
Copy Markdown

Output CCB 16.02.22:

  1. Please take a look on the provided comments.
  2. Which value is supposed to represent "normal" traffic participants? This should not be ROLE_OTHER or ROLE_UNKNOWN.

@kmeidskmeids removed the ReadyForCCBReview Indicates that this PR is ready for a final review and merge by the CCB. label Feb 16, 2022
@HendrikAmelunxen

HendrikAmelunxen commented Apr 8, 2022

Copy link
Copy Markdown
Contributor

Output of harmonization meeting (08.04.22): The roles need more clarification, e.g. what role gets a fire truck without emergency lights? Civil or fire? Same question for police, road assistance, and ambulance.
Is an additional role for garbage trucks needed, since they have a specific driving behaviour when they are in action?

@adrianschultz

adrianschultz commented May 20, 2022

Copy link
Copy Markdown
ContributorAuthor

Output of harmonization meeting (08.04.22): The roles need more clarification, e.g. what role gets a fire truck without emergency lights? Civil or fire? Same question for police, road assistance, and ambulance. Is an additional role for garbage trucks needed, since they have a specific driving behaviour when they are in action?

Wether those are active or not can be looked up in VehicleClassification / LightState. So, I would suggest to set the role in your example to ROLE_FIRE. I can add a clarification in the enum description to explain this.

Adding new roles for garbage related vehicles (truck, sweeper etc.) and road construction related vehicles would be reasonable in my opinion.
e.g. :
// The vehicle role is garbage collection, e.g. a garbage truck or sweeper.
//
ROLE_GARBAGE_COLLECTION = 9;

// The vehicle role is road construction, e.g. a excavator or tipper truck.
//
ROLE_ROAD_CONSTRUCTION = 10;

stefancyliax
stefancyliax reviewed May 23, 2022
View reviewed changes
Comment threadosi_object.proto Outdated
@stefancyliax

stefancyliax commented May 23, 2022

Copy link
Copy Markdown
Contributor

OSI CCB 23.05.2022:
Topic will be finally discussed on 30.05..
Decision about merging then. If it is ready for merging, it will be included in OSI 3.5

@stefancyliaxstefancyliax added this to the V3.5.0 milestone May 23, 2022
@HendrikAmelunxenHendrikAmelunxen added the ReadyForCCBReview Indicates that this PR is ready for a final review and merge by the CCB. label May 25, 2022
@stefancyliax

stefancyliax commented May 30, 2022•
edited
Loading

Copy link
Copy Markdown
Contributor

OSI CCB:

  • There happened something with rebasing, that has to be cleaned up. @pmai to look at this.
  • @HendrikAmelunxen What was the result of the meeting on 25.05.2022? Is this ready to be merged?

@pmai
pmaiforce-pushed the OSI-584-VehicleRole branch from e6111e5 to 09d7599CompareMay 30, 2022 06:21
stefancyliax
stefancyliax reviewed May 30, 2022
View reviewed changes
Comment threadosi_object.proto
@stefancyliaxstefancyliax added ReadyToMerge This PR has been approved to merge and will be merged by a member of the CCB. and removed ReadyForCCBReview Indicates that this PR is ready for a final review and merge by the CCB. labels May 30, 2022
@pmai
pmaiforce-pushed the OSI-584-VehicleRole branch 2 times, most recently from 64c13a7 to 37e4dd8CompareMay 30, 2022 07:04
adrianschultzand others added 4 commits May 30, 2022 12:35
@adrianschultz@pmai
Add Role to VehicleClassification (Issue 584)
cf58c60
Signed-off-by: Adrian Vernickel <adrian.vernickel@hexagon.com>
Signed-off-by: Pierre R. Mai <pmai@pmsf.de>
@adrianschultz@pmai
Clarification of ROLE_CIVIL
44612c1
ROLE_CIVIL is supposed to represent "normal" traffic participants
Signed-off-by: Adrian Vernickel <adrian.vernickel@hexagon.com>
Signed-off-by: Pierre R. Mai <pmai@pmsf.de>
@adrianschultz@pmai
Add new Roles and extend enum description
f17a788
Signed-off-by: Adrian Vernickel <adrian.vernickel@hexagon.com>
Signed-off-by: Pierre R. Mai <pmai@pmsf.de>
@adrianschultz@pmai
change order of Ambulance and Civil
b1d1dd2
The standard case should be the first one.
Signed-off-by: Adrian Vernickel <adrian.vernickel@hexagon.com>
Signed-off-by: Pierre R. Mai <pmai@pmsf.de>
adrianschultzand others added 2 commits May 30, 2022 12:35
@adrianschultz@pmai
Remove spelling mistake and move military to back
b4616db
Signed-off-by: Adrian Vernickel <adrian.vernickel@hexagon.com>
Signed-off-by: Pierre R. Mai <pmai@pmsf.de>
@pmaiStefan Cyliax
Clarify purpose of VehicleClassification role
5eded6b
Co-authored-by: Stefan Cyliax <stefan.cyliax@asam.net>
Signed-off-by: Pierre R. Mai <pmai@pmsf.de>
@pmai
pmaiforce-pushed the OSI-584-VehicleRole branch from 37e4dd8 to 5eded6bCompareMay 30, 2022 10:35
@pmai
pmai merged commit 1024757 into OpenSimulationInterface:masterMay 30, 2022
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@kmeidskmeidskmeids left review comments
@pmaipmaipmai approved these changes
@HendrikAmelunxenHendrikAmelunxenAwaiting requested review from HendrikAmelunxen
+1 more reviewer
@stefancyliaxstefancyliaxstefancyliax left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

@adrianschultzadrianschultz

Labels

HarmonisationThe issue/PR needs harmonisation with another ASAM (or other organisation) standardReadyToMergeThis PR has been approved to merge and will be merged by a member of the CCB.

Projects

None yet

Milestone

V3.5.0

Development

Successfully merging this pull request may close these issues.

Add subType to VehicleClassification in alignment with OpenSCENARIO 1.2

6 participants

@adrianschultz@kmeids@HendrikAmelunxen@stefancyliax@pmai@thempen