Skip to content
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
merged 4 commits into from
Jul 22, 2023
Merged

Adapt voice hints and exporting to changes in BRouter 1.7.X #753

merged 4 commits into from
Jul 22, 2023

Commits on Jul 5, 2023

  1. 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.
    rkflx committed Jul 5, 2023
    Configuration menu
    Copy the full SHA
    3f241c9 View commit details
    Browse the repository at this point in the history

Commits on Jul 6, 2023

  1. 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.
    rkflx committed Jul 6, 2023
    Configuration menu
    Copy the full SHA
    eeb1c5b View commit details
    Browse the repository at this point in the history

Commits on Jul 7, 2023

  1. 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.
    rkflx committed Jul 7, 2023
    Configuration menu
    Copy the full SHA
    96016b7 View commit details
    Browse the repository at this point in the history

Commits on Jul 8, 2023

  1. 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.
    rkflx committed Jul 8, 2023
    Configuration menu
    Copy the full SHA
    8809fa9 View commit details
    Browse the repository at this point in the history