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

Remove ElectronicHorizon struct #2949

Merged
merged 3 commits into from
Apr 28, 2021

Conversation

azarovalex
Copy link
Contributor

@azarovalex azarovalex commented Apr 23, 2021

Description

Fixes #2906.

Implementation

Removed the ElectronicHorizon struct and renamed ElectronicHorizon.Edge to RoadGraph.Edge.

@azarovalex azarovalex added the op-ex Refactoring, Tech Debt or any other operational excellence work. label Apr 23, 2021
@azarovalex azarovalex self-assigned this Apr 23, 2021
@azarovalex azarovalex changed the base branch from main to release-v2.0 April 23, 2021 13:23
@azarovalex azarovalex changed the title Remove ElectronicHorizon struct Remove ElectronicHorizon struct Apr 23, 2021
Comment on lines 4 to 10
/**
An electronic horizon is a probable path (or paths) of a vehicle. This structure contains metadata about an edge in a routing graph. For example, an edge may represent a road segment between two intersections or between the two ends of a bridge. An edge may traverse multiple road objects, and a road object may be associated with multiple edges. The road network ahead of the user is represented as a tree of edges. Each intersection has outlet edges. In turn, each edge has a probability of transition to another edge, as well as details about the road segment that the edge traverses. You can use these details to influence application behavior based on predicted upcoming conditions.

During active turn-by-turn navigation, the user-selected route and its metadata influence the path of the electronic horizon determined by `RouteController`. During passive navigation (free-driving), no route is actively selected, so `PassiveLocationDataSource` will determine the most probable path from the vehicle’s current location. You can receive notifications about changes in the current state of the electronic horizon by observing the `Notification.Name.electronicHorizonDidUpdatePosition`, `Notification.Name.electronicHorizonDidEnterRoadObject`, and `Notification.Name.electronicHorizonDidExitRoadObject` notifications.

Use a `RoadGraph` object to get an edge with a given identifier.
*/
Copy link
Contributor

Choose a reason for hiding this comment

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

Since this is now the documentation comment for the edge type specifically, we shouldn’t bury the lede in terms of describing an edge. Let’s start off with the first paragraph of the old documentation, then follow with the rest.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Rearranged some lines in this doc.

@@ -343,7 +343,8 @@ extension ElectronicHorizon {
public static let positionKey: NotificationUserInfoKey = .init(rawValue: "position")

/**
A key in the user info dictionary of a `Notification.Name.electronicHorizonDidUpdatePosition` notification. The corresponding value is an `ElectronicHorizon` at the root of a tree of edges in the routing graph. */
A key in the user info dictionary of a `Notification.Name.electronicHorizonDidUpdatePosition` notification. The corresponding value is an `ElectronicHorizonEdge` at the root of a tree of edges in the routing graph. This graph represents a probable path (or paths) of a vehicle within the routing graph for a certain distance in front of the vehicle, thus extending the user’s perspective beyond the “visible” horizon as the vehicle’s position and trajectory change.
Copy link
Contributor

Choose a reason for hiding this comment

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

👍 It’s good that we’re putting some more detail here, now that there isn’t a general “electronic horizon” type to look up.

@azarovalex azarovalex force-pushed the azarovalex/electronic-horizon-edge branch from 75fb502 to 83595ec Compare April 27, 2021 15:48
Example/ViewController+FreeDrive.swift Outdated Show resolved Hide resolved
@@ -308,28 +308,28 @@ public extension Notification.Name {
/**
Posted when the user’s position in the electronic horizon changes. This notification may be posted multiple times after `electronicHorizonDidEnterRoadObject` until the user transitions to a new electronic horizon.

The user info dictionary contains the keys `ElectronicHorizon.NotificationUserInfoKey.positionKey`, `ElectronicHorizon.NotificationUserInfoKey.treeKey`, `ElectronicHorizon.NotificationUserInfoKey.updatesMostProbablePathKey`, and `ElectronicHorizon.NotificationUserInfoKey.distancesByRoadObjectKey`.
The user info dictionary contains the keys `RoadGraph.Edge.NotificationUserInfoKey.positionKey`, `RoadGraph.Edge.NotificationUserInfoKey.treeKey`, `RoadGraph.Edge.NotificationUserInfoKey.updatesMostProbablePathKey`, and `RoadGraph.Edge.NotificationUserInfoKey.distancesByRoadObjectKey`.
Copy link
Contributor

Choose a reason for hiding this comment

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

RoadGraph.NotificationUserInfoKey? Here and everywhere else

Sources/MapboxCoreNavigation/CoreConstants.swift Outdated Show resolved Hide resolved
The most probable path may be split at some point if some of edges have a low probability difference (±0.05). For example, `outletEdges` can contain more than one edge with `level` set to 0. Currently, there is a maximum limit of one split per electronic horizon.
*/
public let outletEdges: [Edge]
public let outletEdges: [RoadGraph.Edge]
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this change needed?

Sources/MapboxCoreNavigation/EHorizon/RoadGraphEdge.swift Outdated Show resolved Hide resolved
Sources/MapboxCoreNavigation/EHorizon/RoadGraphPath.swift Outdated Show resolved Hide resolved
Sources/MapboxCoreNavigation/EHorizon/RoadGraph.swift Outdated Show resolved Hide resolved
Copy link
Contributor

@bamx23 bamx23 left a comment

Choose a reason for hiding this comment

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

LGTM

@azarovalex azarovalex force-pushed the azarovalex/electronic-horizon-edge branch from fc9b495 to 7ac85dc Compare April 28, 2021 15:11
@azarovalex azarovalex merged commit 9987b46 into release-v2.0 Apr 28, 2021
@azarovalex azarovalex deleted the azarovalex/electronic-horizon-edge branch April 28, 2021 15:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
op-ex Refactoring, Tech Debt or any other operational excellence work.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Electronic horizon position notification should directly contain pointer to edge tree
3 participants