osmandapp / OsmAnd

OsmAnd
https://osmand.net
Other
4.6k stars 1.01k forks source link

There is no difference in the 'Voice prompts' arrival announcement #10386

Closed AnastasiaTiutina closed 3 years ago

AnastasiaTiutina commented 3 years ago

There is no difference in the 'Voice prompts' arrival announcement

Open Navigation Menu Choose from/to destination Add Waypoint between. you should see 'from /waypoint/ to' destination Click Options Choose navigation settings
Choose Voice prompts activate 'Track waypoints' Click Arrival announcement – Choose from the list Go back to the navigation menu Click start

screenshot-1607610626373 (1)

sonora commented 3 years ago

Please note the difference between 'track waypoints', which are waypoints embedded in a gpx track you may use for navigation, and 'intermediate destinations' which you specify on the route setup screen.

Having said that, can you please specify what the exact issue is: Are you saying the arrival prompt timing works as expected for the final but not for the intermediiate destination?

Wohli4711 commented 3 years ago

Auch mit den mir zugeschickten Einstellungen bleibt die App während der Navigation stumm. Habe heute eine andere App getestet - funktioniert einwandfrei

vshcherb commented 3 years ago

I think we have similar issue here https://github.com/osmandapp/OsmAnd/issues/10416

sonora commented 3 years ago

@Wohli4711 Could you please open a separate issue, because not hearing any sound for the entire app at all is an issue totally different from this one, and needs to be debugged separately.

@vshcherb I am rather certain I understand 10416 you reference, and have commented there. But I am still uncertain about this issue here: It seems to be about setting ARRIVAL_DISTANCE_FACTOR: I think the intended values are {1.5f, 1f, 0.5f, 0.25f}. But where is the code where we actually assign these values?

vshcherb commented 3 years ago

Yes we will double check with #10416 next week hopefully

sonora commented 3 years ago

To clarify, I find no ARRIVAL_DISTANCE_FACTOR.set statement in our code at all (other than the generic initialization to 1f), am I overlooking something?

sonora commented 3 years ago

Looking at our commit history, this has been broken for a long time with nobody reporting it.

The original intent of having a heuristic setting ARRIVAL_DISTANCE_FSCTOR is long obsolete.

I will implement the following unless anybody sees an issue:

  1. Remove setting ARRIVAL_DISTANCE_FACTOR
  2. Arrival announcements will follow the exact same distance/time/etalon trigger mechanism used for TURN_NOW i.e. depending on GPS_TOLERANCE, default speed, current speed
  3. Intermediate destinations will follow the same logic
  4. 10416: The announcement of waypoints, Favorites nearby and nearby POI will also follow the same logic however only for the distance component along the route (i.e. for their projection onto the route)

vshcherb commented 3 years ago
  1. I think ARRIVAL_DISTANCE_FACTOR is not obsolete it was removed by mistake. This factor should be used to announce all points and not turns, so we need to apply it only for Waypoitns, POI, Intermediate Points and Finish Destination.

I would concur to get it back working even though we've only noticed it. It's probably because of me trying to fix for blind people in the issue. I remember I didn't finish it cause I couldn't get it working precisely with 1-2 meters announcement https://github.com/osmandapp/OsmAnd/issues/8749

Am I missing something?

sonora commented 3 years ago

While approaching a destination, the perfect time for the arrival announcement is a small lead distance before the actual destination point. This lead distance is composed of the following factors: (1) The reaction distance, needed to listen to the prompt and then to react (2) The normal stopping distance (e.g. 40 m for a car moving at 50 km/h. Note that this is not the emergency stopping distance) (3) The uncertainty distance we unfortunately have to add to allow for the positioning error (or else some 'arrivals' may never be announced)

(1) and (2) are speed-dependent, (3) is related to the GPS hdop. In any case, there is no user-adjustable parameter here, we do not need a setting! Instead we should make the effort to find the perfect lead distance just based on the above physics (just like we do for turns). We need not be precise, we can approximate all 3 sub-effects. But if we do it right, this suffices for any use case.

