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

Allow restriction value and units for road event level restrictions #155

Closed
j-d-b opened this issue Jan 27, 2021 · 7 comments · Fixed by #198
Closed

Allow restriction value and units for road event level restrictions #155

j-d-b opened this issue Jan 27, 2021 · 7 comments · Fixed by #198
Assignees

Comments

@j-d-b
Copy link
Collaborator

j-d-b commented Jan 27, 2021

This issue relates to #154. I recommend checking out that one as well.

Background

Currently, restrictions (see RoadRestriction enumerated type) can be provided at the lane level or at the road event level. At the lane level, restrictions are provided by an array of LaneRestriction objects, which consistent of a type, value, and units. At the road event level, restrictions are provided by an array of restriction types, the text string from the RoadRestriction enum only.

Allowing restriction values at the road event level

In recent discussions regarding using WZDx, either modified or unmodified, for representing bridge clearances, it was noted that it may be helpful to allow providing a value and unit alongside a restriction given at the road event level. For the bridge clearance example, one could then provide a road event restriction for reduced-height and specify the height value rather than having to provide the restriction on every lane. This is both a convenience but also a necessity if the producer does not have lane-level information to be able to provide the lanes array on the RoadEvent, but wants to give a restriction with a value.

A downside of allowing restriction values at the road event level is that it makes the specification more complex by requiring a new object (or renaming LaneRestriction and its properties) and having another array of objects. The approach in #154 (removing road event level restrictions) is a possible solution to adding complexity.

On a related note, a few restrictions apply best to only the lane-level, such as hov-2 and reduced-width. Also, reading through the list of restrictions in the RoadRestriction enum, I can imagine some values may be best applied only at the road event level. Perhaps lane restrictions should use a different enumerated type from road event level restrictions. That can also be discussed here.

Summary

In summary, the hope is to simplify the specification, both making it easier to implement and understand, ideally without losing functionality. Being able to specify restrictions in two places with different functionality (i.e. values only allowed in one place) seems confusing.

The options up in the air are:
1. Require lanes and remove restrictions from the RoadEvent object (see #154)
2. Make the LaneRestriction generic (e.g. Restriction object) and change the restrictions property on the RoadEvent to be an array of the new Restriction and the restrictions property on the Lane to be an array of the new Restriction as well.
3. Do nothing

Also we should:

  1. Examine if all RoadRestrictions apply to both lanes and road events, or if this should be split into two enumerated types
@DeraldDudley
Copy link
Collaborator

DeraldDudley commented Jun 18, 2021

My preference

  1. Rename the LaneRestrictions Object to Restrictions.
  2. Make the Restrictions object inheritable by the RoadEvent object and the Lane object (Restrictions can be a subclass of the RoadEvent and/or Lane classes)
  3. Rename the LaneRestrictionUnit enumeration to UnitType (Added benefit if units are needed elsewhere in the model)
  4. Deprecate / Remove the restrictions property from the RoadEvent Class

wzdx_object_diagram_low_clearance_extension

@j-d-b
Copy link
Collaborator Author

j-d-b commented Jun 24, 2021

I agree with @DeraldDudley's proposed approach of making the LaneRestriction object generic and using it for the restrictions property on the RoadEvent object in place of just the restriction text.

While we are doing this refactoring, as the fall 2021 release will already be a major release with breaking changes, I think (and it looks like @DeraldDudley thought this as well and included it in the diagram) it is cleaner for the RoadRestriction enumerated type to be renamed RestrictionType as it better describes what the enumerated type is—just the type of restriction.

Thus the changes are:

  1. Make the LaneRestriction object generic:
    • Rename the object to Restriction.
    • Rename the properties of the object to type, value, and units. The prefixes are not needed and object name prefixes are not used on any other objects, so this is a change for consistency as well.
  2. Use the new generic Restriction object for the restrictions property on both the Lane and RoadEvent objects.
  3. Rename the LaneRestrictionUnit enumerated type to be generic, either RestrictionUnit (just drop the "Lane") or an even more general UnitType as proposed by @DeraldDudley.

As for the name of the enumerated type containing units, my preference is RestrictionUnit as the Restriction object is the only place this enumerated type is used—it can be made more general in the future if needed.

@mark-mockett
Copy link
Collaborator

@DeraldDudley I (and the SU co-chairs) would recommend renaming LaneRestrictionUnit to just Units rather than UnitType (which seems like it should describe a type of unit) since it could be relevant in other parts of the specification than just restrictions.

@j-d-b
Copy link
Collaborator Author

j-d-b commented Jul 14, 2021

@mark-mockett I agree that UnitType is misleading as these are not types of units, they are specific units.

I also want to add for suggestions for the unit enumerated type name, from reading the Wikipedia page:

  • MeasurementUnit
  • UnitOfMeasurement
  • DataUnit (added after @DeraldDudley's comment below)

Note that all enumerated types in WZDx are named in singular form, thus @mark-mockett's suggestion of Units would instead be Unit—both are a bit vague.

@DeraldDudley
Copy link
Collaborator

ISO 14825 uses "Data Unit" which include these types: Degree=DEG, Meters=MTR, Kilometers=KMR, Inch=INC, Feet=FET, Mile=MIL, Kilogram=KGR, US Pound=PND, Kilo Pound=KIP, Metric Ton=TON, Minute of Time=MIN, Kilometer Per Hour=KPH, Miles Per Hour=MPH

Are there other specifications we should consider?

@natedeshmukhtowery
Copy link
Collaborator

Looping back to a comment that @daylesworth made on issue #174, SAE's J2735 may be a source to consider; we should probably also aim for alignment with NTCIP standards if possible

@j-d-b j-d-b linked a pull request Aug 24, 2021 that will close this issue
@j-d-b
Copy link
Collaborator Author

j-d-b commented Dec 16, 2021

Implemented in #198.

@j-d-b j-d-b closed this as completed Dec 16, 2021
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 a pull request may close this issue.

4 participants