Country specific highway access - Project-OSRM / osrm-backend #7038

Project-OSRM / osrm-backend

Open Source Routing Machine - C++ backend

http://map.project-osrm.org

BSD 2-Clause "Simplified" License

6.42k stars 3.4k forks source link

Country specific highway access - Project-OSRM / osrm-backend #7038 #7022

Closed tombay closed 1 month ago

tombay commented 2 months ago

An issue https://github.com/Project-OSRM/osrm-backend/issues/6710 has been logged (reopening https://github.com/Project-OSRM/osrm-backend/issues/6701). Address the issue of highway=trunk (and trunk_link) being blocked for foot and bicycle profiles. Discussed in https://wiki.openstreetmap.org/wiki/Key:motorroad and https://wiki.openstreetmap.org/wiki/OSM_tags_for_routing/Access_restrictions (esp #Alternative_ideas) proposed to address issue via osrm-backend ... --location-dependent-data

Tasklist

[x] CHANGELOG.md entry (How to write a changelog entry)
[x] update relevant Wiki pages
[x] add tests (see testing documentation)
[ ] review
[ ] adjust for comments
[ ] cherry pick to release branch

Requirements / Relations

Link any requirements here. Other pull requests this PR is based on?

tombay commented 2 months ago

I had inadvertently created pr #7013 on not the master branch. It deleted when I removed that branch. #6710 and #6701 have described the issue.

frodrigo commented 1 month ago

Please, can you include only one simplified geojson n the PR.
Can you redact the list of commit to simplify the history ? And make it more easy to review.
Can we merge it into the default foot en bicyle profile ?

tombay commented 1 month ago

I have updated foot.lua and bicycle.lua to support country data. I will remove countryfoot.lua and countrybicycle.lua.
The geojson file test/data/countrytest.geojson defines a simplified array of countries: Switzerland (CHE), Finland (FIN), France (FRA), Belgium (BEL), Greece (GRC), Ireland (IRL)and Lithuania (LIU). This file along with features/foot/countryspeeds.feature outlines the relationships.
I am not exactly sure how to clean up the commit stream. (In my repository == git branch master ; git rebase --interactive ?)

frodrigo commented 1 month ago

git rebase --interactive

Yes. It it is the first time you use it. Be careful, make a copy of you local clone ;-).

Switzerland (CHE), Finland (FIN), France (FRA), Belgium (BEL), Greece (GRC), Ireland (IRL)and Lithuania (LIU).

Before I was not understanding you need the countries poly to map to properties. It is not how the other geo-filters work.

It may not be a bad idea. And could be used by other geo-filter. But it probably better to make this in an other PR and keep this one more simple. Just a suggestion.

frodrigo commented 1 month ago

@tombay you can push -f your branch to update the same PR. No need to close it.

tombay commented 1 month ago

@frodrigo - Excuse the mess. I wanted to revert back to the start of my master - to just do the "trunk only" changes. I force pushed to the start of my fork and that closed the PR. I cannot re-open it as it is on Project-OSRM. A force-push to my latest change restores my fork but does not reopen the PR. When I have sorted out my code I will probably recreate my fork and hence a new PR. I had moved my code from my local branch (tombay-country-profiles) to my master branch (tombay:master) because automated testing did not seem to work on tombay-country-profiles.

tombay commented 1 month ago

I have reopened it %-) . I have linked this to the Feature Request #7038. I shall create a PR for issue #6710.

frodrigo commented 1 month ago

Never do a merge from remote on a PR branch, always use rebase to keep you RP branch clean and readable.

tombay commented 1 month ago

Sorty. I shall look into fixing this %-(.

tombay commented 1 month ago

In reviewing https://wiki.openstreetmap.org/wiki/OSM_tags_for_routing/Access_restrictions and just considering highway="trunk" and highway="trunk_link" there are only seven countries that do not support foot and bike access. (France, Belgium, Switzerland, Austria, Slovakia, Hungary and Denmark). Currently foot.lua and bicycle.lua exclude trunk and trunk_link. SO 1/ if uselocationtags = trunk is added to the profile then include trunk and trunk_link in the valid speed list. 2/ if osrm-extract argument --location-dependent-data data/notrunk.geojson is used then the countries can be excluded. The geojson file is only 50k.

The only issue is that if you want to build a dataset that includes a mixture of "trunk" and "notrunk" countries you must remember the location-dependent-data command option.

tombay commented 1 month ago

@frodrigo : I am removing all the extra data that I was writing to full support the access_restriction. there are so many commits I will clean it all up. So git reset --hard (to get to when I forked) git rebase ( to bring the fork to the latest from Project-OSRM:master) then create my branch OR delete and recreate my fork.

In either case this PR will close

frodrigo commented 1 month ago

delete and recreate my fork.

You just need a branch. But yes, you can reset you master.

In either case this PR will close

Yes better.