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

Location verify method #199

Merged
merged 5 commits into from
Nov 18, 2021
Merged

Location verify method #199

merged 5 commits into from
Nov 18, 2021

Conversation

sknick-iastate
Copy link
Collaborator

@sknick-iastate sknick-iastate commented Aug 26, 2021

This pull request addresses the issues identified in #129. Initial discussion focused on providing a list of enumerations for the location verify method but after further discussion it was determined that verified location information should be made using a GPS equipped device (i.e. not verified through cameras, site inspection, etc). It was determined that this would be better achieved by updating the SpatialVerification enumerations to clarify that a GPS equipped device is required for verification and depreciating the location_verify_method.

As further justification the location_verify_method is at the data source level and wouldn't allow for different verification types for individual road events. If this is a desired then another attribute should be added at the event level.

So in summary the changes are:

Deprecating location verify method from schema
Deprecate location verify method and add reference to spatial verification which defines using a GPS equipped device for verification.
Updated description of the spatial verification enumerations.
@j-d-b j-d-b self-requested a review August 26, 2021 12:33
Copy link
Collaborator

@j-d-b j-d-b left a comment

Choose a reason for hiding this comment

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

Thank you @sknick-iastate.

@j-d-b j-d-b linked an issue Aug 26, 2021 that may be closed by this pull request
@j-d-b j-d-b added the v4.0 label Aug 26, 2021
@j-d-b
Copy link
Collaborator

j-d-b commented Aug 27, 2021

Alternatively, here's a different implementation of the same change that simplifies the specification without losing functionality (which is my goal—make WZDx easy to use and understand while still being powerful):

  • Deprecate location_verify_method (same as in this PR)
  • Remove the SpatialVerification enumerated type
  • Remove the RoadEvent's beginning_accuracy and ending_accuracy (which use the SpatialVerification enumerated type)
  • Add new properties is_begin_location_verified and is_end_location_verified to the RoadEvent which have boolean values indicating whether or not the begin/end is verified (if it's not verified, it's estimated, so we don't need an enumerated type if there are only two options).

In addition to being simpler and requiring less documentation (the new WZDx user doesn't have to reference an enumerated type and see it only has two values), I think the is_begin_location_verified is more clear what the property is describing than beginning_accuracy, which doesn't indicate by name that it's for the location (we could name it is_start_point_verified to be more clear, because that's what the property is describing).

Also, it makes it more clear what the buisness rule for the use of is_begin_location_verified could be:

is_begin_location_verified and is_end_location_verified must only be true when the location is based on actual reported data from a GPS equipped device showing the measured location of the work zone

The only downside of this approach is if we wanted to add more options than just estimated and verified for describing the accuracy of the start/end, though there has not been any desire for that. I think if there is not desire and members can't think of other reasonable options for start/end accuracy, we should consider the approach proposed here with the boolean properties.

@j-d-b
Copy link
Collaborator

j-d-b commented Sep 16, 2021

@sknick-iastate let me know what you think of my above approach.

@sknick-iastate
Copy link
Collaborator Author

@j-d-b sorry for the delayed response. I like your approach for changing the verification to a boolean. I'd like to get feedback at our next specification update meeting then can make the change.

@j-d-b j-d-b self-requested a review September 21, 2021 13:22
Copy link
Collaborator

@j-d-b j-d-b left a comment

Choose a reason for hiding this comment

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

@j-d-b j-d-b added the Refactor This item related to refactoring the specification. label Oct 15, 2021
Copy link
Collaborator

@j-d-b j-d-b left a comment

Choose a reason for hiding this comment

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

I moved my alternate approach to a new issue, #213. The implementation here looks good for v4.0.

@sknick-iastate sknick-iastate merged commit f2a54ed into usdot-jpo-ode:v4.0-release Nov 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Refactor This item related to refactoring the specification.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve machine-usability of location_verify_method
2 participants