From 3f241c918089b275505a0255c2cac4e01ef41b4c Mon Sep 17 00:00:00 2001 From: Henrik Fehlauer Date: Wed, 5 Jul 2023 06:26:07 +0000 Subject: [PATCH 1/4] Add BL and TLU to VoiceHints as contained in BRouter 1.7.2 abrensch/brouter@c9ae7c8681 added support for two new voice hints: A hint for beelines (`BL`), and a hint for 180 degree u-turns (`TU`). By adding support for both, we now know about all types of voice hints as defined in BRouter again. What makes things confusing is that the `TU` name for the respective `static final int` constant in BRouter's `VoiceHint.java` was repurposed for 180 degree u-turns, with left u-turns now being mapped to the new `TLU` constant name. Also note that originally the indexing of voice hints as used in BRouter's GeoJSON API has been changed as well due to inserting new commands in the middle of the numbering scheme instead of at the end. This API break has been fixed only in abrensch/brouter@82fecf9. Here we will rely on the fixed indexing, BRouter versions 1.7.0 and 1.7.1 without the re-indexing revert will not be supported. In addition, the voice hint mapping table has been checked to be identical to BRouter (this led to adding a missing `OFFR` symbol), and clarifying comments for planned future changes (e.g. changing the `TU` output token to `TLU` for OsmAnd) have been added. Note that beelines and 180 degree u-turns are only added to the mapping table for completeness. As BRouter-Web is handling straight lines on the client-side exclusively (which makes sense performance-wise when loading a route from a pasted URL with lots of them tracing an unmapped path), they are not expected to be in any GeoJSON response from BRouter, at least as of now. The same is true for 180 degree u-turn voice hints at cul-de-sac-style vias. If and when to emit voice hints for both cases in BRouter-Web itself is a different question, though it could likely also use the table for lookup. Test Plan: - `yarn test` - Confirm voice hints for routes with roundabouts and u-turns are unchanged. --- js/format/VoiceHints.js | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/js/format/VoiceHints.js b/js/format/VoiceHints.js index e842122..ac94081 100644 --- a/js/format/VoiceHints.js +++ b/js/format/VoiceHints.js @@ -116,6 +116,7 @@ // from BRouter btools.router.VoiceHint VoiceHints.commands = (function () { return { + // Command(name, locus, orux, symbol, fit, message) 1: new Command('C', 1, 1002, 'Straight', 'straight', 'straight'), 2: new Command('TL', 4, 1000, 'Left', 'left', 'left'), 3: new Command('TSLL', 3, 1017, 'TSLL', 'slight_left', 'slight left'), @@ -125,11 +126,19 @@ 7: new Command('TSHR', 8, 1018, 'TSHR', 'sharp_right', 'sharp right'), 8: new Command('KL', 9, 1015, 'TSLL', 'left_fork', 'keep left'), 9: new Command('KR', 10, 1014, 'TSLR', 'right_fork', 'keep right'), - 10: new Command('TU', 13, 1003, 'TU', 'u_turn', 'u-turn'), + // According to getCommandString() in BRouter, Command.name==TU for index TLU + // "should be changed to TLU when osmand uses new voice hint constants" + // According to getMessageString() in BRouter, Command.message==u-turn for index TLU + // "should be changed to u-turn-left when osmand uses new voice hint constants" + 10: new Command('TU', 13, 1003, 'TU', 'u_turn', 'u-turn'), // Left U-turn + // According to getMessageString() in BRouter, Command.message==u-turn for index TRU + // "should be changed to u-turn-right when osmand uses new voice hint constants" 11: new Command('TRU', 14, 1003, 'TU', 'u_turn', 'u-turn'), // Right U-turn - 12: new Command('OFFR', undefined, undefined, undefined, 'danger', undefined), // Off route + 12: new Command('OFFR', undefined, undefined, 'OFFR', 'danger', undefined), // Off route 13: new Command('RNDB', 26, 1008, 'RNDB', 'generic', 'Take exit '), // Roundabout 14: new Command('RNLB', 26, 1008, 'RNLB', 'generic', 'Take exit '), // Roundabout left + 15: new Command('TU', 12, 1003, 'TU', 'u_turn', 'u-turn'), // 180 degree u-turn + 16: new Command('BL', undefined, undefined, 'BL', 'danger', undefined), // Beeline }; })(); From eeb1c5bde3ce1ddece517ac22abbab84edef31da Mon Sep 17 00:00:00 2001 From: Henrik Fehlauer Date: Thu, 6 Jul 2023 09:46:47 +0000 Subject: [PATCH 2/4] Emit console warning when using an unsupported BRouter version abrensch/brouter@c9ae7c8681 changed indexing of voice hint ids, because some new hints were inserted in the middle instead of strictly at the end, changing the numbering of existing ids. For example, now id `12` was sent to indicate a right u-turn, while we still assume the old meaning of `12`, i.e. "off route". This clearly was an API break. This leads us to abort exporting with the Gpsies turn instructions style, since `OFFR` has an `undefined` symbol assigned, as well as emitting wrong voice hints for ids after `9`. Another unwelcome side effect is showing negative exit numbers for roundabouts. This breakage in the GeoJSON HTTP API has been shipping in BRouter 1.7.0 and 1.7.1 and finally got fixed with abrensch/brouter@82fecf9 contained in BRouter 1.7.2 or later. Earlier releases like 1.6.3 are also unaffected. To avoid emitting incorrect voice hints in BRouter-Web, running with broken versions of BRouter should be avoided. By checking the "Creator" field after receiving the first response from BRouter, we can now emit a warning if the version of BRouter used is unsupported. The warning mostly targets administrators and power users, i.e. those responsible for choosing the software versions used, and it is also only shown once per session. Note that the version check is compatible with the common "SemVer" scheme, so the check should continue working and even support more complex version compatibility scenarios as long as BRouter stays SemVer-compliant. Ref #751 Test Plan: - Run with BRouter 1.6.3 and 1.7.2, no warnings shown. - Run with BRouter 1.7.0 and 1.7.1, warnings shown only for the first segment. --- README.md | 2 ++ js/router/BRouter.js | 27 +++++++++++++++++++++++++++ package.json | 6 ++++++ yarn.lock | 5 +++++ 4 files changed, 40 insertions(+) diff --git a/README.md b/README.md index fe49bf0..987f37b 100644 --- a/README.md +++ b/README.md @@ -148,3 +148,5 @@ Copyright (c) 2018 Norbert Renner and [contributors](https://github.com/nrenner/ Copyright (c) 2012 davidshimjs [The MIT License](https://github.com/llyys/qrcodejs/blob/master/LICENSE) - [Bootstrap Icons](https://github.com/twbs/icons) Copyright (c) 2019-2023 The Bootstrap Authors [The MIT License](https://github.com/twbs/icons/blob/main/LICENSE) +- [compare-versions](https://github.com/omichelsen/compare-versions) + Copyright (c) 2015-2021 Ole Michelsen [The MIT License](https://github.com/omichelsen/compare-versions/blob/main/LICENSE) diff --git a/js/router/BRouter.js b/js/router/BRouter.js index 7b191f4..ccdf2be 100644 --- a/js/router/BRouter.js +++ b/js/router/BRouter.js @@ -10,6 +10,7 @@ L.BRouter = L.Class.extend({ GROUP_SEPARATOR: '|', ABORTED_ERROR: 'aborted', CUSTOM_PREFIX: 'custom_', + SUPPORTED_BROUTER_VERSIONS: '< 1.7.0 || >=1.7.2', // compatibility string should be in npm package versioning format isCustomProfile: function (profileName) { return profileName && profileName.substring(0, 7) === L.BRouter.CUSTOM_PREFIX; }, @@ -178,6 +179,7 @@ L.BRouter = L.Class.extend({ try { geojson = JSON.parse(xhr.responseText); layer = this._assignFeatures(L.geoJSON(geojson).getLayers()[0]); + this.checkBRouterVersion(layer.feature.properties.creator); return cb(null, layer); } catch (e) { @@ -189,6 +191,31 @@ L.BRouter = L.Class.extend({ } }, + versionCheckDone: false, + checkBRouterVersion: function (creator) { + if (this.versionCheckDone) { + return; + } + this.versionCheckDone = true; + + try { + const actualBRouterVersion = creator.replace(/^BRouter-/, ''); + if (!compareVersions.satisfies(actualBRouterVersion, L.BRouter.SUPPORTED_BROUTER_VERSIONS)) { + console.warn( + 'BRouter-Web ' + + BR.version + + ' requires BRouter versions ' + + L.BRouter.SUPPORTED_BROUTER_VERSIONS + + ', but only ' + + creator + + ' was found.' + ); + } + } catch (e) { + console.error(e); + } + }, + getRouteSegment: function (l1, l2, cb) { this.queue.push({ segment: [l1, l2] }, cb); }, diff --git a/package.json b/package.json index f61a047..2383ab6 100644 --- a/package.json +++ b/package.json @@ -49,6 +49,7 @@ "bootstrap-select": "1.13.18", "bootstrap-slider": "11.0.2", "codemirror": "5.65.13", + "compare-versions": "^6.0.0", "core-js-bundle": "3.29.1", "datatables": "1.10.18", "fit-file-writer": "tbsmark86/fit-file-writer#3eebe13", @@ -309,6 +310,11 @@ "index.js" ] }, + "compare-versions": { + "main": [ + "lib/umd/index.js" + ] + }, "@mapbox/maki": { "main": [ "icons/art-gallery.svg", diff --git a/yarn.lock b/yarn.lock index 564d8f6..13d0316 100644 --- a/yarn.lock +++ b/yarn.lock @@ -4197,6 +4197,11 @@ commander@^9.0.0: resolved "https://registry.yarnpkg.com/commander/-/commander-9.5.0.tgz#bc08d1eb5cedf7ccb797a96199d41c7bc3e60d30" integrity sha512-KRs7WVDKg86PWiuAqhDrAQnTXZKraVcCc6vFdL14qrZ/DcWwuRo7VoiYXalXO7S5GKpqYiVEwCbgFDfxNHKJBQ== +compare-versions@^6.0.0: + version "6.0.0" + resolved "https://registry.yarnpkg.com/compare-versions/-/compare-versions-6.0.0.tgz#a3edb527e4487bfab9a8b62ffe70cebc9b87675b" + integrity sha512-s2MzYxfRsE9f/ow8hjn7ysa7pod1xhHdQMsgiJtKx6XSNf4x2N1KG4fjrkUmXcP/e9Y2ZX4zB6sHIso0Lm6evQ== + component-emitter@^1.2.1: version "1.3.0" resolved "https://registry.yarnpkg.com/component-emitter/-/component-emitter-1.3.0.tgz#16e4070fba8ae29b679f2215853ee181ab2eabc0" From 96016b7fd0573719cc0793d036ac771e60bebd0f Mon Sep 17 00:00:00 2001 From: Henrik Fehlauer Date: Fri, 7 Jul 2023 16:44:21 +0000 Subject: [PATCH 3/4] Document new export formats added in BRouter 1.7.0 and fix export error BRouter 1.7.0 implemented support for three new export formats: "Cruiser", "BRouter internal" and "Locus(-new)". "Cruiser" (`turnInstructionMode=8`) and "BRouter internal" (`turnInstructionMode=9`) are not yet exposed in BRouter-Web's UI through profiles, so we do not need to implement them at the moment. Here we only document them by making them explicit unimplemented `cases` in the code. In addition, BRouter changed "locus-style" with `turnInstructionMode=2` to emit a different format for newer releases of Locus, while the old format is now referred to as "locus-old-style" from profiles with `turnInstructionMode=7`. Since BRouter-Web does not know yet about the the new id, exports will fail with "unhandled turnInstructionMode" errors. To fix the latter issue, we now map `turnInstructionMode=7` to the newly renamed `LocusOldVoiceHints()`. Note that `turnInstructionMode=2` is also currently using `LocusOldVoiceHints()`, i.e. the new format still needs an implementation. Test Plan: - `yarn test` - Check choosing "locus-old-style" now exports without an error. --- js/format/VoiceHints.js | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/js/format/VoiceHints.js b/js/format/VoiceHints.js index ac94081..0910af2 100644 --- a/js/format/VoiceHints.js +++ b/js/format/VoiceHints.js @@ -203,7 +203,7 @@ } } - class LocusVoiceHints extends WaypointVoiceHints { + class LocusOldVoiceHints extends WaypointVoiceHints { _addToTransform(transform) { transform.gpx = function (gpx, features) { // hack to insert attribute after the other `xmlns`s @@ -364,7 +364,10 @@ BR.voiceHints = function (geoJson, turnInstructionMode, transportMode) { switch (turnInstructionMode) { case 2: - return new LocusVoiceHints(geoJson, turnInstructionMode, transportMode); + // TODO: + // Use locus-old-style voice hints for now (same style as returned by BRouter 1.6.3 + // for turnInstructionMode=2), implementation for new-style locus still missing. + return new LocusOldVoiceHints(geoJson, turnInstructionMode, transportMode); case 3: return new OsmAndVoiceHints(geoJson, turnInstructionMode, transportMode); case 4: @@ -373,6 +376,10 @@ return new GpsiesVoiceHints(geoJson, turnInstructionMode, transportMode); case 6: return new OruxVoiceHints(geoJson, turnInstructionMode, transportMode); + case 7: + return new LocusOldVoiceHints(geoJson, turnInstructionMode, transportMode); + case 8: // Cruiser export, not exposed in the web UI through profiles yet + case 9: // BRouter internal export, not exposed in the web UI through profiles yet default: console.error('unhandled turnInstructionMode: ' + turnInstructionMode); return new VoiceHints(geoJson, turnInstructionMode, transportMode); From 8809fa94cf11fa011271a531fb9842b035210b7e Mon Sep 17 00:00:00 2001 From: Henrik Fehlauer Date: Sat, 8 Jul 2023 09:50:13 +0000 Subject: [PATCH 4/4] Prevent Gpsies-style export from throwing for undefined symbols While abrensch/brouter@82fecf9 fixed the export already by avoiding the problematic `OFFR` voice hint, we would still throw in case we encountered an `undefined` symbol entry: > Uncaught TypeError: Cannot read properties of undefined (reading > 'toLowerCase') This can be prevented by not calling `toLowerCase()` on `undefined` objects. Fixes #751 Test Plan: - Change `VoiceHints.commands` to contain `undefined` entries for a particular voice hint. - Create a route with that voice hint. - Test that exporting for each `turnInstructionMode` does not throw. --- js/format/VoiceHints.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/js/format/VoiceHints.js b/js/format/VoiceHints.js index 0910af2..bcc748d 100644 --- a/js/format/VoiceHints.js +++ b/js/format/VoiceHints.js @@ -179,7 +179,7 @@ class GpsiesVoiceHints extends WaypointVoiceHints { _getWpt(hint, cmd, coord) { - return { name: cmd.message, sym: cmd.symbol.toLowerCase(), type: cmd.symbol }; + return { name: cmd.message, sym: cmd.symbol?.toLowerCase(), type: cmd.symbol }; } }