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

Handle conditional access tags for date ranges #6984

Open ivarbrek opened 4 months ago

ivarbrek commented 4 months ago

Issue

OSRM currently don't support conditional access tags. This is a very useful feature, especially in the context of roads being closed due to construction work or events.

Several people have requested this feature (#4231, #4300, #5801, #6706)

This PR takes a simple and lightweight approach to this problem, and handles it through the profile libs.

Note that we here only handle conditional time ranges, and require them to span for more than a week, using the format as described in the OSM wiki, e.g. motor_vehicle:conditional=no @ 2018 May 22-2018 Oct 7.

Tasklist

Requirements / Relations

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

SiarheiFedartsou commented 4 months ago

Thanks @ivarbrek! Could you please also extend tests? Feel free to ask for a help if it is not clear what to update…

ivarbrek commented 4 months ago

I've tried to add a few test cases.

However, the tests seem to fail in the conan-linux-release step, and I'm struggling with proceeding from this point. I've downloaded the test logs, but don't find any clear direction from the logs of car/access.feature/.../309_car_conditional_restrictions.log. It's shutting down with SIGINT, but that's similar to all other log files I checked.

How do I proceed?

SiarheiFedartsou commented 4 months ago

@ivarbrek this test works locally, right? Try to bump cache key version here https://github.com/Project-OSRM/osrm-backend/blob/4930d2ef054542851729c4ea5f8b7a61de2e0789/.github/workflows/osrm-backend.yml#L404 Sometimes it helps (you need to change v4 to v5 in 2 places there)

ivarbrek commented 4 months ago

Finally got the tests running locally 😊 And think it should be good now! Who can I tag for review @SiarheiFedartsou?

DennisOSRM commented 4 months ago

Happy to help with the review

ivarbrek commented 3 months ago

I've looked into this a bit more now, and here's my summary/understanding of the current situation:

Some background I found out: Conditional turn restrictions are already implemented at C++ level in the contract step (see test here). A good thing about this implementation is that the osrm-contract command supports arguments to set a custom time for the parsing conditional turn restrictions.

Next steps Ideally we'd like the conditional access feature to be easily configurable, and here are the options as I see them:

What I don't like about the two latter options is that we duplicate the date configuration we already have in the contract step. Would be interested to hear your take on that and your suggested next steps @DennisOSRM and @SiarheiFedartsou.

SiarheiFedartsou commented 3 months ago

@ivarbrek having it in osrm-customize/osrm-contract is indeed more preferable. In practice you would need to re-run this logic many times per day for the same dataset and such kind of things are done with these tools in OSRM(e.g. live traffic or conditional turn restrictions you mentioned are implemented using it)

ivarbrek commented 2 months ago

Allright, so we are maybe just closing this then?