smartdevicelink / sdl_evolution

Tracking and proposing changes to SDL's public APIs.
https://smartdevicelink.github.io/sdl_evolution/
BSD 3-Clause "New" or "Revised" License
33 stars 122 forks source link

SDL 0167 - App Services - Navigation Service #591

Closed theresalech closed 5 years ago

theresalech commented 6 years ago

Hello SDL community,

The review of the Navigation Service type of the larger proposal “SDL 0167 - App Services” begins now and runs through August 22, 2018.

The original review of “SDL 0167 - App Services” occurred May 2 - May 8, 2018 with the vote of acceptance pending each service type review on the feature happening July 10, 2018. The app service type of the proposal in review, Navigation, is available here:

https://github.com/smartdevicelink/sdl_evolution/blob/master/proposals/0167-app-services.md#navigation

The original review can be found here:

https://github.com/smartdevicelink/sdl_evolution/issues/485

Reviews are an important part of the SDL evolution process. All reviews should be sent to the associated Github issue at:

https://github.com/smartdevicelink/sdl_evolution/issues/591

What goes into a review?

The goal of the review process is to improve the proposal under review through constructive criticism and, eventually, determine the direction of SDL. When writing your review, here are some questions you might want to answer in your review:

More information about the SDL evolution process is available at

https://github.com/smartdevicelink/sdl_evolution/blob/master/process.md

Thank you, Theresa Lech

Program Manager - Livio theresa@livio.io

joeljfischer commented 6 years ago

For reference, the current service is:

<struct name="NavigationServiceManifest">
    <param name="acceptsWayPoints" type="Boolean" mandatory="false" />
</struct>

<struct name="NavigationServiceData">
    <description> This data is related to what a navigation service would provide</description>

    <param name="waypoints" type="LocationDetails" array="true" minsize="2" mandatory="false">
        <description> This array should be ordered with the first object being the start location of the current trip and the last object the final destination. </description>
    </param>
    <param name="eta" type="DateTime" mandatory="false"/>
</struct>
  1. Is the eta to the next waypoint, or to the final destination?
  2. Should there be any maneuver data involved? e.g. An array of all maneuvers to reach the next waypoint or final destination?
robinmk commented 6 years ago

This array should be ordered with the first object being the start location of the current trip and the last object the final destination.

Q. Is there a need for the start location to sent? Wouldn't the app be able to use the current gps position as the starting location?

Should there be any maneuver data involved? e.g. An array of all maneuvers to reach the next waypoint or final destination?

I think this is a good idea. Example: A gas app can make use of the exact route and fuel level to suggest possible gas stations on the route. However, it may complicate the use case if there is a lot of maneuver data.

joeygrover commented 6 years ago

Is the eta to the next waypoint, or to the final destination?

ETA to destination

Should there be any maneuver data involved? e.g. An array of all maneuvers to reach the next waypoint or final destination?

Yes, I think this would be useful.

Q. Is there a need for the start location to sent? Wouldn't the app be able to use the current gps position as the starting location?

This was intended to be the trip in its entirety from start to finish. An app my subscribe to the navigation service after the trip has already started and therefore it's GPS location will not be the starting location of the trip.

I think if we add the maneuver list maybe we add upcomingManeuvers and completedManeuvers and we do the same for way points.

theresalech commented 6 years ago

The Steering Committee voted to defer this proposal, keeping it in review until our next meeting on 2018-09-04, to allow additional time for SDLC members to advise on any other information navigation apps have that should be included in this section of the proposal (example: avoid highways, tolls, ferries, etc.). This will help to make sure that the service provider is providing as much information as possible. The Project Maintainer will also work to provide sample parameters.

MazdaMKok commented 6 years ago

I mentioned in the last meeting that we may want to include Avoid Highways, Tolls, and Ferries? Does anybody think we need to have this data?

Home & work locations could also be helpful for a gas app. They could use this data to find a station near home or work.

