Blind user mode. #3694

grzezlo · 2022-08-04T11:34:56Z

Special optional mode implemented in Valhalla turn-by-turn, for use in Seeing Assistant applications from @transition-technologies:

Announcing crossed streets, the stairs, bridges, tunnels, gates and bollards, which need to be passed on route.
Information about traffic signals on crosswalks.
Route numbers not announced for named routes.
Added blind_user_mode switch in query parameters.

No idea how to write the tests, also where to announce this new feature in CHANGELOG.md (maybe under ADDED: Indoor routing near line 84?)

Added processing of crossing=traffic_signals in mjolnir/pbfgraphparser.c, so updated `taginfo.json' too.

Issue

fixes #2989

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

No special requirements.

Special optional mode implemented in Valhalla for use in Seeing Assistant applications from @transition-technologies: - Announcing crossed streets, the stairs, bridges, tunnels, gates and bollards, which need to be passed on route. - Information about traffic signals on crosswalks. - Route numbers not announced for named routes. - Added blind_user_mode switch in query parameters.

kevinkreiser · 2022-08-04T11:40:27Z

This is awesome I'll take a look at it more closely tonight most likely

kevinkreiser · 2023-03-29T14:25:33Z

@grzezlo would there be any chance you could update your branch to get the latest master merged in? i know its been a long time but i'm hoping you'd still be interested in merging this

grzezlo · 2023-03-31T17:38:54Z

No problem, @kevinkreiser, merged.

nilsnolde · 2023-12-02T22:34:37Z

I'm just looking at this again after a long time. Sorry for the wait @grzezlo I could fully understand if you moved on by now. I'd still be super interested to merge this and I know @kevinkreiser as well. So I'd help out on this if you'd prefer that.

One thing I'm wondering: how about including the OSM tag tactile_paving? It's used quite well it seems, > 3 Mio nodes & ways and could be used to prefer edges when blind_user_mode = true. Could also be done in a separate PR. Maybe that'd even be better. I'll spend some more time in the next weeks reviewing this work. Thanks again!

nilsnolde · 2023-12-03T20:26:24Z

proto/options.proto

@@ -482,4 +482,5 @@ message Options {
    uint32 matrix_locations = 54;                                  // a one to many or many to one time distance matrix. Does not affect
  }                                                                // sources_to_targets when either sources or targets has more than 1 location
                                                                   // or when CostMatrix is the selected matrix mode.
+  bool blind_user_mode = 81;                           // route: additional maneuvers and informations for blind users.


I was just looking at "vehicle types" and realized we already have smth you want in our request parameters, however, not documented. In PBF we have transport_type which is being set by either bicycle_type request param or type for all other modes, also pedestrian (not documented). Maybe we should use that instead of this boolean? Then we can make it a TravelType which will make more sense once we get to implement tactile_paving in costing. Let me know if you need some code pointers. What do you think @grzezlo ?

grzezlo · 2023-12-21T12:10:47Z

Thanks @nilsnolde for your comments and first of all for interest in the subject. Sorry for my late answer. In fact I moved to other projects, but Valhalla is still interesting for me and used as a routing engine in our app.
Regarding your idea: my implementation of the blindUserMode focused primarily on the narrative directions extension, as a next stage I thought about extending it to suggest some route planning optimisations, but I didn't manage it in given time.
But your idea opens exciting possibilities.
Preferring tactile pawing is one of them, other would be to prefer intersections with "traffic_signals:sound" or "traffic_signals:vibration" tags.
In some cases Valhalla plans route straight via the intersection, not giving precisely the crossings, although they are drawn on map: some workaround was to make our own customizations of "walkway_factor" and "sidewalk_factor" in query, but it affected some other cases which became non-optimal.
I thought about forcing the engine in blind user mode to always route via crossings on intersections if they're drawn, but didn't have enough understanding of the internal route planning mechanism to achieve it.

nilsnolde · 2023-12-21T15:26:04Z

Thanks for your good thoughts here @grzezlo ! It's again a while gone that I said I'll look into it more. It's far down the PR list by now, I guess it's not as much hitting my attention, I'll try to change that, I really want this functionality.

Is it ok for you if I commit to your branch instead of reviewing? I mean both in the end of course, but maybe that'd be more efficient?

kevinkreiser · 2023-12-21T15:34:09Z

i felt the same way about this but again didnt find anyy time. unless @grzezlo has time to also work on it, i would suggest you make a new branch from master and then merge @grzezlo's branch into that. that way you can let his branch unchanged and have all your work in a place you can more easily commit to

grzezlo · 2023-12-21T20:13:03Z

@nilsnolde modyfying this branch is ok for me, in such scenario all work on it will be in one place.

chrstnbwnkl · 2024-01-23T10:47:11Z

I have picked this up again, and apart from resolving merge conflicts, I did two things:

move the request option from /blind_user_mode to /costing_options/type, making use of an existing parameter
add some gurka tests

Having it as a costing option while not actually impacting the costing at all is a bit misleading maybe, but @nilsnolde pointed out that also modifying the costing could be a good addition to this in the future.

EDIT: had to fix some tests, astar_bikeshare was a particularly ugly one 😄 since no admins are built for the paris_bss_tiles, driving_side_right() was set to false by default, leading to higher stop impacts at right turns (assuming left side driving). Since this PR introduces parsing of crossing=traffic_signals tags, there are some edges which now have traffic_signal=true, which led to higher stop impacts at unexpected turns.

~~I assume the best way to go about this is to simply acknowledge missing admins for this test and all the weird behavior that comes with it for now.~~

~~Some matrix and isochrone tests were also affected by the added traffic signal tag, I adapted them to match the new results.~~

For reference, the failing isochrone test in the Utrecht tiles (blue = old result, yellow = new result, green points: nodes tagged with crossing=traffic_signals ). As expected, the contours are a bit smaller due to the increased stop impact.

Update: reversed parsing of nodes tagged crossing=traffic_signals, see comment below

kevinkreiser · 2024-02-05T13:57:36Z

src/odin/maneuversbuilder.cc

+    : options_(options), trip_path_(etp),
+      blind_mode_(options.costing_type() == Costing_Type_pedestrian &&
+                  options.costings().find(Costing::pedestrian)->second.options().transport_type() ==
+                      "blind") {


ok so to look a bit more like the other types we should:

over in pedestriancost.cc we parse out kFook and kWheelchair, we should add kBlind
then in triplegbuilder we call on a trip edge set_pedestrian_type which stores this type on the edges of the path so we'll need to update the proto with that blind value as well (trip edge is a proto struct)

should also add std::to_lower in costing for parsing to be a little more lenient

kevinkreiser · 2024-02-05T14:13:31Z

src/mjolnir/pbfgraphparser.cc

      } else if (tag.first == "highway") {
-        n.set_traffic_signal(tag.second == "traffic_signals");
+        n.set_traffic_signal(n.traffic_signal() || tag.second == "traffic_signals");


this has the potential to make the alogrithm think that we have to stop multiple times in a row in quick succession when a car traffic light and a cross walk light are next to each other. we might want to move pedestrian crossing to be a new node type or use the spare bit on node

kevinkreiser · 2024-02-05T14:17:31Z

src/odin/maneuversbuilder.cc

@@ -278,7 +281,33 @@ std::list<Maneuver> ManeuversBuilder::Produce() {
              std::string(" | left_similar_traversable_outbound =") +
              std::to_string(xedge_counts.left_similar_traversable_outbound));
 #endif
-
+    if (blind_mode_)


nit: can we add more braces here, i know i never ask for that but in this case wrap the switch

…ests again

chrstnbwnkl · 2024-02-07T13:53:13Z

valhalla/odin/narrative_dictionary.h

@@ -134,6 +144,7 @@ constexpr auto kCrossStreetNamesTag = "<CROSS_STREET_NAMES>";
 constexpr auto kRoundaboutExitStreetNamesTag = "<ROUNDABOUT_EXIT_STREET_NAMES>";
 constexpr auto kRoundaboutExitBeginStreetNamesTag = "<ROUNDABOUT_EXIT_BEGIN_STREET_NAMES>";
 constexpr auto kRampExitNumbersVisualTag = "<EXIT_NUMBERS>";
+constexpr auto kObjectLabelTag = "<object_label>";


chrstnbwnkl · 2024-02-09T08:58:10Z

After some review rounds now, the only big question remaining is how to enable announcements for passing pedestrian traffic signals without messing with stop_impact on DirectedEdge and without compromising backwards compatibility (as @gknisely pointed out, adding a new NodeType on TripLeg would lead to a runtime error).

…lind-mode-resolve-conflicts

kevinkreiser · 2024-02-12T15:26:03Z

is there anything left to do here? from my point of view we covered most of it and since we reverted the graphparser change we can merge now and come back to the whole crosswalk (traffic_signals) thing as a separate issue

chrstnbwnkl · 2024-02-12T16:21:28Z

From my side this is all set. In the last CI run, the tests I added failed only in the build release (and I wasn't able to reproduce it locally). Just merged master, so let's see if this was a hick up of some sort

chrstnbwnkl · 2024-02-12T17:09:25Z

Okay, seems like I have to have another look at those tests

kevinkreiser

thank you so much @grzezlo for startign this and @chrstnbwnkl for bringing it over the line!

mkandulavm · 2024-02-17T11:07:57Z

Hi

I am getting this exception when making a route call
Caught an exception: No such node (instructions.pass)

Any possibility above merged code requires changes for graph tile generation ?
I ask as the "instructions.pass" line in narrative_dictionary.h was recently committed and previously where I was getting a route response is now failing with exception after pulling latest from master :(

nilsnolde · 2024-02-17T12:58:53Z

from what I can see that key is in all 29 published locales, so your problem seems to be that you're again not really on master. updating locales for odin is done during building the library, which will write them from locales/*.json to a public header in ${CMAKE_BINARY_DIR}/odin/locales.h.

and as a general rule, unless we bump the major version, we never require new tile builds for any new commits..

nilsnolde · 2024-02-17T13:00:33Z

are you sure you're really re-compiling valhalla when you pull (vs just installing new headers..)?

mkandulavm · 2024-02-17T13:05:57Z

@nilsnolde thank you for the comment. I have a custom locale which is missing the "pass". I am not aware of this to be added and I now understand the meaning of the exception.
Thanks again ! My issue got fixed.

nilsnolde · 2024-02-17T13:14:43Z

Well we don’t expect people to bring their own locales tbh. I’d recommend to consider adding it to transifex. If it’s > 95% translated we’ll add it to the repo and this sort of thing can’t happen.

kevinkreiser requested review from kevinkreiser and dgearhart August 4, 2022 11:41

kevinkreiser added the enhancement label Aug 17, 2022

nilsnolde mentioned this pull request Oct 2, 2022

Open PRs summary #3775

Open

1ec5 mentioned this pull request Oct 5, 2022

Wiki says crossing=marked should not be used on ways? openstreetmap/iD#9303

Closed

Grzegorz Zlotowicz added 2 commits March 31, 2023 17:12

Merge branch 'master' into BlindUserMode.

f9c4cca

Merge branch 'master' into BlindUserMode.

98c9161

nilsnolde reviewed Dec 3, 2023

View reviewed changes

chrstnbwnkl added 4 commits January 22, 2024 19:10

Merge branch 'master' into cb-blind-mode-resolve-conflicts

c36eb17

tz submodule was not merged properly it seems

7a9ef7e

rolled submodules back

b99cf94

moved option to costing type, added test

b832779

chrstnbwnkl force-pushed the BlindUserMode branch from 66b5116 to b832779 Compare January 23, 2024 10:35

chrstnbwnkl added 7 commits January 24, 2024 16:49

fix node traffic_signals tag parsing

ab14287

format

a37e6eb

fixed multicue instructions

f0ae4ea

changelog

05d6456

text fixes

37daba7

isochrone test

805220c

matrix test

12de315

kevinkreiser reviewed Feb 5, 2024

View reviewed changes

chrstnbwnkl and others added 5 commits February 6, 2024 10:48

blind travel type parsed and passed more idiomatically

7731861

Merge branch 'master' into BlindUserMode

1a83ad8

reversed parsing of crossing=traffic_signal tags on nodes, adjusted t…

4a19798

…ests again

removed blind_mode_ members from narrativebuilder and maneuverbuilder

da328d4

get pedestrian type from correct trip edge/maneuver

b194938

chrstnbwnkl reviewed Feb 7, 2024

View reviewed changes

chrstnbwnkl and others added 3 commits February 9, 2024 09:38

maneuver combine test fix

9297498

comment

c7914a9

Merge branch 'master' into BlindUserMode

137c7a8

chrstnbwnkl added 2 commits February 9, 2024 10:11

fix locales

0764f8c

Merge branch 'BlindUserMode' of github.com:grzezlo/valhalla into cb-b…

8bb454a

…lind-mode-resolve-conflicts

Merge branch 'master' into BlindUserMode

c02cb7e

chrstnbwnkl and others added 2 commits February 13, 2024 13:06

forgot to add kBlind in enum to string function

98659bd

Merge branch 'master' into BlindUserMode

e31a1ec

kevinkreiser approved these changes Feb 14, 2024

View reviewed changes

kevinkreiser merged commit e7a5c16 into valhalla:master Feb 14, 2024
7 checks passed

grzezlo deleted the BlindUserMode branch February 15, 2024 09:48

chrstnbwnkl mentioned this pull request Feb 23, 2024

UTF8 Related protobuf error #4600

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Blind user mode. #3694

Blind user mode. #3694

grzezlo commented Aug 4, 2022 •

edited by chrstnbwnkl

kevinkreiser commented Aug 4, 2022

kevinkreiser commented Mar 29, 2023

grzezlo commented Mar 31, 2023

nilsnolde commented Dec 2, 2023 •

edited

nilsnolde Dec 3, 2023 •

edited

grzezlo commented Dec 21, 2023

nilsnolde commented Dec 21, 2023

kevinkreiser commented Dec 21, 2023

grzezlo commented Dec 21, 2023

chrstnbwnkl commented Jan 23, 2024 •

edited

kevinkreiser Feb 5, 2024

kevinkreiser Feb 5, 2024

kevinkreiser Feb 5, 2024

kevinkreiser Feb 5, 2024

chrstnbwnkl Feb 7, 2024

chrstnbwnkl commented Feb 9, 2024

kevinkreiser commented Feb 12, 2024

chrstnbwnkl commented Feb 12, 2024

chrstnbwnkl commented Feb 12, 2024

kevinkreiser left a comment

mkandulavm commented Feb 17, 2024 •

edited

nilsnolde commented Feb 17, 2024

nilsnolde commented Feb 17, 2024

mkandulavm commented Feb 17, 2024 •

edited

nilsnolde commented Feb 17, 2024

Blind user mode. #3694

Blind user mode. #3694

Conversation

grzezlo commented Aug 4, 2022 • edited by chrstnbwnkl

Issue

Tasklist

Requirements / Relations

kevinkreiser commented Aug 4, 2022

kevinkreiser commented Mar 29, 2023

grzezlo commented Mar 31, 2023

nilsnolde commented Dec 2, 2023 • edited

nilsnolde Dec 3, 2023 • edited

Choose a reason for hiding this comment

grzezlo commented Dec 21, 2023

nilsnolde commented Dec 21, 2023

kevinkreiser commented Dec 21, 2023

grzezlo commented Dec 21, 2023

chrstnbwnkl commented Jan 23, 2024 • edited

kevinkreiser Feb 5, 2024

Choose a reason for hiding this comment

kevinkreiser Feb 5, 2024

Choose a reason for hiding this comment

kevinkreiser Feb 5, 2024

Choose a reason for hiding this comment

kevinkreiser Feb 5, 2024

Choose a reason for hiding this comment

chrstnbwnkl Feb 7, 2024

Choose a reason for hiding this comment

chrstnbwnkl commented Feb 9, 2024

kevinkreiser commented Feb 12, 2024

chrstnbwnkl commented Feb 12, 2024

chrstnbwnkl commented Feb 12, 2024

kevinkreiser left a comment

Choose a reason for hiding this comment

mkandulavm commented Feb 17, 2024 • edited

nilsnolde commented Feb 17, 2024

nilsnolde commented Feb 17, 2024

mkandulavm commented Feb 17, 2024 • edited

nilsnolde commented Feb 17, 2024

grzezlo commented Aug 4, 2022 •

edited by chrstnbwnkl

nilsnolde commented Dec 2, 2023 •

edited

nilsnolde Dec 3, 2023 •

edited

chrstnbwnkl commented Jan 23, 2024 •

edited

mkandulavm commented Feb 17, 2024 •

edited

mkandulavm commented Feb 17, 2024 •

edited