Your issue with "precise announcements" is simply the mathematical limit of the above for speed -> 0 (walking speed will be close enough). In this case you are limited by having to account for the GPS hdop. If your device is e.g. 'guessing your position within 12 meters', you will naturally have a hard time announcing anything correctly within 1 meter. You can get the announcements right "on average", but there will always be a deviation (spread) of the order of your mean positioning uncertainty. And/or by applying a cut-off you run the risk of not announcing some events at all.

In other words: To announce something precisely depends on a precise enough estimate of where you are. A strategy for that could e.g. involve time-averaging a larger number of position measurements, but this is a different story from this issue.

vshcherb commented 3 years ago

@sonora There is a difference between react to the Turn and react to the Point. So Arrival announcement is specifically designed for points, where you will need to make decision much in advance:

So we definitely need to have that setting but probably we would better measure it in prior seconds (meters) - announce me these points +30 seconds in advance +60 seconds in advance to the normal time of any other turn point.

Regarding problem of 0m announcement : actually I wanted to achieve 1m announcement. Imagine we put speed 0.5m/s (which is probably normal speed for a vision impaired person) and we give 4 seconds, so it's 2 m.

vshcherb commented 3 years ago

I'm checking the code and trying to clean up a bit...

sonora commented 3 years ago

We can certainly base it all on the more abstract lead seconds, but only if it is a significant simplification. Essentially we already have:

(The latter is to be early enough for car drivers to still catch subsequent highway signs, e.g. freeway exit signs, typically placed at 1/2 mile, 1 km, 1/4 mile or 500 m out.)

Once we have this scheme decided, we can improve GPS_TOLERANCE constant by the actual hdop and gain a little in good reception circumstances for the 1m use case.

vshcherb commented 3 years ago

I just tested in real life and actually I'm surprised how it works. I think we need to make it obvious to the user what's expectation announce time and try to consolidate formulas in one place at least.

sonora commented 3 years ago

Yes, exactly. Creating a transparent ruleset is key to manage and finetune user expectation, that's why already in 2013 I had published my trigger table! :)

Moving the triggering and algorithms to one code place would be a good start before working on this. Our code is ok but far from transparent to the newcomer. (But it has worked pretty sufficienly for 10 years.)

And there are some more dimensions to understand like earliest and latest point to announce, i.e. when to drop an announcement, priorization of several pending announcements, correct sequencing, and the nasty GPS tolerance allowance.

vshcherb commented 3 years ago

So far I didn't do any change in algorithm except moving constants

I think arrival distance should be aligned somehow POI / GPX Waypoint / Other Alarms - but it requires another refactoring to involve announcement factor

sonora commented 3 years ago

Yes, now we're talking... ;)

Like I said: Arrival distance/time for all points must be one master algorithm, but of course with different parametrization per point type. (And for in-radius but off-route points we remove the orthogonal distance component.) And good to have it all in one helper.

We could expand my trigger table by a target scenario column and discuss the details there? http://docs.osmand.net/en/main@latest/development/algorithms/voice-prompt-triggering

vshcherb commented 3 years ago

Done - refactoring https://github.com/osmandapp/OsmAnd/commit/ee77be05b875d12df0faefe7402664c60cb3f782. Now it will be important to display numbers in UI and apply arrival factor to waypoints constants

sonora commented 3 years ago

Yes, I think you've done well.

Any objections if we use the following, before we start: I have renamed some states, also the misleading "GPS_TOLERANCE", added STATE_PNT_ARRIVAL we will need, and sorted by type and typical distance:

    private static final int POSITIONING_TOLERANCE = 12;
    public final static int STATE_TURN_NOW = 0;
    public final static int STATE_TURN_IN = 1;
    public final static int STATE_PREPARE_TURN = 2;
    public final static int STATE_LONG_PREPARE_TURN = 3;
    public final static int STATE_SHORT_ALARM_ANNOUNCE = 4;
    public final static int STATE_LONG_ALARM_ANNOUNCE = 5;
    public final static int STATE_PNT_ARRIVAL = 6;
    public final static int STATE_SHORT_PNT_APPROACH = 7;
    public final static int STATE_LONG_PNT_APPROACH = 8;
vshcherb commented 3 years ago

Yes, I will need to output it somewhere on UI and explain it here https://docs.osmand.net/en/main@latest/development/algorithms/voice-prompt-triggering

sonora commented 3 years ago

Current status and my proposed actions are now in the table.

Primarily I am still convinced we can kill the manual ARRIVAL_DISTANCE_FACTOR by simply applying a good rule factoring in not just the default_speed but also the current_speed. (What proves me right: See turns: There we have no manual user setting!)

Regarding the POSITIONING_TOLERANCE, I have added a comment explaining how it works. i.e. the tradeoff it makes. Once that is understood, we can see what could be improved towards something like a "1 m precision" prompting.

vshcherb commented 3 years ago

Default speed is generic setting and it shouldn't be only one setting to control announcement even if we always use SPEED*FACTOR. Reminder: default speed is used to calculate time for some segments with unknown speed etc

sonora commented 3 years ago

My wording was imprecise for brevity: Please read the second bullet under "Principles" in https://docs.osmand.net/en/main@latest/development/algorithms/voice-prompt-triggering, where I explain how the algorithm works. All we have left to do now is apply it to all cases.

vshcherb commented 3 years ago

https://github.com/osmandapp/osmand/commit/60c521aad5

vshcherb commented 3 years ago

image Added voice prompts times when development plugin is enabled

sonora commented 3 years ago

Yes, fine! But see my comments to the commit, I think there are still some issues ...

vshcherb commented 3 years ago

https://docs.osmand.net/en/main@latest/development/algorithms/voice-prompt-triggering#trigger-behavior - updated & reviewed docs. If you see something wrong please let me know

sonora commented 3 years ago

Checked and corrected, I think the table is now ok.

One point seems open: You had for "" Prepare to turn" "Skipped if speed < 10 km/h", but it seems this restriction is commented out in the code?

vshcherb commented 3 years ago

Still there

// if (DEFAULT_SPEED < 2.3) { - same as below if
if (PREPARE_DISTANCE_END - TURN_IN_DISTANCE < 150) {
    PREPARE_DISTANCE_END = PREPARE_DISTANCE * 2;
}
sonora commented 3 years ago

Ah, true, that yields the same restriction, but for 8 km/h. Will update the documentation.

Something else: My testing does not seem to speak any favorites or POIs (neither approach nor passing), possible issue there still.

vshcherb commented 3 years ago

keep it open so we can debug it later

sonora commented 3 years ago

Yes. Other than that: Do you want ro leave it as is, or do some fine-tuning now?

It haa run pretty well with few user complaints, but we could look at

vshcherb commented 3 years ago

I don't think to do any changes unless we have reports about it. It was not the goal of that refactoring to change actual behavior. So I think to keep it and handle with different issue. The only thing is to check how it works and describe it. I thin, In this issue we're going only refactor how the table looks inside the app.

In the issue https://github.com/osmandapp/OsmAnd/issues/10416 we're going to double check GPX / POI approaching distance

sonora commented 3 years ago

Ok, perfect! I also tend to be very conservative here because I had personally put many hundreds of testing hours into the system and its parametrization over the years and think is has served us well.

The 2 small tweaking ideas I currently have (alarm timing, and increase low speed precision in narrow settings like medieval towns by 20 meters) are not tested enough yet. Will make them separate stories when the time is right.

And also: Experimenting with the POSITIONING_TOLERANCE (for a high precisipn use case) is tricky and we can do separate.

dmpr0 commented 3 years ago

image img_help_announcement_time_day Снимок экрана 2021-01-11 в 17 43 32

sonora commented 3 years ago

I am tempted to suggest we run a survey or report how many of our users change that setting from its default at all? I would be very surprised if that was even just 1%, so spending development effort for refining its UI may have a strikingly poor return of invest?

sonora commented 3 years ago

@Chumva Vitaly, it appears that when you enter the UI, the screen does not correctly reflect the current status of the setting.I have e.g. setting 'Normal', but when I enter the new screen the slider is at 'Early.'

Chumva commented 3 years ago

@sonora It should be fixed here - https://github.com/osmandapp/OsmAnd/commit/bf398ef0cc407a4a3762f136ea16811233d557db.

sonora commented 3 years ago

Still not fixed as of just now...

sonora commented 3 years ago

Sorry, yes, fixed now - my mistake!

dmpr0 commented 3 years ago

Review

cepprice commented 3 years ago

PR: https://github.com/osmandapp/OsmAnd/pull/10651