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.

Sign up for GitHub

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

Jump to bottom

Add new message for sub-bounding boxes #732

Merged
merged 14 commits into from
Mar 8, 2024
Merged

Add new message for sub-bounding boxes #732

merged 14 commits into from
Mar 8, 2024

Conversation

pmai
Copy link
Contributor

@pmai pmai commented Jun 16, 2023

This PR is a continuation of #695 (and #685) with fixed history. It proposes the addition of bounding box sub-sections to BaseMoving and BaseStationary objects, to help sub-divide and classify different parts of the overall object structure.

@pmai pmai added this to the V3.6.0 milestone Jun 16, 2023
@pmai pmai self-assigned this Jun 16, 2023
@pmai pmai force-pushed the feature/sub-bounding-boxes branch 3 times, most recently from 4206564 to 58ecd44 Compare June 16, 2023 09:32
@thempen
Copy link
Contributor

thempen commented Jan 29, 2024 •
edited
Loading

It was unlcear which pivot point the Position definition represents in relation to the dimension and rotation. Added a comment, that it is the center of the BoundingBox.

For Type "TYPE_OTHER", there was no possibility to express the object more detailed. I introduced a string type to describe it.

This other_object_type offers the possibility to reduce the Type enum and split it up an an additional PR to update it later on. This type should not be in conflict to other current approaches harmonizing object type definition. Through the other_object_type filed, the overall SubBoundingBox approach is already usable.

@pmai Why did my DCO fail again? :.(

@thempen thempen added the ReadyForCCBReview Indicates that this MR is ready for a final review and merge by the CCB. label Jan 29, 2024
@pmai
Copy link
Contributor Author

pmai commented Jan 29, 2024

CCB 2024-01-29: Type enum should be reviewed (e.g. by @PhRosenberger, @thomassedlmayer, @pmai) esp. w.r.t. sensor modeling needs and a minimum list should be proposed. If no such list emerges, then having no enums, but also dropping the string fallback (as e.g. model_reference can already be used as a last effort fallback) would be the preferred way forward. Will be re-reviewed at next CCB.

@thempen thempen force-pushed the feature/sub-bounding-boxes branch from 24dfdbb to 12ef6dc Compare February 12, 2024 07:47
@thempen thempen removed the ReadyForCCBReview Indicates that this MR is ready for a final review and merge by the CCB. label Feb 12, 2024
thomassedlmayer
thomassedlmayer reviewed Feb 15, 2024
View reviewed changes
osi_common.proto Outdated Show resolved Hide resolved
@thempen thempen added the ReadyForCCBReview Indicates that this MR is ready for a final review and merge by the CCB. label Feb 26, 2024
@pmai
Copy link
Contributor Author

pmai commented Feb 26, 2024

CCB 2024-02-26: Small addition of examples for TYPE_DOOR in the context of vehicles to be added by @pmai, otherwise is ready for merge as-is.

PhRosenberger
PhRosenberger approved these changes Feb 26, 2024
View reviewed changes
Copy link
Contributor

@PhRosenberger PhRosenberger left a comment

Choose a reason for hiding this comment

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

Besides the comment on the doors, I approve this PR.

osi_common.proto Show resolved Hide resolved
@pmai pmai force-pushed the feature/sub-bounding-boxes branch from 9230083 to 7d8e37b Compare February 26, 2024 12:38
@pmai pmai 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 MR is ready for a final review and merge by the CCB. labels Feb 26, 2024
ndunningbmw and others added 9 commits March 8, 2024 16:18
@ndunningbmw @pmai
Add BoundingBox section
2a4902a 
Signed-off-by: Nicholas Dunning <Nicholas.Dunning@bmw.de>
@ndunningbmw @pmai
Add clarity for limbs
e32e566 
Signed-off-by: Nicholas Dunning <Nicholas.Dunning@bmw.de>
@thempen @pmai
Enhance description for bounding_box_section
7b24c44 
Add information regarding the handling of side mirrors and the
definition of the bounding box.

Signed-off-by: Pierre R. Mai <pmai@pmsf.de>
@pmai
Rewording and typo fixes as per discussions
c71f9c3 
Signed-off-by: Pierre R. Mai <pmai@pmsf.de>
@thempen @pmai
Updated the BoundingBox message.
1ab8432 
- Clarified the position definition.
- Added a "other_object_type" sting to define object types, which are not part of the Type definition enum.
- Reduced the Type field, because of compatibility reason. There is a general approach to harmonize object type definitions. So the definition should not get in conflict to that approach.

Signed-off-by: Thomas Hempen <thomas.hempen@carissma.eu>
@thempen @pmai
Reworked the Type fields.
6e09399 
Signed-off-by: Thomas Hempen <thomas.hempen@carissma.eu>
@thempen @pmai
Fixed numbering.
2b5149b 
Signed-off-by: Thomas Hempen <thomas.hempen@carissma.eu>
@thempen @pmai
Changed formatting.
79ee488 
Signed-off-by: Thomas Hempen <thomas.hempen@carissma.eu>
@thempen @pmai
Typo correction.
e8a5ed0 
Signed-off-by: Thomas Hempen <thomas.hempen@carissma.eu>
thempen and others added 5 commits March 8, 2024 16:18
@thempen @pmai
Typo corrected.
dab24b1 
Signed-off-by: Thomas Hempen <thomas.hempen@carissma.eu>
@thomassedlmayer @pmai
Reorder sub bounding box enums
c4338e7 
Signed-off-by: Thomas Sedlmayer <tsedlmayer@pmsfit.de>
@thempen @pmai
Updated comments
65b814d 
Signed-off-by: Thomas Hempen <thomas.hempen@carissma.eu>
@thempen @pmai
Updated format.
5cfed34 
Removed invalid tabs.

Signed-off-by: Thomas Hempen <thomas.hempen@carissma.eu>
@pmai
Add explicit list of doors for vehicles
7e81917 
Signed-off-by: Pierre R. Mai <pmai@pmsf.de>
@pmai pmai force-pushed the feature/sub-bounding-boxes branch from 7d8e37b to 7e81917 Compare March 8, 2024 15:18
@pmai pmai merged commit 6807643 into master Mar 8, 2024
8 checks passed
@pmai pmai deleted the feature/sub-bounding-boxes branch March 8, 2024 15:30
@pmai pmai modified the milestones: V3.6.0, V3.7.0 Apr 4, 2024
jdsika
jdsika reviewed Apr 24, 2024
View reviewed changes
Copy link
Contributor

@jdsika jdsika left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Reviewed with critical comment v3.7.0
@PhRosenberger

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@thomassedlmayer thomassedlmayer thomassedlmayer left review comments

@ClemensLinnhoff ClemensLinnhoff ClemensLinnhoff left review comments

@jdsika jdsika jdsika left review comments

@PhRosenberger PhRosenberger PhRosenberger approved these changes

@thempen thempen Awaiting requested review from thempen

Assignees

@pmai pmai

Labels
Other Models ReadyToMerge This PR has been approved to merge and will be merged by a member of the CCB.
Projects
None yet
Milestone
V3.7.0
Development

Successfully merging this pull request may close these issues.
7 participants
@pmai @thempen @jdsika @PhRosenberger @ClemensLinnhoff @thomassedlmayer @ndunningbmw