OSRM Serializer: Adds voice instructions

[eikes]

Issue

There is a proprietary extension of the OSRM routing format used in the Mabox Navi SDK and MapLibre Navi SDK, both in iOS and Android. These SDKs work better when the voiceInstructions attribute is present in the steps.

This PR adds these voice instructions by using the existing verbal_pre_transition_instruction and verbal_post_transition_instruction in the maneuver.

Fixes #4482

Tasklist

• Add tests
• Add #fixes with the issue number that this PR addresses
• Update the docs with any new request parameters or changes to behavior described
• Update the changelog
• If you made changes to the lua files, update the taginfo too.

Requirements / Relations

[kevinkreiser]

looking excellent so far. i have to review it a bit more but i have broader questions:

1. verbal multi-cue for short successive maneuvers, we should have unit tests for that because i can imagine that could be tricky here
2. if i remember right, there are two verbal call outs before a maneuver if the maneuver is long enough, i couldnt figure out how you accomplished that. the first should say like "in Xm turn right onto Y street" and then the second sh ould say "turn onto Y street". i guess for now they both say "turn onto Y street" because we don thave the language support for the prepositional phrase "in Xm" right?

[kevinkreiser]

im sure after trying it out we'll want to tweak it but just having this even untweaked is a great place to start, thank you!

[kevinkreiser]

@nilsnolde we can ask contributors to merge master into thier branches and then merge when we see they have done that. that way we don't pollute the squash merge with other people's commits

[eikes]

looking excellent so far. i have to review it a bit more but i have broader questions:

Thank you!

1. verbal multi-cue for short successive maneuvers, we should have unit tests for that because i can imagine that could be tricky here

I am not entirely sure I understand what you mean. Can you give me an example for verbal multi-cue for short successive maneuvers?

2. if i remember right, there are two verbal call outs before a maneuver if the maneuver is long enough, i couldnt figure out how you accomplished that. the first should say like "in Xm turn right onto Y street" and then the second sh ould say "turn onto Y street". i guess for now they both say "turn onto Y street" because we don thave the language support for the prepositional phrase "in Xm" right?

I would love to see the prepositional phrase "in Xm". It would tie in nicely with the distance_along_geometry function and it would make the verbal instruction a lot more helpful. Alas, I don't know if I can somehow add it.

The way I approached this problem was to simply take the verbal instructions which are available in the default valhalla serializer and expose them in the OSRM serializer as well.

These are the verbal instructions available in the maneuver:

1. text_instruction
2. verbal_transition_alert_instruction
3. verbal_succinct_transition_instruction
4. verbal_pre_transition_instruction
5. verbal_post_transition_instruction

Examples:

  "instruction": "Turn left onto Main Street/Route 123. Continue on Route 123.",
  "verbal_transition_alert_instruction": "Turn left onto Main Street.",
  "verbal_succinct_transition_instruction": "Turn left.",
  "verbal_pre_transition_instruction": "Turn left onto Main Street, Route 123.",
  "verbal_post_transition_instruction": "Continue on Route 123 for 2 kilometers.",

The instruction isn't explicitly marked as a verbal instruction, but it would work. I don't know how it differs from other instructions. It is already exposed in the maneuver->instruction key within each OSRM format step.

From the other instructions I really wasn't sure which instructions would be the best to choose. My impression was that the verbal_pre_transition_instruction was the most reliable in terms of it actually being available. The verbal_post_transition_instruction made the most sense to add, since it compliments the other information nicely.

Maybe what you are saying is, that I should add the verbal_transition_alert_instruction only a couple of seconds before the end of the maneuver? If you don't mind, maybe in another PR?

[eikes]

Having just written that I went back to the docs and found this:

https://valhalla.github.io/valhalla/api/turn-by-turn/api-reference/#trip-legs-and-maneuvers

So the actual order should be:

1. verbal_transition_alert_instruction a certain distance before the end of the maneuver (using distance_along_geometry with 15 seconds)
2. verbal_pre_transition_instruction right before the end the maneuver (using distance_along_geometry with 3 (?) seconds)
3. Drive the actual maneuver
4. verbal_post_transition_instruction if the maneuver is long enough.

[eikes]

I changed the code to use both verbal_transition_alert_instruction and verbal_pre_transition_instruction in the correct order when they are available.

What do you think @kevinkreiser ? I would love to see this get merged. Is there anything else that I can do to move this forward?

Getting the distance into the verbal_transition_alert_instruction is an issue of its own, as it would affect the regular Valhalla serializer... That should happen in a separate PR

[kevinkreiser]

let me take a quick look tomorrow and then yeah we can merge

[kevinkreiser]

@eikes why has so much of the test been commented out? it kind of looks like maybe so you could focus on one test maybe and you forgot to uncomment the others? there is a way to do that when running with the tests. if this is what you are trying to do let me know

[eikes]

My bad! I was only running these tests. I should have checked again. Sorry! I‘ll fix it ASAP

[eikes]

@kevinkreiser The tests are back again, sorry. Yes I was trying to only run my specific tests. I am a bit ashamed to admit that I used the commenting method, because I was too lazy to figure out how to run individual tests. Now that it caused this lapse, I will not use that method anymore. I looked it up and I could and should have used this:

build/test/gurka/gurka_osrm_serializer --gtest_filter=*VoiceInstructions*

[kevinkreiser]

haha yep thats what i was going to tell you about thanks!

[eikes]

🥳