Time dependent pedestrian routing

[Robot8A]

Pull Request Checklist

• [x] 1. I have rebased the latest version of the master branch into my feature branch and all conflicts have been resolved.
• [x] 2. I have added information about the change/addition to functionality to the CHANGELOG.md file under the [Unreleased] heading.
• [X] 3. I have documented my code using JDocs tags.
• [X] 4. I have removed unnecessary commented out code, imports and System.out.println statements.
• [X] 5. I have written JUnit tests for any new methods/classes and ensured that they pass.
• [ ] 6. I have created API tests for any new functionality exposed to the API.
• [ ] 7. If changes/additions are made to the ors-config.json file, I have added these to the ors config documentation along with a short description of what it is for, and documented this in the Pull Request (below).
• [x] 8. I have built graphs with my code of the Heidelberg.osm.gz file and run the api-tests with all test passing
• [ ] 9. I have referenced the Issue Number in the Pull Request (if the changes were from an issue).
• [ ] 10. For new features or changes involving building of graphs, I have tested on a larger dataset (at least Germany), and the graphs build without problems (i.e. no out-of-memory errors).
• [X] 11. For new features or changes involving the graphbuilding process (i.e. changing encoders, updating the importer etc.), I have generated longer distance routes for the affected profiles with different options (avoid features, max weight etc.) and compared these with the routes of the same parameters and start/end points generated from the current live ORS. If there are differences then the reasoning for these MUST be documented in the pull request.
• [X] 12. I have written in the Pull Request information about the changes made including their intended usage and why the change was needed.
• [ ] 13. For changes touching the API documentation, I have tested that the API playground renders correctly.

Fixes # .

Information about the changes

• Key functionality added: Extend time dependent routing to pedestrians. Changed FootFlagEncoder.java inspired by the changes done to CommonBikeFlagEncoder.java. Created testTimeDependent() in PedestrianFlagEncoderTest.java().
• Reason for change: Extend this feature to users that are not in vehicles, as streets may also change for them. Part of my Master's thesis work.

Examples and reasons for differences between live ORS routes, and those generated from this pull request

• Pedestrian routing is affected, example of where such thing happens: https://www.openstreetmap.org/way/399423763

Required changes to ors config (if applicable)

• Add |conditional_access=true to the necessary profiles

[Robot8A]

Comments about checklist:

1. I didn't manage to rebase, so I merged with the conflict errors dialog in GitHub website.
2. No new functionality in the API, just extended previous functionality to other profiles.
3. No new changes to config file, just extended previous functionality to other profiles.
4. Changes not from issue
5. Sadly my computer is not powerful enough to test that (I only have a laptop-tablet convertible with very limited specs). Would love if any of you could test it for me.

[aoles]

Thanks a lot for your contribution @Robot8A! I will get back to you with a review soon.

[takb]

Currently the CI test can't run due to a NPE in the wheelchair profile (probably because WheelchairFlagEncoder extends FootFlagEncoder). Either @Robot8A fixes in his fork or we need to pull this in a separate branch and fix the issue there.

[Robot8A]

Fixed!

[takb]

Thanks, we'll merge as soon as @aoles had time to review!

[Robot8A]

Thanks @aoles for the review.

Fixing testNonHighwayConditionallyClosed() makes ferries fail. A way of solving it would be to change (FootFlagEncoder.java)

        if (!acceptPotentially.canSkip()) {
            if (way.hasTag(restrictions, restrictedValues))
                return isRestrictedWayConditionallyPermitted(way, acceptPotentially);
            return isPermittedWayConditionallyRestricted(way);
        }

with

        if (!acceptPotentially.canSkip()) {
            if (way.hasTag(restrictions, restrictedValues))
                return isRestrictedWayConditionallyPermitted(way, acceptPotentially);
            return isPermittedWayConditionallyRestricted(way, acceptPotentially);
        }

However, that function does not exist. One could create it by changing AbstractFlagEncoder.java in the Graphhopper repo, by adding it like this:

    public EncodingManager.Access isPermittedWayConditionallyRestricted(ReaderWay way, EncodingManager.Access accept) {
        return getConditionalAccess(way, accept, false);
    }

Should I make a PR at the GIScience/graphhopper repo to create those changes? Or is it better to just add the changes in FootFlagEncoder as it implements AbstractFlagEncoder?

Best, Héctor

[Robot8A]

In the end I added it to this repo, with a TODO comment to add it later into graphhopper.

[aoles]

Dear Héctor,

Thanks a lot for your updates, this looks great!

Would you still mind moving the new method isPermittedWayConditionallyRestricted from ORSAbstractFlagEncoder to AbstractFlagEncoder as described in your TODO comment? I think the current stripped down implementation actually fails to flag the ways which are tagged with conditional access as identified by conditionalTagInspector.hasLazyEvaluatedConditions.

Looking forward to your corresponding PR in https://github.com/GIScience/graphhopper.

Cheers! Andrzej

[Robot8A]

https://github.com/GIScience/graphhopper/pull/53

[takb]

I just merged https://github.com/GIScience/graphhopper/pull/53, next update the references in openrouteservice/pom.xml to
v0.13.23

[aoles]

Dear Héctor,

could you please adapt the ORS code to use the new method introduced in https://github.com/GIScience/graphhopper/pull/53?

Cheers!

[Robot8A]

Done!

[aoles]

Awesome, thanks a lot for your prompt update! 👍

One last thing I've just noticed: as you have introduced the method FootFlagEncoder.setProperties, would you mind for the sake of being consistent to use it in HikingFlagEncoder as well?

Cheers!

[Robot8A]

I have added a new setProperties that includes a boolean to change the block_fords behaviour. Should I leave it how I have done it, take block fords completely from setProperties, or make the HikingFlagEncoder override the default later? The original FootFlagEncoder setProperties (with one parameter) was adapted from VehicleFlagEncoder. Best.

[aoles]

Thanks for getting back to me so quickly. I would suggest yet another solution, namely modify setProperties(PMap properties) method to call setProperties(properties, true). Cheers!

[Robot8A]

Sure, sounds right! Done.