Favorite Locations/Want To Go Locations may be a nice piece of data as well. An app could use this to notify the driver that he/she is passing by a Favorite/Want To Go location

If the map app has VR capabilities could be one other data point worth looking at.

The different types of maps that a map can provide and switch between, such as satellite, terrain, or default.

What are your opinions?

joeygrover commented 6 years ago

Here's what I have adding in maneuvers. I'm not 100% this is how it should be and welcome all comments.

<struct name="NavigationServiceData">
    <description> This data is related to what a navigation service would provide</description>

    <param name="maneuvers" type="Maneuver" array="true" minsize="2" mandatory="false">
        <description> This array should be ordered with the first object being the start location of the current trip and the last object the final destination. </description>
    </param>
    <param name="nextManeuverETA" type="DateTime" mandatory="false"/>

    <param name="destination" type="LocationDetails" mandatory="false"/>
    <param name="destinationETA" type="DateTime" mandatory="false"/>

</struct>

<struct name="Maneuver">
    <param name="locationDetails" type="LocationDetails" mandatory="true">
    </param>
    <param name="maneuverAction" type="ManeuverAction" mandatory="true">
    </param>    
    <param name="maneuverDirection" type="Direction" mandatory="false">
    </param>
    <param name="maneuverModifier" type="DirectionModifier" mandatory="false">
    </param>
</struct>

 <enum name="ManeuverAction">
        <element name="TURN">
        </element>
        <element name="EXIT">
        </element>
        <element name="STAY">
        </element>              
        <element name="MERGE">
        </element>     
 <enum>

 <enum name="Direction">
        <element name="RIGHT">
        </element>
        <element name="LEFT">
        </element>
        <element name="STRAIGHT">
        </element>    
         <element name="U">
        </element>                
<enum>

 <enum name="DirectionModifier">
        <element name="SLIGHT">
        </element>
        <element name="SHARP">
        </element>        
 </enum> 

Supported RPCs

SendLocation
GetWayPoints
SubscribeWayPoints
OnWayPointChange
MazdaMKok commented 6 years ago

Thanks @joeygrover for the data structures, but I have one comment. For the ManeuverAction enum, I believe we should add another ManeuverAction named "MERGE"

I don't believe that would be a redundancy and could be a good piece of data to capture.

joeygrover commented 6 years ago

@MazdaMKok great suggestion. I missed that one. Previous comment updated to include MERGE.

jordynmackool commented 6 years ago

The following was discussed in the Steering Committee meeting on 2018-09-04, with regards to this proposal: SDLC members are looking for more in-depth features in Navigation Service. Whereas navigation service data would be intended to use for data that changes quite often, the data discussed thus far seems somewhat static. We therefore might want to consider adding RPCs that handle these types of actions that the navigation service would support, rather than adding to the navigation service itself. It was determined that this proposal should be deferred, keeping the proposal in review until the next Steering Committee meeting on 2018- 09-18, to allow SDLC members with navigation expertise to weigh in. It was agreed that members should reach out to their navigation partners to make sure their voices are heard as well.

tvkanters commented 6 years ago

From TomTom's perspective:

Some more general questions:

MazdaMKok commented 6 years ago

@tvkanters Thanks for your feedback. As an extension of the last question asked, do we want this API to only contain purely navigation data, from the start location to end location, or are we trying to pull as much relevant data the nav service can provide.

My belief is that we should try to collect as much relevant data as we can so that other app developers can leverage this data to provide a higher level of end-user utility. It's great that we can get apps to effectively communicate with other apps, but we need robust data structs to make the App Services proposal really stand out as a key feature of SDL and to support the creativity of app developers

MazdaMKok commented 6 years ago

I understand that what I recommended, above, could be very intensive in order to collect all those data points. Since this topic hasn't moved much for about 2 weeks, maybe we could just collect some basic info from the struct that Joey provided and expand upon it at a later time.

jordynmackool commented 6 years ago

