Voice and Banner instructions for OSRM by ccritne · Pull Request #318 · Project-OSRM/osrm-text-instructions

ccritne · 2024-12-08T18:12:44Z

Issue

I want to use OSRM API Call in Maplibre-Navigation adding voice and banner instructions. I was inspired by Valhalla keys and I added them to RouteStep.

The algorithm is based on these properties that can be set up into options to personalize the output.

key	description
`verboseNavigation`	Allows you to add voice and banner instructions
`announcementsDistancesBelow500m`	Determines how often to receive instructions under 500 meter.
`announcementsWayMinimumDistance`	Determines the distance from maneauver location to include way name into instruction.
`distanceLastAnnouncement`	Determines when to receive last announcement.

Tasklist

- [x] Add voice and banner instructions
- [x] Add changelog entry 
- [x] Test with osrm-frontend (?)
- [] Review

Requirements / Relations

None

Copilot

Pull request overview

This PR adds an optional “verbose navigation” mode to generate voice and banner instructions from OSRM RouteStep data, along with new configuration options and documentation updates.

Changes:

Documented new compile() options for verbose navigation output.
Refactored instruction-string selection into a helper and added logic to generate step.voiceInstructions and step.bannerInstructions.
Added a changelog entry for the new functionality.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 10 comments.

File	Description
README.md	Documents new verbose-navigation options for `compile()`
index.js	Implements verbose navigation: voice + banner instruction generation and helper functions
CHANGELOG.md	Notes addition of voice and banner instructions

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-01T18:50:01Z

            return wayName;
        },
