Refactor agency/service_areas to express geofences.
#203
Conversation
|
@thekaveman Whoops, fixed that. |
| | `forbidden_events` | List | Required | List of events that is not permitted inside the area | | ||
| | `speed_cap` | Integer | Optional | Speed cap to be enable on device, in integers, for devices that support governors. Expressed in miles per hour | | ||
| | `preferred_pick_up` | Boolean | Optional | If true, means this service area is an incentive zone for user pick up, and should be expressed to users | | ||
| | `preferred_drop_off` | Boolean | Optional | If true, means this service area is an incentive zone for user drop offs, and should be expressed to users in app | | ||
|
|
There was a problem hiding this comment.
Does it make sense to refactor those last three columns speed_cap, preferred_pick_up, preferred_drop_off into an enum or sub-object? It feels like there might be more of these "attributes" in the future - for example a field that mirrors the allowed_events as a list of enum types.
I haven't thought about this too hard yet. Just throwing it out there.
There was a problem hiding this comment.
hmmm yeah. I think speed_cap is worth separating because type(int), but how does something like:
zones: [list_of_zone_types]
with zone_types == preferred_pick_up, preferred_drop_off, valet_station, any others?
There was a problem hiding this comment.
👍 I like zone_type. Also agreed that speed_cap is different enough.
One clarification on speed_cap - this is intended to be the new maximum speed the device is supposed to be able to go, within this zone - right? We should probably be crystal clear in the definition column there.
| | restricted | Areas where device pick-up/drop-off is not allowed | | ||
| | preferred_pick_up | Areas where users are encouraged to pick up devices | | ||
| | preferred_drop_off | Areas where users are encouraged to drop off devices | | ||
| | `service_start_date` | Unix Timestamp | Required | Datetime at which this service area enters into effect. For the base service_area, the time should be the start of permitted operations in the City. | |
There was a problem hiding this comment.
I thought the spec was standardizing on milliseconds everywhere, not seconds. I realize millis is overkill for this, but the spec should use the same notion of timestamp everywhere.
| | `service_area` | MultiPolygon | Required | Per [GeoJSON spec](https://tools.ietf.org/html/rfc7946), must follow the right-hand rule to designate interior. | | ||
| | `allowed_events` | List | Required | List of permitted events inside the area | | ||
| | `forbidden_events` | List | Required | List of events that is not permitted inside the area | | ||
| | `speed_cap` | Integer | Optional | Speed cap to be enable on device, in integers, for devices that support governors. Expressed in miles per hour | |
There was a problem hiding this comment.
Prefer meters/sec, with KPH as a second choice.
There was a problem hiding this comment.
good call. @karcass will change.
|
|
||
| The endpoint returns an a ordered list of GeoJSON MultiPolygons, per provider, shows which type of events are allowed in what areas. The list of the events in the same as defined in [provider/status_changes](https://github.com/CityOfLosAngeles/mobility-data-specification/tree/master/provider#status-changes). | ||
|
|
||
| Rules are given from least-specific to most-specific. For any given point, the most recent point-in-polygon operation should be considered valid. For example, if you have two overlapping Polygons, the polygons who appeared closest to the end of the list represents in the in affect rules, subject to time rounds. |
There was a problem hiding this comment.
Similar wording confusion. Perhaps:
Where geofences overlap in space and time, rules are enforced according to the linear order of the geofences as returned by the API. When two or more in-effect geofences overlap, the fence closest to the end of the list represents in the applicable set of rules.
| Gets the list of service areas available to the provider. | ||
| Gets the list of service areas available to the provider. A service area is a rules engine that provides a set of service areas that implements [geofences and incentive areas](https://docs.google.com/document/d/170W8rWGMmBDb7U-4OvNAQgk_Jusk6UKrsE-BP-Gk5VQ/edit) along with a base layer set of rules for operation of devices inside a jurisdiction running the Agency endpoint. | ||
|
|
||
| The endpoint returns an a ordered list of GeoJSON MultiPolygons, per provider, shows which type of events are allowed in what areas. The list of the events in the same as defined in [provider/status_changes](https://github.com/CityOfLosAngeles/mobility-data-specification/tree/master/provider#status-changes). |
There was a problem hiding this comment.
The wording is a little garbled here. How about
The endpoint returns an ordered list of GeoJSON MultiPolygons, per provider, detailing which type of vehicle events are allowed in those areas.
|
How does this relate to #197? The changes to the Agency API seem to directly conflict. |
| | `forbidden_events` | List | Required | List of events that is not permitted inside the area | | ||
| | `speed_cap` | Integer | Optional | Speed cap to be enable on device, in integers, for devices that support governors. Expressed in miles per hour | | ||
| | `preferred_pick_up` | Boolean | Optional | If true, means this service area is an incentive zone for user pick up, and should be expressed to users | | ||
| | `preferred_drop_off` | Boolean | Optional | If true, means this service area is an incentive zone for user drop offs, and should be expressed to users in app | |
There was a problem hiding this comment.
As said in #197 , it seems risky to tie together the service_areas definitions with requested behaviors.
In order to be extensible, it would be nice to decouple triggers (area, time, type of vehicle) and rules (speed cap, vehicles cap, ...).
This could still be done in the service_areas endpoint (also this name becomes less and less relevant) by using a list of rules:
{
"id": "...",
// Triggers
"start_date": 123,
"end_date": 123,
"area": {},
"vehicles": ["bike", "scooter"],
// Rules
"rules": [
{ type: "vehicles_cap", max: 100 },
{ type: "speed_cap", unit: "mph", max: 12 }, // A lot of providers also operate in countries that use kmh and not mph
]
|
moving this to 0.4.0, since we are still undecided on how best to implement this. |
|
@hunterowens should the service areas endpoint revert portion of this PR be retained for 0.3.0 still? |
| | preferred_drop_off | Areas where users are encouraged to drop off devices | | ||
| | `service_start_date` | Unix Timestamp | Required | Datetime at which this service area enters into effect. For the base service_area, the time should be the start of permitted operations in the City. | | ||
| | `service_end_date` | Unix Timestamp | Optional | Datetime at which the service area is no longer in effect. For example, a temporary restriction for an event can be placed in advance and set to end at a certain time. | | ||
| | `service_area` | MultiPolygon | Required | Per [GeoJSON spec](https://tools.ietf.org/html/rfc7946), must follow the right-hand rule to designate interior. | |
There was a problem hiding this comment.
"service_area" is already the endpoint name, maybe consider renaming this field ? (eg. "polygons")
|
@karcass you have time to revisit this with teh revised "policies" model you referenced? |
|
Yes, sorry for the delay. We have been prototyping a policy-interpreter to make sure the policy schema is sensible and easily converted into compliant/not-compliant outputs. Hoping to get you a draft Monday before releasing to the wider community. |
|
Recommend we close in favor of Policy proposal |
|
agreed. closing. |
This PR refactors agency implementation of the service areas to expresses geofences fully.
@black-tea wrote up a document a expressing what cities have done with a geofence so far, I recommend reading it as the design spec for this PR.
The implementation is fairly simple.
service_areasbecomes an ordered list ofservices_areathat are implement in a last to first order, withallowed_eventsandforbidden_eventsbased on the event type fromprovider/status_changes.Additionally, this PR removes that provider maintains a list of service areas, per comments in #110 since agencies, rather than providers, are the source of truth for service_areas.