The Steering Committee voted to return this proposal for revisions on 2018-09-18. The author will revise based on the feedback provided by the TomTom and Mazda teams left on the review issue. The proposal will be clarified with regards to dynamic and static data, also noting that GetAppServiceData should return all parameters, and only data that changed will be included in OnAppServiceData.

joeygrover commented 5 years ago

For reference the service data is as followed:

Navigation

A navigation service is defined as a service that is currently listed as the navigation provider. This service type will likely deprecate the TBT RPCs. However, when the Service managers are created they could send the TBT information based on the service manager to maintain compatibility with older head units.

    <struct name="NavigationServiceManifest">
        <param name="acceptsWayPoints" type="Boolean" mandatory="false">
        <description>  Informs the subscriber if this service can actually accept way points.</description>
        </param>
    </struct>

    <struct name="NavigationServiceData">
        <description> This data is related to what a navigation service would provide.</description>

        <param name="timestamp" type="" mandatory="true">
            <description> This is the timestamp of when the data was generated. This is to ensure any time or distance given in the data can accurately be adjusted if necessary. </description>
        </param>

        <param name="origin" type="LocationDetails" mandatory="false"/>

        <param name="destination" type="LocationDetails" mandatory="false"/>

        <param name="destinationETA" type="DateTime" mandatory="false"/>

       <param name="wayPoints" type="LocationDetails" mandatory="false" array="true">
           <description> The current way points for the current navigation session. See also LocationDetails</description>
        </param>

        <param name="instructions" type="NavInstruction" array="true" mandatory="false">
            <description> This array should be ordered with the first object being the start location of the current trip and the last object the final destination. </description>
        </param>

        <param name="nextInstructionETA" type="DateTime" mandatory="false"/>
        <param name="nextInstructionDistance" type="Float" mandatory="false">
            <description>The distance to this instruction from current location. This should only be updated ever .1 unit of distance. For more accuracy the consumer can use the GPS location of itself and the next instruction. </description>
        </param>
        <param name="nextInstructionDistanceScale" type="Float" mandatory="false">
            <description>Distance till next maneuver (starting from) from previous maneuver.</description>
        </param>

    </struct>

    <struct name="NavInstruction">
        <param name="locationDetails" type="LocationDetails" mandatory="true">
        </param>

        <param name="action" type="NavAction" mandatory="true">
        </param>    

        <param name="direction" type="int" minValue="0" maxValue="359" mandatory="false">
            <description>The angle at which this instruction takes place. For example, 0 would mean straight, 45 a sharp right, 180 a U-Turn, etc. </description>
        </param>

        <param name="junctionType" type="NavJunctionType" mandatory="false">
        </param>    
    </struct>

    <enum name="NavAction">
        <element name="TURN">
            <description> Using this action plus a supplied direction can give the type of turn. </description>
        </element>
        <element name="EXIT">
        </element>
        <element name="STAY">
        </element>              
        <element name="MERGE">
        </element>
        <element name="FERRY">
        </element>        
    <enum>

    <enum name="NavJunctionType">
        <element name="REGULAR">
            <description> A junction that represents a standard intersection with a single road crossing another.</description>
        </element>
        <element name="BIFURCATION">
            <description> A junction where the road splits off into two paths; a fork in the road.</description>
        </element>
        <element name="MULTI_CARRIAGEWAY">
            <description> A junction that has multiple intersections and paths.</description>
        </element>              
        <element name="ROUNDABOUT">
            <description>A junction where traffic moves in a single direction around a central, non-traversable point to reach one of the connecting roads.</description>
        </element>
        <element name="TRAVESABLE_ROUNDABOUT">
        <description> Similar to a roundabout, however the center of the roundabout is fully traversable. Also known as a mini-roundabout.</description>
        </element>
        <element name="JUGHANDLE">
        <description>A junction where lefts diverge to the right, then curve to the left, converting a left turn to a crossing maneuver.</description>
        </element>
        <element name="ALL_WAY_YIELD">
            <description> Multiple way intersection that allows traffic to flow based on priority; most commonly right of way and first in, first out.</description>
        </element>
        <element name="TURN_AROUND">
            <description> A junction designated for traffic turn arounds.</description>
        </element>
    <enum>