+        scaleOf: function(number) {
+            return Math.pow(10, parseInt(Math.log10(number)))


scaleOf will return NaN for number === 0 (because Math.log10(0) is -Infinity and parseInt(-Infinity) becomes NaN), which then propagates through the distance rounding logic when step.distance is 0. Consider guarding for non-positive/invalid inputs (e.g., return 1 for 0) and using Math.floor(Math.log10(number)) instead of parseInt for clarity.

Suggested change

return Math.pow(10, parseInt(Math.log10(number)))

if (typeof number !== 'number' || !isFinite(number) || number <= 0) {

return 1;

}

return Math.pow(10, Math.floor(Math.log10(number)));

Copilot · 2026-04-01T18:50:02Z

+            var value = Math.round(precision)*parseInt(parseFloat(distance)/parseFloat(precision))
+            return value


roundToIntPrecision is named/used like rounding, but it actually truncates via parseInt(distance/precision) (e.g., 3999 with precision 1000 becomes 3000). If you need true rounding to the nearest multiple, use precision * Math.round(distance / precision) (and avoid parseInt here).

Suggested change

var value = Math.round(precision)*parseInt(parseFloat(distance)/parseFloat(precision))

return value

var value = precision * Math.round(distance / precision);

return value;

Copilot · 2026-04-01T18:50:02Z

+         * @return {string}                                         Localized text instruction. 
+         * @return {void}                                           If verbose navigation is enabled  


The JSDoc now has two @return tags, and compile() conditionally returns undefined when verboseNavigation is enabled. This breaks the function’s return contract and will also violate the repo’s consistent-return ESLint rule. Consider always returning the compiled instruction string (and additionally mutating step when verbose mode is enabled), or change the API to return a structured object consistently.

Suggested change

* @return {string} Localized text instruction.

* @return {void} If verbose navigation is enabled

* @return {string|void} Localized text instruction, or void if verbose navigation is enabled.

Copilot · 2026-04-01T18:50:02Z

+            var instruction_one = this.tokenize(language, instruction, replaceTokens, options);
+
+            if (!options.verboseNavigation)
+                return instruction_one
+
+            var announcements = []


This new block introduces many missing semicolons (e.g., after return instruction_one, var announcements = [], etc.). The repo enforces semicolons via ESLint (.eslintrc.json sets "semi": "error"), so this will fail linting.

Copilot · 2026-04-01T18:50:03Z

+                if (thatDistance > distanceLastAnnouncement) {
+
+                    replaceTokens.distance = thatDistance
+                    announcement = this.tokenize(language, phrase, replaceTokens, options);
+                    distanceAlongGeometry = thatDistance                
+                } else {
+                    announcement = instruction_one
+                    distanceAlongGeometry = distanceLastAnnouncement
+                }


distanceAlongGeometry is assigned without being declared (var/let/const), which creates an implicit global and can cause cross-call contamination. Declare it as a local variable before assigning/using it inside the loop.

Copilot · 2026-04-01T18:50:03Z

+                announcements.push(
+                    {
+                        announcement: announcement,
+                        ssmlAnnouncement: `<speak>${announcement}</speak>`,
+                        distanceAlongGeometry: distanceAlongGeometry
+                    }


ssmlAnnouncement wraps the plain-text announcement in <speak>...</speak> without escaping XML entities. If announcement contains characters like &, <, or >, the generated SSML becomes invalid. Consider XML-escaping the content (or generating SSML via an SSML-safe builder) before embedding it.

Copilot · 2026-04-01T18:50:03Z

+
+            var distanceLastAnnouncement = options.distanceLastAnnouncement || 30.0 
+            if (distanceLastAnnouncement > 100.0)
+                throw new Error('Reduce last announcement distance');


The thrown error message Reduce last announcement distance is not very actionable. Consider including the actual constraint and value (e.g., “distanceLastAnnouncement must be <= 100m; got X”) so callers can correct options more easily.

Suggested change

throw new Error('Reduce last announcement distance');

throw new Error('distanceLastAnnouncement must be <= 100.0; got ' + distanceLastAnnouncement);

Copilot · 2026-04-01T18:50:03Z

+`announcementsDistancesBelow500m` | double | Determines how often to receive instructions under 500 meter (default: `150`) 
+`announcementsWayMinimumDistance` | double | Determines the distance from maneauver location to include way name into instruction (default: `250`) 
+`distanceLastAnnouncement` | double |Determines when to receive last announcement (default: `30`)


Spelling/grammar: “maneauver” should be “maneuver”. Also consider “under 500 meters” instead of “under 500 meter”.

Suggested change

`announcementsDistancesBelow500m` | double | Determines how often to receive instructions under 500 meter (default: `150`)

`announcementsWayMinimumDistance` | double | Determines the distance from maneauver location to include way name into instruction (default: `250`)

`distanceLastAnnouncement` | double |Determines when to receive last announcement (default: `30`)

`announcementsDistancesBelow500m` | double | Determines how often to receive instructions under 500 meters (default: `150`)

`announcementsWayMinimumDistance` | double | Determines the distance from maneuver location to include way name into instruction (default: `250`)

`distanceLastAnnouncement` | double | Determines when to receive last announcement (default: `30`)

Copilot · 2026-04-01T18:50:04Z

+`verboseNavigation` | boolean |Allows you to add voice and banner instructions (default: false)
+`announcementsDistancesBelow500m` | double | Determines how often to receive instructions under 500 meter (default: `150`) 
+`announcementsWayMinimumDistance` | double | Determines the distance from maneauver location to include way name into instruction (default: `250`) 
+`distanceLastAnnouncement` | double |Determines when to receive last announcement (default: `30`)


Minor formatting/grammar: missing spaces after | in the table descriptions (e.g., boolean |Allows / double |Determines). Keeping consistent spacing improves readability and matches the surrounding table style.

Suggested change

`verboseNavigation` | boolean |Allows you to add voice and banner instructions (default: false)

`announcementsDistancesBelow500m` | double | Determines how often to receive instructions under 500 meter (default: `150`)

`announcementsWayMinimumDistance` | double | Determines the distance from maneauver location to include way name into instruction (default: `250`)

`distanceLastAnnouncement` | double |Determines when to receive last announcement (default: `30`)

`verboseNavigation` | boolean | Allows you to add voice and banner instructions (default: false)

`announcementsDistancesBelow500m` | double | Determines how often to receive instructions under 500 meter (default: `150`)

`announcementsWayMinimumDistance` | double | Determines the distance from maneauver location to include way name into instruction (default: `250`)

`distanceLastAnnouncement` | double | Determines when to receive last announcement (default: `30`)

Copilot · 2026-04-01T18:50:04Z

+            if (!options.verboseNavigation)
+                return instruction_one
+
+            var announcements = []


Verbose navigation adds substantial new behavior (mutating step.bannerInstructions / step.voiceInstructions and generating announcements), but there are currently no tests covering compile() with opts.verboseNavigation: true (searching test/ shows no references). Consider adding unit/integration tests to validate the generated instructions structure and distances, and to lock down edge cases (e.g., step.distance 0, small distances, and custom option values).

ccritne added 3 commits December 8, 2024 19:08

Added voice and banner instructions

1209841

[docs] README.md updated

8718f6e

[log] CHANGELOG.md updated

e1e8c12

DennisOSRM requested a review from Copilot April 1, 2026 18:45

Copilot started reviewing on behalf of DennisOSRM April 1, 2026 18:46 View session

Copilot AI reviewed Apr 1, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Voice and Banner instructions for OSRM#318

Voice and Banner instructions for OSRM#318
ccritne wants to merge 3 commits into
Project-OSRM:masterfrom
ccritne:master

ccritne commented Dec 8, 2024

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Apr 1, 2026

Uh oh!

Copilot AI Apr 1, 2026

Uh oh!

Copilot AI Apr 1, 2026

Uh oh!

Copilot AI Apr 1, 2026

Uh oh!

Copilot AI Apr 1, 2026

Uh oh!

Copilot AI Apr 1, 2026

Uh oh!

Copilot AI Apr 1, 2026

Uh oh!

Copilot AI Apr 1, 2026

Uh oh!

Copilot AI Apr 1, 2026

Uh oh!

Copilot AI Apr 1, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

-            return Math.pow(10, parseInt(Math.log10(number)))
+            if (typeof number !== 'number' || !isFinite(number) || number <= 0) {
+                return 1;
+            }
+            return Math.pow(10, Math.floor(Math.log10(number)));

		var value = Math.round(precision)*parseInt(parseFloat(distance)/parseFloat(precision))
		return value

		* @return {string} Localized text instruction.
		* @return {void} If verbose navigation is enabled

	* @return {string} Localized text instruction.
	* @return {void} If verbose navigation is enabled
	* @return {string\|void} Localized text instruction, or void if verbose navigation is enabled.

	throw new Error('Reduce last announcement distance');
	throw new Error('distanceLastAnnouncement must be <= 100.0; got ' + distanceLastAnnouncement);

Conversation

ccritne commented Dec 8, 2024

Issue

Tasklist

Requirements / Relations

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants