-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Add a separate layer group in debug UI for elevator edges and vertices #6885
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
base: dev-2.x
Are you sure you want to change the base?
Changes from all commits
bb469d6
57eedbd
4da3aaf
c5f7ea3
fdb5bf5
67aac2b
d765661
633cf78
14f0bdb
c8b8de7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,12 +4,14 @@ | |
| import static org.opentripplanner.utils.lang.DoubleUtils.roundTo2Decimals; | ||
|
|
||
| import com.google.common.collect.Lists; | ||
| import java.time.Duration; | ||
| import java.util.Collection; | ||
| import java.util.List; | ||
| import org.opentripplanner.apis.support.mapping.PropertyMapper; | ||
| import org.opentripplanner.inspector.vector.KeyValue; | ||
| import org.opentripplanner.street.model.StreetTraversalPermission; | ||
| import org.opentripplanner.street.model.edge.Edge; | ||
| import org.opentripplanner.street.model.edge.ElevatorHopEdge; | ||
| import org.opentripplanner.street.model.edge.EscalatorEdge; | ||
| import org.opentripplanner.street.model.edge.StreetEdge; | ||
| import org.opentripplanner.utils.collection.ListUtils; | ||
|
|
@@ -24,7 +26,13 @@ protected Collection<KeyValue> map(Edge input) { | |
| case StreetEdge e -> mapStreetEdge(e); | ||
| case EscalatorEdge e -> List.of( | ||
| kv("distance", e.getDistanceMeters()), | ||
| kv("duration", e.getDuration().map(d -> d.toString()).orElse(null)) | ||
| kv("duration", e.getDuration().map(Duration::toString).orElse(null)) | ||
| ); | ||
| case ElevatorHopEdge e -> List.of( | ||
| kv("permission", e.getPermission()), | ||
| kv("levels", e.getLevels()), | ||
| kv("wheelchairAccessible", e.isWheelchairAccessible()), | ||
| kv("travelTime", e.getTravelTime()) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The default value for the travel time is 0 which means that it's not actually being used. Maybe it would be less confusing to not expose this field for the edge if it's 0 or expose it as null as otherwise it leads to confusion.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Mapping zero to null can be a problem if the value from OSM is zero. Having a default value of -1 is also not good imo because it would represent traveling back in time. Maybe travelTime could be defined as a Nullable Duration instead of an int. We could also just leave it as it is and expect the user to be able to look up what the value represents. What does @leonardehrenfried think of this?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The method is only used in tests and now here, so we don't have to adjust any calling code. I would indeed return |
||
| ); | ||
| default -> List.of(); | ||
| }; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,10 @@ | ||
| package org.opentripplanner.street.model.edge; | ||
|
|
||
| import java.util.Arrays; | ||
| import java.util.List; | ||
| import org.locationtech.jts.geom.Coordinate; | ||
| import org.locationtech.jts.geom.LineString; | ||
| import org.opentripplanner.framework.geometry.GeometryUtils; | ||
| import org.opentripplanner.framework.i18n.I18NString; | ||
| import org.opentripplanner.routing.api.request.preference.RoutingPreferences; | ||
| import org.opentripplanner.street.model.StreetTraversalPermission; | ||
|
|
@@ -98,6 +103,25 @@ public StreetTraversalPermission getPermission() { | |
| return permission; | ||
| } | ||
|
|
||
| @Override | ||
| public LineString getGeometryForDebugUi() { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would like us to think if we consider how the edge should look like in the debug UI to be part of its domain model. If it is, then this code can stay here. It it isn't we should create a new mapper inside the But I generally agree that there can be debug-specific geometries.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This could be easily moved into
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think @t2gran suggested this current solution (although I'm not sure if he gave any name suggestions for the method name).
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Okay then, lets call it |
||
| // If coordinates are equal, move the other one slightly to make the edge visible | ||
| if (fromv.getCoordinate().equals(tov.getCoordinate())) { | ||
| Coordinate newTo = tov.getCoordinate().copy(); | ||
| newTo.setX(tov.getX() + 0.000001); | ||
| return GeometryUtils.makeLineString(Arrays.asList(fromv.getCoordinate(), newTo)); | ||
| } | ||
| List<Coordinate> segmentCoordinates = Arrays.asList(fromv.getCoordinate(), tov.getCoordinate()); | ||
| return GeometryUtils.makeLineString(segmentCoordinates); | ||
| } | ||
|
|
||
| /** | ||
| * The number of levels that the elevator travels | ||
| */ | ||
| public double getLevels() { | ||
optionsome marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| return levels; | ||
| } | ||
|
|
||
| public int getTravelTime() { | ||
| return travelTime; | ||
| } | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.