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

Apply stricter motor vehicle nothrough traffic rules in Finland #6254

Conversation

vesameskanen
Copy link
Contributor

Summary

According to OSM guide lines, ways tagged as service=parking_aisle,driveway,alley,emergency_access,drive-through are not intended for pass through traffic. It seems that at least in Finland, such ways often lack explicit access restriction tagging (e.g. access=private).

This PR extends the pass through traffic restrictions of Finland tag mapping for car routing to include such service ways. We may consider if the same logic should be applied in the default tag mapper class.

Two pass through methods are now renamed. Double negation in the names made those hard to understand.

Unit tests

A test for a service way added.

Documentation

Self-documenting code.

@vesameskanen vesameskanen requested a review from a team as a code owner November 15, 2024 08:53
Copy link

codecov bot commented Nov 15, 2024

Codecov Report

Attention: Patch coverage is 66.66667% with 2 lines in your changes missing coverage. Please review.

Project coverage is 69.71%. Comparing base (46be411) to head (48cac2e).
Report is 85 commits behind head on dev-2.x.

Files with missing lines Patch % Lines
.../opentripplanner/osm/tagmapping/FinlandMapper.java 50.00% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@              Coverage Diff              @@
##             dev-2.x    #6254      +/-   ##
=============================================
- Coverage      69.73%   69.71%   -0.03%     
- Complexity     17681    17695      +14     
=============================================
  Files           2007     2008       +1     
  Lines          75772    75831      +59     
  Branches        7754     7762       +8     
=============================================
+ Hits           52843    52863      +20     
- Misses         20216    20256      +40     
+ Partials        2713     2712       -1     

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


🚨 Try these New Features:

@miklcct
Copy link
Contributor

miklcct commented Nov 15, 2024

Could you please give some documentation on the OSM guideline, especially why alleys, driveways, drive-throughs, etc., should not be used for through traffic?

I don't see a problem driving through driveways / alleys / etc.

@vesameskanen
Copy link
Contributor Author

vesameskanen commented Nov 15, 2024

https://wiki.openstreetmap.org/wiki/Key:service

Car routing is supposed to find an appropriate route between 2 places. Taking a weird shortcut through a parking area or a drive-in restaurant line is not convenient.

@miklcct
Copy link
Contributor

miklcct commented Nov 15, 2024

I think applying the through-traffic prohibition on highway=service is more appropriate in this situation because highway=service means ways which are not in the public highway network.

However, There is nothing in the documentation says that these roads shouldn't be used for through traffic. The presence of these tags only represents the function of these roads in the road network, not for any access restrictions.

I still doubt if you should really disallow through traffic on these ways. If using such ways can result in a time / fuel saving, it should be presented to the user. A lot of drivers cut through highway=service across, for example, a car park or a fuel station, in order to avoid traffic.

@vesameskanen
Copy link
Contributor Author

Right - I would not suggest to exclude them if OTP had fastest/shortest/most convenient option for car routing. Currently it does not and we have to find a good compromise.

This change is based on a complaint of a client. As a reference, Google Maps does not suggest such shortcuts.

@miklcct
Copy link
Contributor

miklcct commented Nov 15, 2024

Can we generalise our bike safety factor to cars as well?

@leonardehrenfried
Copy link
Member

Just because some drivers cut through car parks and petrol stations, doesn't mean that we should encourage that.

Besides, this PR only proposes changes for Finland.

We kind of have something like safety value for cars: the edge-specific speed.

@leonardehrenfried
Copy link
Member

leonardehrenfried commented Nov 19, 2024

I had a read of https://wiki.openstreetmap.org/wiki/Tag:highway%3Dservice again. It says that highway=service often prohibits through traffic but this is not a general rule. For this reason I think this is a good change. In any case, it only applies to Finland.

I think it's only really relevant to the constant speed mapper because in a real one these service ways have a lower speed and would be discarded anyway.

Comment on lines 229 to 241
@Test
public void constantSpeedCarRouting() {
OsmTagMapper osmTagMapper = new ConstantSpeedFinlandMapper(20f);

var slowWay = new OsmWithTags();
slowWay.addTag("highway", "residential");
assertEquals(20f, osmTagMapper.getCarSpeedForWay(slowWay, true));

var fastWay = new OsmWithTags();
fastWay.addTag("highway", "motorway");
fastWay.addTag("maxspeed", "120 kmph");
assertEquals(20f, osmTagMapper.getCarSpeedForWay(fastWay, true));
}
Copy link
Member

Choose a reason for hiding this comment

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

I think this should be in a separate test file for the constant speed mapper.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree, moved.

@vesameskanen vesameskanen merged commit e3e6999 into opentripplanner:dev-2.x Nov 21, 2024
5 checks passed
@vesameskanen vesameskanen deleted the stricter-motor-vehicle-nothrough branch November 21, 2024 12:45
t2gran pushed a commit that referenced this pull request Nov 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants