Skip to content

Add stairsTimeFactor to StreetEdge - #3832

Merged
hannesj merged 6 commits into
opentripplanner:dev-2.xfrom
entur:otp2_stairs_time_factor
Feb 9, 2022
Merged

Add stairsTimeFactor to StreetEdge#3832
hannesj merged 6 commits into
opentripplanner:dev-2.xfrom
entur:otp2_stairs_time_factor

Conversation

@hannesj

@hannesj hannesj commented Jan 26, 2022

Copy link
Copy Markdown
Contributor

Summary

Currently stairs are traversed using the same speed as regular horizontal ways. This adds a specific factor which slows down stair traversal by a configurable factor

Issue

Currently traversal between two platforms in Oslo S takes less than two minutes, which is unrealistic. Much of this is due to the stairs being too quick to traverse.

image

Unit tests

None added (yet)

Code style

Documentation

None required

Changelog

The changelog file
is generated from the pull-request title, make sure the title describe the feature or issue fixed.
To exclude the PR from the changelog add [changelog skip] in the title.

@hannesj hannesj added the !Improvement A functional improvement or micro feature label Jan 26, 2022
@hannesj hannesj added this to the 2.1 milestone Jan 26, 2022
@hannesj
hannesj requested a review from a team as a code owner January 26, 2022 22:23
@hannesj
hannesj force-pushed the otp2_stairs_time_factor branch from 4f77709 to 244b2e1 Compare January 27, 2022 13:05
@t2gran

t2gran commented Jan 27, 2022

Copy link
Copy Markdown
Member

We should probably not apply the elevation adjustments to stairs.

@t2gran
t2gran requested review from optionsome and t2gran January 27, 2022 14:37
@t2gran
t2gran marked this pull request as draft January 27, 2022 14:37
@hannesj
hannesj marked this pull request as ready for review February 1, 2022 08:58
@hannesj

hannesj commented Feb 1, 2022

Copy link
Copy Markdown
Contributor Author

We should probably not apply the elevation adjustments to stairs.

This is now done by using raw length on stairs

Comment thread src/main/java/org/opentripplanner/routing/api/request/RoutingRequest.java Outdated
@t2gran
t2gran removed their request for review February 1, 2022 10:45
Comment thread src/main/java/org/opentripplanner/routing/api/request/RoutingRequest.java Outdated

@optionsome optionsome left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@optionsome

Copy link
Copy Markdown
Member

I'm getting different duration on walk leg with and without elevation data on long outdoor stairs (https://www.openstreetmap.org/way/160560554) where I set the origin and destination to be along the stairs. Could there still be some issue with splitting stair edges or maybe I'm missing something?

@hannesj
hannesj marked this pull request as draft February 3, 2022 14:53
@hannesj

hannesj commented Feb 4, 2022

Copy link
Copy Markdown
Contributor Author

Could there still be some issue with splitting stair edges or maybe I'm missing something?

This seem to be covered by #3862

@flaktack

flaktack commented Feb 4, 2022

Copy link
Copy Markdown
Contributor

I'm getting different duration on walk leg with and without elevation data on long outdoor stairs (https://www.openstreetmap.org/way/160560554) where I set the origin and destination to be along the stairs. Could there still be some issue with splitting stair edges or maybe I'm missing something?

There could be two related factors:

@hannesj

hannesj commented Feb 8, 2022

Copy link
Copy Markdown
Contributor Author

Could there still be some issue with splitting stair edges or maybe I'm missing something?

Yes, the stairs flag was not being copied to the split edges. This should be handled now. However there are still some flags, which do not get copied, and we should check which ones should be copied.

@hannesj
hannesj marked this pull request as ready for review February 8, 2022 08:42
@hannesj
hannesj merged commit 8b7ebe6 into opentripplanner:dev-2.x Feb 9, 2022
@hannesj
hannesj deleted the otp2_stairs_time_factor branch February 9, 2022 08:47
t2gran pushed a commit that referenced this pull request Feb 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

!Improvement A functional improvement or micro feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants