Remove FlagEncoder

[easbar]

Fixes to a large extent #2463.

The FlagEncoder interface is gone. But what previously was an implementation of it, the now very simple VehicleEncodedValues class, still remains. All it does now is grouping a few encoded values together, like car_access+car_average_speed.

So basically there is no such thing as a 'flag encoder' anymore. We already moved everything that was related to 'vehicle-specific' OSM parsing to the VehicleTagParsers in #2561. But with this PR the EncodingManager also no longer offers methods like encodingManager.getEncoder("car"). It only 'manages' encoded values now. So the only thing we can fetch from the EncodingManager are encoded values. The only thing that still remains is that we can add a group of certain encoded values to the encoding manager collectively, i.e. there still is EncodingManager#add(VehicleEncodedValues.bike()) which, e.g. adds access, speed + priority encoded values for a bike. The reason I kept this here is that it corresponds to our config option graph.flag_encoders which defines multiple encoded values for each entry (and enables the corresponding vehicle parsers as well). I think we should also remove this, but since this requires some discussion about the new format better in a later PR. Actually I removed this pattern from almost all tests already and and it only remains on a high level / the GraphHopper class now.

The most important change here is that the fastest weighting (and others, also see here: 15426ef) no longer receives a FlagEncoder, but the encoded values directly. So for example this:

FlagEncoder encoder = encodingManager.getEncoder("car");
Weighting weighting = new FastestWeighting(encoder);

is now this:

BooleanEncodedValue accessEnc = encodingManager.getBooleanEncodedValue(VehicleAccess.key("car"));
DecimalEncodedValue speedEnc = encodingManager.getDecimalEncodedValue(VehicleSpeed.key("car"));
Weighting weighting = new FastestWeighting(accessEnc, speedEnc);

Importantly, the FastestWeighting also used encoder.getMaxSpeed() from the encoder, which we need to replace with speedEnc.getMaxValue(). This still remains todo before we can merge here. @karussell already did some work in this direction here: b3b3b11. Update: This was done in #2614, #2616, #2619 and the new method is now called: getMaxOrMaxStorableDecimal().

The other thing that is still missing are the private/destination penalties in FastestWeighting, which we also configured via the FlagEncoder so far. They should probably be replaced with config options for the weighting, i.e. weighting: fastest|private_penalty_factor=10, see the todo comments in FastestWeighting. Update: This is done in DefaultWeightingFactory now.

[easbar]

Either put it into the weighting string like weighting: fastest|private_factor=10 or into the profile like weighting: fastest, private_factor: 10. Or somehow map the vehicle/profile name to isMotorVehicle...

[easbar]

This seems to be the last open question for me here.

[easbar]

Maybe the least invasive thing we can do is use a list of vehicle names that we consider !isMotorVehicle like foot,hike,wheelchair,bike,mtb,racingbike,bike2? This is a bit ugly, because the list needs to be adjusted if someone adds a new 'vehicle', but this requires adjusting the flag encoder factor also, anyway and we can leave a comment there.

[easbar]

Seems reasonable to me: 97ca231

[otbutz]

Would it be possible to add a new vehicle without forking GraphHopper then? With the old FlagEncoder approach this was still doable.

[easbar]

Yes, absolutely. This is just about the private/destination penalty in FastestWeighting. And even that can be adjusted by overwriting the DefaultWeightingFactory. Let alone that the private/destination penalty should be a thing of the past as we have much more possibilities using CustomWeighting.

... then again if you have a certain use-case in mind it would also help if you tried this branch to make sure I did not mess this up somehow :)

[easbar]

You can even change the private/destination penalty using the hints in the profile definition, i.e. in config.yml, so without any java programming.

[karussell]

So finally. Really nice. When will you livestream the merge ;)

I think we should also remove this, but since this requires some discussion about the new format better in a later PR.

👍

BooleanEncodedValue accessEnc = encodingManager.getBooleanEncodedValue(EncodingManager.getKey("car", "access"));

Can you update your comment to use the VehicleAccess and maybe also mention this in the changelog?

[karussell]

Nice idea :)

[easbar]

Can you update your comment to use the VehicleAccess and maybe also mention this in the changelog?

👍 done :)

So finally. Really nice. When will you livestream the merge ;)

3, 2, 1... few years later, and its gone!