Skip to content

Make indoor=area and indoor=corridor routable for UK OSM tag mapper - #6119

Merged
optionsome merged 13 commits into
opentripplanner:dev-2.xfrom
Aubin-MaaS:indoor-routing
Oct 31, 2024
Merged

Make indoor=area and indoor=corridor routable for UK OSM tag mapper#6119
optionsome merged 13 commits into
opentripplanner:dev-2.xfrom
Aubin-MaaS:indoor-routing

Conversation

@miklcct

@miklcct miklcct commented Oct 7, 2024

Copy link
Copy Markdown
Contributor

PR Instructions

Summary

This makes indoor=area and indoor=corridor routable with pedestrian permission.

It consists of 3 parts.

  1. Refactor existing area handling.
  2. Make indoor=area, indoor=corridor areas.
  3. Assign pedestrian permissions to indoor=area and indoor=corridor.

Issue

Fixes #6118

Unit tests

Added to test areas and permissions

Documentation

Generated

@miklcct
miklcct requested a review from a team as a code owner October 7, 2024 10:44
@codecov

codecov Bot commented Oct 7, 2024

Copy link
Copy Markdown

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 69.91%. Comparing base (e58513f) to head (5cc750a).
Report is 16 commits behind head on dev-2.x.

Additional details and impacted files
@@              Coverage Diff              @@
##             dev-2.x    #6119      +/-   ##
=============================================
- Coverage      69.91%   69.91%   -0.01%     
- Complexity     17701    17715      +14     
=============================================
  Files           1996     1996              
  Lines          75305    75338      +33     
  Branches        7705     7719      +14     
=============================================
+ Hits           52646    52669      +23     
- Misses         19989    19992       +3     
- Partials        2670     2677       +7     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@miklcct
miklcct force-pushed the indoor-routing branch 3 times, most recently from cb8a485 to 963c08d Compare October 8, 2024 09:15
Comment thread src/main/java/org/opentripplanner/openstreetmap/model/OSMWithTags.java Outdated
Comment thread src/main/java/org/opentripplanner/openstreetmap/model/OSMWay.java Outdated
Comment thread src/main/java/org/opentripplanner/openstreetmap/model/OSMWay.java
@miklcct
miklcct force-pushed the indoor-routing branch 2 times, most recently from eac00b8 to dda0d34 Compare October 15, 2024 10:16
Comment thread application/src/main/java/org/opentripplanner/osm/model/OsmWithTags.java Outdated
@miklcct

miklcct commented Oct 15, 2024

Copy link
Copy Markdown
Contributor Author

Sorry for the delays and problems. Java is not my best thing for me to work at, our current production is at 2.6 and the module refactor in 2.7 has made massive headache for me in setting up my IDE.

@leonardehrenfried leonardehrenfried added the !Improvement A functional improvement or micro feature label Oct 15, 2024

@vesameskanen vesameskanen left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code is OK but this change is not totally problem free. I checked ten cities around the world to understand where and how indoor=area is used. It is quite rare in data. In half of the cases I found, indoor=area is applied to restaurants and shops in shopping center or market hall. Routing should not use those kind of areas. Anyway, in some cities, especially in Paris, there are significant structures where indoor area routing would be useful. For example: https://www.openstreetmap.org/way/558799205 .

It is also necessary to test how well OTP can handle overlapping indoor areas on different levels.

@miklcct
miklcct requested a review from vesameskanen October 15, 2024 14:17
@miklcct

miklcct commented Oct 15, 2024

Copy link
Copy Markdown
Contributor Author

Code is OK but this change is not totally problem free. I checked ten cities around the world to understand where and how indoor=area is used. It is quite rare in data. In half of the cases I found, indoor=area is applied to restaurants and shops in shopping center or market hall. Routing should not use those kind of areas. Anyway, in some cities, especially in Paris, there are significant structures where indoor area routing would be useful. For example: https://www.openstreetmap.org/way/558799205 .

It is also necessary to test how well OTP can handle overlapping indoor areas on different levels.

That may be a problem as it seems that, from the wiki, that indoor=area and indoor=room are distinguished by if they are enclosed by walls, rather than the usage of the place. (indoor=area is not walled, while indoor=room is by default, walled). In this case possibly if these places are not intended for through traffic, some access= tags are needed.

I have just sent an email to the tagging mailing list for discussion.

@vesameskanen

Copy link
Copy Markdown
Contributor

Getting the OSM data obey some new guidelines will take time. Meanwhile, we might consider configuring suitable permissions in OTP tag mapping.

I guess indoor=corridor sounds like always routable. We might require some additional tags from indoor areas to make them routable. Also, it is possible to add routing permissions to certain countries only using application/src/test/java/org/opentripplanner/osm/tagmapping/ mapper classes. I noticed that indoor tag conventions seem to vary heavily on each country.

Currently there is a bug which by default grants pedestrian and bicycle access to areas, and ignores country specific tag mapping. I have already fixed this and will open a PR soon.

@t2gran t2gran added this to the 2.7 (next release) milestone Oct 16, 2024
@optionsome optionsome changed the title Make indoor=area and indoor=corridor routable Make indoor=area and indoor=corridor routable for UK OSM tag mapper Oct 31, 2024
@optionsome
optionsome merged commit 283e3f9 into opentripplanner:dev-2.x Oct 31, 2024
t2gran pushed a commit that referenced this pull request Oct 31, 2024
@miklcct
miklcct deleted the indoor-routing branch November 26, 2024 16:16
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.

can't route over indoor area

5 participants