RPCs to be handled:
joeygrover commented 5 years ago

@MazdaMKok @tvkanters do either of you have any feedback on the new changes?

kshala-ford commented 5 years ago

I'm sorry if I chime in late but I have some questions:

A navigation service is defined as a service that is currently listed as the navigation provider. This service type will likely deprecate the TBT RPCs. However, when the Service managers are created they could send the TBT information based on the service manager to maintain compatibility with older head units.

In how far are the existing RPCs compatible to the new service based approach? I'm considering backward compatibility done by the SDL libraries to avoid app developers to have double the work.

Looking at the params nextInstructionETA, nextInstructionDistance, nextInstructionDistanceScale I believe they should be moved into NavInstruction so that every instruction ETA with distance between instructions is known at a time. The reason for it is that some instructions are so close to each other that nav systems do "Do a right turn, then immediately to a left turn". With the current approach this cannot be realized but with ShowConstantTBT you could.

<struct name="NavInstruction">

Can we always use Navigation instead of Nav? So it should be

<param name="wayPoints" type="LocationDetails" mandatory="false" array="true">

  1. waypoints not wayPoints
  2. Is this a waypoint array of the whole route, the remaining route or until the next instruction? Actually the reason I'm asking is that many routing APIs provide waypoints between instructions. Should NavigationInstruction contain a waypoint array to the instruction point? This could be an addition or instead of waypoints within NavigationServiceData.

Above waypoint change could solve my next questions:

  1. How do we handle left or right turn-arounds?
  2. How do we solve left side driving? Thinking about clockwise roundabouts (see this crazy movie)
  3. Where are maneuver icons? Do we continue with this?

I guess question 3 is answered by "we generate images using instruction direction"? If so how do we know the roundabout direction? How do we know the u-turn direction? Having waypoints within the instruction could solve it by following and evaluating the waypoints for each instruction (or at least the current instruction).

MazdaMKok commented 5 years ago

@joeygrover `

The angle at which this instruction takes place. For example, 0 would mean straight, 45 a sharp right, 180 a U-Turn, etc.
    </param>`

I believe 90 would be a sharp right, correct? I wish I had more experience with Navigation so I can contribute more to this issue. Sorry for that.

@kshala-ford I did not know that it's common for routing API's to give waypoints in between instructions. In this case we may have to have an additional waypoints because this type of waypoint is a different type of waypoint than an end user would think about. I believe the original wayPoints was to describe waypoints in between the starting location and end location, not in between instructions. I could be mistaken... @joeygrover can correct me if i'm wrong.

To address left side driving and turn arounds (U-turns) I believe all U-turns when you're driving on the right side will be made going left and all U-turns when you're driving on the left side will be made going right. There should be an additional parameter to address left/right side driving.

joeygrover commented 5 years ago

In how far are the existing RPCs compatible to the new service based approach? I'm considering backward compatibility done by the SDL libraries to avoid app developers to have double the work.

The comment added is specifically to call out the manager would do the heavy lifting in translating to older RPCs. I believe the new service definitions are getting close enough to make the translation easy enough. If there is something I missed please let me know.

Looking at the params nextInstructionETA, nextInstructionDistance, nextInstructionDistanceScale I believe they should be moved into NavInstruction

The only reason the ETA wasn't included into the NavInstruction was because of its likely update rate, but if the pros outweigh the cons I'm alright with adding an ETA to each instruction in the array. I believe the immediate turn feature could still be done using GPS location of the two instructions, but again I don't have strong feelings either way.

Can we always use Navigation instead of Nav?

Sounds good to me.

Waypoints

  1. Ah, yup agreed, need to fix that typo.
  2. (Including feedback to @MazdaMKok ) The way I understood waypoints is that they are actual stops along a navigation instruction set, as in start and end as well as any minor stop along the way. The service data would provide only the remaining waypoints, meaning if a waypoint has already been reached it will be removed from the list with the head of the list pointing to the next waypoint. I can see how it might be difficult to organize the waypoints into the status of the instruction set so I will have to give this more thought. Maybe including the waypoints into the instruction set as NavigationAction WAYPOINT?

How do we handle left or right turn-arounds? I believe you mean U_TURN? Would using something like 179 for degrees mean right and 181 mean left work?

How do we solve left side driving? Thinking about clockwise roundabouts (see this crazy movie)

Good question, completely open for suggestions here.

Where are maneuver icons? Do we continue with this?

They aren't included in the proposal, but I think they could be added. The app services proposal already has a method for retrieval so it would just need a name for the file.

I believe 90 would be a sharp right, correct? I wish I had more experience with Navigation so I can contribute more to this issue. Sorry for that.

Yes, you are correct, not sure what I was thinking.

I wish I had more experience with Navigation so I can contribute more to this issue. Sorry for that.

I am in the same boat. I am not trying to act as the expert but rather provide something that experts can weigh in on and hopefully correct me.

jordynmackool commented 5 years ago

The Steering Committee voted to defer this proposal on 2019-01-22, keeping it in review to allow additional time for SDLC members with navigation expertise to weigh in and respond to this comment. It was agreed that members should reach out to their navigation partners to probe if they can provide additional guidance and review on this proposal.

tvkanters commented 5 years ago

Input from how these topics are considered from TomTom's perspective:

jordynmackool commented 5 years ago

The Steering Committee voted to return this proposal for revisions on 2019-01-29 to allow the author to update the proposal to include feedback given in this comment.

jordynmackool commented 5 years ago

The original proposal "SDL 0167 - App Services" was accepted by the Steering Committee on 2018-05-23, alterations to the proposal have been requested.

This review will focus on only the changes present in the navigation section of the proposal: here. Review for this section begins now and runs through February 12, 2019.

Note: All comments should be contained in this issue!

The original proposal is available here:

https://github.com/smartdevicelink/sdl_evolution/blob/master/proposals/0167-app-services.md

Noted Revisions

joeygrover commented 5 years ago

For ease of understanding, here is the new Navigation Service information:

Navigation

