-
Notifications
You must be signed in to change notification settings - Fork 68
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Adapt voice hints and exporting to changes in BRouter 1.7.X #753
Merged
nrenner
merged 4 commits into
nrenner:master
from
rkflx:PR/adapt-voicehints-to-brouter-changes
Jul 22, 2023
Merged
Adapt voice hints and exporting to changes in BRouter 1.7.X #753
nrenner
merged 4 commits into
nrenner:master
from
rkflx:PR/adapt-voicehints-to-brouter-changes
Jul 22, 2023
Commits on Jul 5, 2023
-
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.
Configuration menu - View commit details
-
Copy full SHA for 3f241c9 - Browse repository at this point
Copy the full SHA 3f241c9View commit details
Commits on Jul 6, 2023
-
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.
Configuration menu - View commit details
-
Copy full SHA for eeb1c5b - Browse repository at this point
Copy the full SHA eeb1c5bView commit details
Commits on Jul 7, 2023
-
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.
Configuration menu - View commit details
-
Copy full SHA for 96016b7 - Browse repository at this point
Copy the full SHA 96016b7View commit details
Commits on Jul 8, 2023
-
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.
Configuration menu - View commit details
-
Copy full SHA for 8809fa9 - Browse repository at this point
Copy the full SHA 8809fa9View commit details
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.