open-simulation-interface icon indicating copy to clipboard operation
open-simulation-interface copied to clipboard

Draft for discussion: Add DAY to TrafficSign-Value-Unit and change DAY to WEEKDAY

Open FlorianMueller87 opened this issue 3 years ago • 1 comments

Reference to a related issue in the repository

Issue: #635

Add a description

ASAM OSI and ISO 23150 or AUTOSAR ADI have a common history. Unfortunately, the inner structure, the naming and the definitions of the standards are differentiated from each other. This makes the work of developers unnecessary complicated for mostly no technical reasons. All sides should strive to reduce inequality.

osi_trafficsign.proto message TrafficSignValue { enum Unit } need an entry which describes the day of the month.

Furthermore, the type need a entry kHour which describe the hour of the day.

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

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

The checks will follow.

Additional context

ISO23150:2021A.2.67 Sign value unit:

@ThomasNaderBMW @jdsika @schmidtlorenz

FlorianMueller87 avatar Aug 09 '22 14:08 FlorianMueller87

Comments:

  • Renaming a value will break compatibility and is not reasonable if the semantic meaning persists. Reanming it will be done in v4.0.0 in the process of reworking the whole enum
  • In general we have to ask what needs to be achieved: Do we want to add e.g. a full or partial date to a supplementary sign or how do you intend to use this unit? Are we missing then things like "MONTH_OF_YEAR" or "YEAR"?

Current situation:

  • we have "DAY" meaning "day of the week" 0=Monday, and 1=Tuesday. Note: ISO starts with 0=Sunday potentially violating the ISO8601 but starting with 1=Monday. @carsten-kuebler can you have look at this here, please?

What could be done for v3.6.0:

  • add "DAY_OF_MONTH" starting with 1

What could be done for 4.0.0:

  • rename "DAY" to "DAY_OF_WEEK"
  • Let all values start with the value 1 instead of 0 as this is normally done for dates

jdsika avatar Oct 17 '22 07:10 jdsika

Comments:

  • Duration (missing) vs time of day
    • UNIT_DURATION_MINUTE
    • UNIT_DURATION_HOUR
    • UNIT_DURATION_DAY
  • Use singular and not plural: v4.0
    • UNIT_MINUTES to UNIT_MINUTE (comment is wrong -> BUG)
    • UNIT_FEET to UNIT_FOOT
  • Add comment referencing ISO8601 and add e.g. that minute and hour start with 0

jdsika avatar Nov 23 '22 10:11 jdsika

Proposal:

  • PR für v3.6
  • Issue für v4.0

jdsika avatar Nov 23 '22 10:11 jdsika

@pmai please review and set READY_FOR_CCB afterwards. Sorry for messing up the DCO.

jdsika avatar Nov 23 '22 10:11 jdsika