A navigation service is defined as a service that is currently listed as the navigation provider. This service type will likely deprecate the TBT RPCs. However, when the Service managers are created they could send the TBT information based on the service manager to maintain compatibility with older head units.

    <struct name="NavigationServiceManifest">
        <param name="acceptsWayPoints" type="Boolean" mandatory="false">
        <description>  Informs the subscriber if this service can actually accept way points.</description>
        </param>
    </struct>

    <struct name="NavigationServiceData">
        <description> This data is related to what a navigation service would provide.</description>

        <param name="timestamp" type="" mandatory="true">
            <description> This is the timestamp of when the data was generated. This is to ensure any time or distance given in the data can accurately be adjusted if necessary. </description>
        </param>

        <param name="origin" type="LocationDetails" mandatory="false"/>

        <param name="destination" type="LocationDetails" mandatory="false"/>

        <param name="destinationETA" type="DateTime" mandatory="false"/>

        <param name="instructions" type="NavigationInstruction" array="true" mandatory="false">
            <description> This array should be ordered with all remaining instructions. The start of this array should always contain the next instruction.</description>
        </param>

        <param name="nextInstructionETA" type="DateTime" mandatory="false"/>
        <param name="nextInstructionDistance" type="Float" mandatory="false">
            <description>The distance to this instruction from current location. This should only be updated ever .1 unit of distance. For more accuracy the consumer can use the GPS location of itself and the next instruction. </description>
        </param>
        <param name="nextInstructionDistanceScale" type="Float" mandatory="false">
            <description>Distance till next maneuver (starting from) from previous maneuver.</description>
        </param>

        <param name="prompt" type="String" mandatory="false">
            <description>This is a prompt message that should be conveyed to the user through either display or voice (TTS). This param will change often as it should represent the following: approaching instruction, post instruction, alerts that affect the current navigation session, etc.</description>
        </param>
    </struct>

    <struct name="NavigationInstruction">
        <param name="locationDetails" type="LocationDetails" mandatory="true">
        </param>

        <param name="action" type="NavigationAction" mandatory="true">
        </param>

        <param name="eta" type="DateTime" mandatory="false">
        </param>    

        <param name="bearing" type="int" minValue="0" maxValue="359" mandatory="false">
            <description>The angle at which this instruction takes place. For example, 0 would mean straight, <=45 is bearing right, >= 135 is sharp right, between 45 and 135 is a regular right, and 180 is a U-Turn, etc. </description>
        </param>

        <param name="junctionType" type="NavigationJunction" mandatory="false">
        </param>    

        <param name="drivingSide" type="Direction" mandatory="false">
            <description>Used to infer which side of the road this instruction takes place. For a U-Turn (Action=Turn, direction=180) this will determine which direction the turn should take place. </description>
        </param>

        <param name="details" type="String" mandatory="false">
            <description>This is a string representation of this instruction, used to display instructions to the users. This is not intended to be read aloud to the users, see the param prompt in NavigationServiceData for that.</description>
        </param>

        <param name "image"  type="Image" mandatory="false">
            <description>An image representation of this instruction. </description>
        </param>
    </struct>

    <enum name="NavigationAction">
        <element name="TURN">
            <description> Using this action plus a supplied direction can give the type of turn. </description>
        </element>
        <element name="EXIT">
        </element>
        <element name="STAY">
        </element>              
        <element name="MERGE">
        </element>
        <element name="FERRY">
        </element>
        <element name="CAR_SHUTTLE_TRAIN">
        </element>
        <element name="WAYPOINT">
        </element>      
    <enum>

    <enum name="NavigationJunction">
        <element name="REGULAR">
            <description> A junction that represents a standard intersection with a single road crossing another.</description>
        </element>
        <element name="BIFURCATION">
            <description> A junction where the road splits off into two paths; a fork in the road.</description>
        </element>
        <element name="MULTI_CARRIAGEWAY">
            <description> A junction that has multiple intersections and paths.</description>
        </element>              
        <element name="ROUNDABOUT">
            <description>A junction where traffic moves in a single direction around a central, non-traversable point to reach one of the connecting roads.</description>
        </element>
        <element name="TRAVESABLE_ROUNDABOUT">
        <description> Similar to a roundabout, however the center of the roundabout is fully traversable. Also known as a mini-roundabout.</description>
        </element>
        <element name="JUGHANDLE">
        <description>A junction where lefts diverge to the right, then curve to the left, converting a left turn to a crossing maneuver.</description>
        </element>
        <element name="ALL_WAY_YIELD">
            <description> Multiple way intersection that allows traffic to flow based on priority; most commonly right of way and first in, first out.</description>
        </element>
        <element name="TURN_AROUND">
            <description> A junction designated for traffic turn arounds.</description>
        </element>
    </enum>

    <enum name="Direction">
        <element name="LEFT"/>
        <element name="RIGHT"/>
    </enum>
RPCs to be handled:
joeygrover commented 5 years ago

@MazdaMKok @tvkanters do either of you have any feedback on the new changes?

jordynmackool commented 5 years ago

The Steering Committee voted to accept this proposal. There was additional discussion regarding lane guidance being included in this iteration of the proposal. It was concluded that with how complicated this App Service Type implementation is, it should be put on hold to be included. The author noted that a prompt string was added to the servicedata that could accomplish this in a non-structure manner, but still gets the point across; The author noted that a prompt string was added to the servicedata of the Navigation service that could accomplish this in a non-structured manner. Adding this string made it a little freer forming, as currently the implementation has a good base. Adding additional features in the future is always an option.