Skip to content

Conversation

@thomassedlmayer
Copy link
Contributor

@thomassedlmayer thomassedlmayer force-pushed the adopt-role-harmonization branch 3 times, most recently from d20c45a to fbb8506 Compare November 27, 2025 15:38
Signed-off-by: Thomas Sedlmayer <tsedlmayer@pmsfit.de>
Signed-off-by: Thomas Sedlmayer <tsedlmayer@pmsfit.de>
@thomassedlmayer thomassedlmayer force-pushed the adopt-role-harmonization branch from fbb8506 to c90791b Compare December 2, 2025 12:11
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not too familiar with the OSI structure, but in 3.7 I found the roles under "Vehicle Classification", see screenshot below. Is this something we need to change as we address all kinds of moving objects here?

Image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, we should also think about how to include it generically for all moving objects. But I though, we might want to harmonize the existing enum first.

We could just reference the existing enum definition in MovingObjectClassification or - for a cleaner result - move the role enum definition out of the VehicleClassification to be a generic enum definition (osi3::Role instead of osi3::MovingObject::VehicleClassification::Role) and then reference it in two places. I believe this should work in a backwards compatible way because we wouldn't change the data representation by moving enum definitions.

Copy link
Contributor

Choose a reason for hiding this comment

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

I can't judge the implications on backwards compatibility of your suggestion for the cleaner result, but it does seem like the better / less confusing approach.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants