-
Notifications
You must be signed in to change notification settings - Fork 293
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
Aggregate available vehicle_types at a station #261
Conversation
Based on conversations in issue MobilityData#250 Modified station_status.json to aggregate the number of available vehicles per vehicle_type and removed the `bike_id`, `is_reserved`, `is_disabled`, `vehicle_type_id` and `current_range_meters` from the feed since they're available in free_bike_status.json. Also modified free_bike_status.json to specify the station_id where the vehicle is, in order to provide more details on the vehicle (such as 'current_range_meters'). So the free_bike_status.json could be used for free floating vehicles and docked vehicles as well to provide more information on each of the vehicles.
The proposal sounds logical but includes some fundamental changes to the semantic. So far the endpoint free_bike_status.json referred to free floating bikes. Bikes that are not docked or locked at a station. So the free in free_bike_status had the meaning of free floating rather then available to rent. But with this proposal the free now means available to rent. So I think this is a breaking change and should be labeled as this. Primarily because available docked bikes will no longer show up at the endpoint station_status.json but rather at free_bile_status.json. And following this argumentation it would make sense to rename free_bike_status.json into bike_status.json. Bike_status.json would thus become a counterpart to station_status.json. Although I can follow the basic idea of this PR I don't see a significant advantage for providing or consuming apps of this PR right away? What would be the most important benefits of this breaking PR? |
@matt-wirtz thanks for you comment. The goal of this PR is to reduce the size of the The number of available docked bikes would still be in @davidlewis-ito, @kanagy, @mplsmitch, since involved in the original issue leading to this PR, just thought you would be interested to see the proposed changes. |
Fixed a typo (missing quote)
@matt-wirtz The intent of |
@sfaubert1 Thanks for pointing again to issue #250. I kind of overlooked the reference and directly read through the proposed changes. So by removing the @mplsmitch Maybe I'm overlooking something but I still don't see how this PR should be non-breaking. Let's say there is a consumer app who uses the information of the |
gbfs.md
Outdated
 \- `is_disabled` <br/>*(added in v2.1-RC)* | Yes | Boolean | Is the vehicle currently disabled (broken) | ||
 \- `vehicle_type_id` <br/>*(added in v2.1-RC)* | Yes | ID | The vehicle_type_id of this vehicle as described in [vehicle_types.json](#vehicle_typesjson). | ||
 \- `current_range_meters` <br/>*(added in v2.1-RC)* | Conditionally Required | Non-negative float | If the corresponding vehicle_type definition for this vehicle has a motor, then this field is required. This value represents the furthest distance in meters that the vehicle can travel without recharging or refueling with the vehicle's current charge or fuel. | ||
\- `vehicle_types_available` <br/>*(added in v2.1-RC)* | Conditionally Required | Object | This field is required if the [vehicle_types.json](#vehicle_typesjson) file has been defined. This field is an object where each key is a `vehicle_type_id` as described in [vehicle_types.json](#vehicle_typesjson) and the value is a number representing the total number of available vehicle at this station for the specified vehicle type. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
vehicles should be plural: available vehicles
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
now changed, thanks for letting me know
gbfs.md
Outdated
@@ -450,15 +431,15 @@ Example: | |||
|
|||
### free_bike_status.json | |||
|
|||
Describes vehicles that are not at a station and are not currently in the middle of an active ride. | |||
Describes all vehicles that are not at a station and are not currently in the middle of an active ride. From v2.1-RC, also provides more information about vehicles that are at a station if the [vehicle_types.json](#vehicle_typesjson) file has been defined. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would like to clarify this definition and add line about vehicles in active rental from Best Practices. Suggesting this:
"Describes all vehicles that are not currently in active rental. Required of free floating (dockless) vehicles. Conditionally required of station based (docked) vehicles when vehicle_types.json has been defined. Vehicles that are part of an active rental must not appear in this feed."
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This definition should also be updated under "Files" above on line 71.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes that's more clear, thanks! Updated at both places.
Need to remove the reference to station_status.json in the definition of vehicle_types.json under "Files" on line 68 |
- updated free_bike_status.json description (2 places) - corrected a typo ("available vehicles")
About previous comment:
Please correct me if I'm wrong, but I think the reference to |
@sfaubert1 You are correct - the reference to |
I hereby call a vote on this proposal. Voting will be open for 7 full days, until 11:59PM UTC on September 23rd. Please vote for or against the proposal, and include the organization for which you are voting in your comment. Please note if you can commit to implementing the proposal. |
+1 from PBSC :) and also committing to implement |
Nits:
Question: Why is docked free_bike_status conditioned on vehicle_types.json being defined? I think free_bikes could describe the vehicles at a station even when the system doesn't define different vehicle_types. Otherwise, in principle this looks good to me (Google Maps), but we don't plan to implement this soon. In particular I think this is a good direction for supporting hybrid systems where bikes can be both dockless and docked. |
+1 from itoworld We will be consuming and producing |
Updated based on comments from @kanagy Fixed typo in free_bike_status description, fixed indentation in examples, matched style of vehicle_docks_available in vehicle_types_available.
Thanks @mplsmitch for the updates! For the description of For us the count of available bike will always be for one type of vehicle only; in the event of a case where multiple vehicle types would have the same count, the meaning could be confusing, for example:
In the example above, does it mean there's 2 vehicles of type "abc123" and 2 of type "xyz789" available as well or something else? Since
If we have to go with an array of objects, maybe consider a single string instead of an array for
The advantage of this last example would be if we need to include more properties to the |
@sfaubert1 Thanks for catching that - the intention was to make it a single string like in your 3rd example as was discussed in #250 . I'll make the change. |
vehicle_type_id becomes string
+1 from Stae |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Following up on previous comments and also noticed few typos.
Fix typos Update definition of free_bike_status.json
+1 from SkedGo We will be consuming |
+1 from Transit |
+1 from Google Maps |
The vote on this is now closed, and it passes! Votes in favor: No votes against. We'll be merging, creating v2.2RC (likely after the vote on pricing!), and then we'll follow up with those of you who said you'd be implementing. Thanks for all of your help making sure the new dockless support is implementable! |
minor edits, updates stations_status to align with changes in #261
Based on conversations in issue #250
Modified station_status.json to aggregate the number of available vehicles per vehicle_type and removed the
bike_id
,is_reserved
,is_disabled
,vehicle_type_id
andcurrent_range_meters
from the feed since they're available in free_bike_status.json.Also modified free_bike_status.json to specify the station_id where the vehicle is, in order to provide more details on the vehicle (such as 'current_range_meters'). So the free_bike_status.json could be used for free floating vehicles and docked vehicles as well to provide more information on each of the vehicles.