y20k / trackbook

Trackbook - Movement Recorder for Android
http://y20k.org/trackbook/
MIT License
106 stars 27 forks source link

Feature request: configurable update frequency, not fixed to 15s #96

Open voussoir opened 3 years ago

voussoir commented 3 years ago

Hi y20k. Thank you for this app. I have read through the other issues and your FAQ and I understand you want to keep configuration to a minimum. However I think an update frequency setting would be quite reasonable alongside the accuracy threshold setting.

For example, when I am walking and improving sidewalks / crossings on OSM I would like updates more frequently than 15s, preferably down to 1-2s, but when I am hiking I'd be satisfied with slower updates, like 60s if it helps preserve battery.

If you are open to the idea I can try to write a PR for it. I would just put another slider like the accuracy threshold setting. Thanks for your time.

Edit: It seems that getting fine-grained GPX tracks would also require changing / configuring DEFAULT_THRESHOLD_DISTANCE, which you may not wish to do. I didn't want to add two new options of course.

y20k commented 3 years ago

Hi @voussoir

Increasing the update interval would not result in less power consumption. Let me explain: Since GPS always takes a while to get a good fix on your location Trackbook requests location updates from the system continuously, as soon as you tap on the record button. It just does not store all of the received locations. Pausing location requests between saves would significantly reduce the quality of the location information Trackbook receives.

Anyway. For other purposes your feature idea might make more sense... Is Trackbook really a good tool for working on OSM (if the interval was 1s) - aren't there specialized tools for that? And are there any other use cases for that an adjustable interval would be a good idea?

If yes, I too think, that a slider 1s-1min would be a good addition. I am not against settings in general. They need a good justification though.

Hob111 commented 3 years ago

Hi @y20k

Your explanation puzzles me for the following reason.

I've found that when I run Trackbook alongside another tracking app Trackbook seems to record more track points per unit distance than it does when the other app isn't running. What you say above suggests this is pure imagination on my part. Can you think of any possible explanation for this or am I going nuts? :confused:

voussoir commented 3 years ago

@y20k Thanks for your remarks about the battery conservation idea. When I first read through the code I thought REQUEST_CURRENT_LOCATION_INTERVAL only applied when the map view was currently open, but I can see now that it registers a listener that will repeat in the background until the tracking is stopped. This code is located in MapFragment.kt instead of TrackerService.kt but I think I am understanding it correctly.

You're also right that there are other tools for editing OSM. OsmAnd can record tracks with configurable timing, and Vespucci can record tracks which I think are fixed at 1s. What can I say, I guess I just like Trackbook's UI more than the others for this purpose. OsmAnd is a heavy application with too many menus when all I want to do is make a gpx, and Vespucci doesn't show the travel time, km, and other stats like TrackBook does. My uses are walking and hiking so those are the only justifications I can offer myself.

@Hob111 I have not noticed this yet personally, but if you examine the gpx file in a text editor, you should find that the points are never less than 15 seconds apart. Trackbook also rejects points that are less than 15 meters apart. When you run Trackbook alone, are your points more than 15 seconds apart or much farther apart?

Hob111 commented 3 years ago

Thanks @voussoir

I hadn't thought of looking at the gpx files (god I'm getting old! :older_man:). I don't have a record of when I've run both apps together but if I check a few tracks or cross reference file dates I should come across a few. I'll get back in a few days. (unlike another thread, which I found tonight has been awaiting a response from me for months! :grin: ).

Hob111 commented 3 years ago

This seemed so simple. Life is never like that! To check the time interval for each pair of points is very slow and tedious unless I can find a way of parsing the file - I don't think my scripting is up to that any more. Checking the distance is even more difficult because each one has to be calculated from the latitude and longitude of two points.

It seems to me the simplest approach would be to calculate the average time and the average distance between points for two similar tracks, one with just Trackbook running and one with the second app running as well. All this is effectively doing is quantifying what I think I've observed. If the values are very similar then I've imagined it, which I suspect is the case.

Returning to the original proposal, is there anything magical about 15s or 15m? Unless conditions are unfavourable (eg tall buildings) GPS should reliably detect changes of less that 15m.

y20k commented 3 years ago

There is a reason for the 15 m. I introduced that quite a while ago, when I wanted to implement a pause detection.

Imagine taking a break on a hike. You pause for say 10 minutes and leave Trackbook running. During that time Trackbook continues to to save locations every 15 seconds. During that break Trackbook would record 40 waypoints. Due to the inaccuracy of some GPS chips that can add a significant distance to your recording.

That's why Trackbook only adds new waypoint if they are significantly different to the last recorded one.

voussoir commented 3 years ago

@y20k Yes I think this is a very reasonable feature. This is why I edited my original post after discovering that variable, because I don't want to change a whole bunch of values just to achieve my one goal.

I do notice however that Trackbook already includes if (averageAccuracy > Keys.DEFAULT_THRESHOLD_DISTANCE) {distanceThreshold = averageAccuracy} here. So unless the user's device has very fine accuracy, or the accuracy value is somehow incorrect, lowering the default shouldn't create phantom movement points during a rest. And if the device does have very fine accuracy, maybe it's ok to record small movements because they wouldn't be phantom points.

Under that light, would removing the default entirely, and only using distanceThreshold = averageAccuracy, accomplish the same goal?

y20k commented 3 years ago

I remember distinctly a situation where I was testing Trackbook. I was sitting outside of a nice cafe, clear sky, good GPS reception, while the app was recording. During that session quite a distance was recorded while I was just sitting in a chair in the sun. By the time I already had a function that demanded a minimum distance between two locations. But that somehow did not work. So I came up with a more complicated solution that takes the accuracy reported by the GPS into consideration. I am honestly not sure if the solution I found is the best. I barely understand it anymore. But for my purposes it works. It detects stops reliably. I am open to suggestions for good/better way to detect if a location is different enough from another.

Hob111 commented 3 years ago

I'm having problems getting my head round this (so what's new?).

@y20k > There is a reason for the 15 m. I introduced that quite a while ago, when I wanted to implement a pause detection.

Are you saying that when I pause Trackbook (which I think was an enhancement I requested/suggested) it carries on looking at the GPS location and only ignores it because of the 15m test you've implemented? ie It's not really paused but "idling" like a car engine ticking over? I ask this question in total ignorance but is it not possible simply to pause data collection and then restart when instructed? On a few occasions I've forgotten to restart Trackbook after a break and it doesn't seem to record another trackpoint until I do restart it, giving me a straight line between the pause point and the restart point, sometimes many 100s of metres apart. I cant reconcile this with your description above.

voussoir commented 3 years ago

@Hob111 He is referring to when you leave Trackbook recording but you physically stop walking. Trackbook uses the GPS accuracy and 15 meter rule to avoid recording a bunch of dots in the same place while you stand still.

Hob111 commented 3 years ago

@voussoir > Under that light, would removing the default entirely, and only using distanceThreshold = averageAccuracy, accomplish the same goal?

It seems to me that using the accuracy reported by a device (accuracy is actually based on statistical standard deviations, but that's another issue which I must respond to at some point!) to determine the minimum distance accepted by Trackbook would be a way of linking this to the quality of a gps chip.

Hob111 commented 3 years ago

@voussoir So when @y20k talks of a "pause detection" he didn't mean pausing the app but stopping briefly? Even more reason for using the accuracy reported by the device to determine if two locations differ statistically.

Hob111 commented 3 years ago

My final input for tonight. If the minimum distance is linked to the accuracy of the device is there justification for not having a fixed time interval between recorded points? So the "granularity" of a track would be determined by the quality of the GPS chip?

y20k commented 3 years ago

Not sure if you all already read it somewhere. Accuracy is a value that differs for every Location information that Trackbook receives. Here is an explanation of what this value means https://developer.android.com/reference/android/location/Location#getAccuracy() I probably interpreted this value not 100% correct. So if you feel you got an idea how to best factor that value in for the difference-detection, please share it. If possible already in some form of pseudo code. Plus: Thank you for the great discussion here and have a nice weekend.

Hob111 commented 3 years ago

Hope you both had a good weekend. I'm thinking through some pseudo code (sorry, don't do Java) which I'll try to post in the next day or two. It should be possible to do as I suggested above but I need to check a few things and refresh my memory :thought_balloon:.

voussoir commented 3 years ago

@y20k @Hob111 Thank you for the continued discussion. Here are my current thoughts about the isDifferentEnough function:

  1. Earlier, I suggested removing the DEFAULT_THRESHOLD_DISTANCE constant, but I was wrong. Google says "If this location does not have a horizontal accuracy, then 0.0 is returned", so the app needs some kind of fallback, so we should keep this default for that purpose.
  2. As long as the fix's accuracy is not 0, I think we should use the location.accuracy value directly instead of clipping it to DEFAULT_THRESHOLD_DISTANCE, because we should not artificially limit the user's fixes to 15m accuracy if their device is better than that.
  3. I think the line val averageAccuracy: Float = (previousLocation.accuracy + location.accuracy) / 2 should be removed and we should use location.accuracy directly in the following distance comparison. If the new fix has a worse accuracy than the previous one, lowering the value to the average gives it more leeway than it deserves. If the new fix has better accuracy, raising the value to the average means we are not taking full advantage of a good fix. So I think averaging is not a good idea in either case.

We define horizontal accuracy as the radius of 68% confidence.

68% is 1 standard deviation, which @Hob111 mentioned. Instead of using the 15m limit to prevent false points, we should instead manipulate location.accuracy to achieve higher or lower confidence.

Currently, the settings menu lets the user pick their accuracy threshold. But they probably don't realize that they're only getting 68% confidence in that number, and 32% of fixes are worse. If we want to help the user reject points outside of their accuracy threshold, we should multiply the location.accuracy to raise our confidence. For example, if location.accuracy * 2 < prefAccuracyThreshold gives us 95% confidence that we have satisfied the user's preference. Of course, if the user chooses a preference that's too low for their device, they might never get a fix with 95% confidence.

Finally, I wonder if we should not give the user a configurable waypoint timer. Since the background service is always requesting new fixes every 1s anyway, we could just do waypoints at the same 1s interval. The combination of user's accuracy threshold preference and the isBetterThan function will naturally regulate the pace at which new waypoints are actually added or not added. Therefore, we get finer and more accurate tracks, and we don't have to add the new configuration that I suggested.

To summarize:

  1. Manipulate location.accuracy to improve confidence that we are meeting the user's preferred accuracy threshold.
  2. Lock waypoint interval to 1s alongside the background location updater, and let the accuracy measurements control whether or not the point is recorded.

I'm sorry this comment got so long. I will try testing these changes in my own copy of the code soon.

Hob111 commented 3 years ago

@y20k @voussoir I'm enjoying this discussion and thanks to @voussoir for his last contribution. Some worthwhile points needing consideration and I concur with the summary.

  1. Manipulate location.accuracy to improve confidence that we are meeting the user's preferred accuracy threshold.
  2. Lock waypoint interval to 1s alongside the background location updater, and let the accuracy measurements control whether or not the point is recorded.

There is a way of achieving these using a simple statistical equation which will take into account the quality/accuracy of the data without too many assumptions, but before I post my suggestion I need to check with you guys that squaring a value and taking a square root is relatively straightforward in Java. I assume it is because I have a mathematical calculator on my phone which I imagine is written in Java.

Regarding the 15s fixed waypoint interval, this has the big disadvantage that the distance between waypoints becomes excessive if the user is travelling fairly quickly. A quick calculation suggests that a fit cyclist could easily cover over 200m in 15s and waypoints at such a separation will lead to "clipping" of bends and a large error in the total distance.

y20k commented 3 years ago

@Hob111 I think https://developer.android.com/reference/java/lang/Math provides methods to deal with square root and squaring. Floating point operations in general seemed to work fine for me so far.

Hob111 commented 3 years ago

The promised statistical approach.

If we have 2 values, A and B, each with an associated standard deviation, sigmaA and sigmaB, we can check if the positive difference between A and B, |A-B|, is significant using the standard deviation of the difference (sigma|A-B|) which is calculated as follows.

sigma|A-B| = ((sigmaA)^2 + (sigmaB)^2)^1/2

In words, the standard deviation of the difference is given by the square root of the sum of the squares of the standard deviations of A and B.

If |A-B| > sigma|A-B| then there is a 68.3% probability that A and B are different. If |A-B| > 2sigma|A-B| then there is a 95.5% probability that A and B are different. If |A-B| > 3sigma|A-B| then there is a 99.7% probability that A and B are different.

If squares and square roots don't present any computational problems this is relatively easy to compute, it can be split into 4 separate steps. The distancebetween the two waypoints equates to |A-B| and the previousLocation.accuracy and location.accuracy of each waypoint to sigmaA and sigmaB. [Note: I hope I'm picking the right names out of the code. I'm learning on the job!]

I think this meets the criteria laid out above by @voussoir and should produce tracks with the spacing between waypoints determined by the quality of the gps signal, the hardware, and the velocity of the user.

I don't think the 15s interval is needed or the user definable accuracy threshold but if this works we might need to consider what control is offered to the user. We do need something to deal with cases where the location.accuracy is returned as 0 (I don't understand why this should happen but it does). I'd suggest that if the location.accuracy is returned as 0 it's set to about 12m which is a conservative estimate based on the upper limit usually reported by my phone, a modest moto E5.

If someone is willing to produce an apk of a beta version I'd be very happy to test it. Any thoughts?

voussoir commented 3 years ago

@Hob111 Thank you for those formulas. This function should correspond to that:

fun isDifferentEnough(previousLocation: Location?, location: Location): Boolean {
    // check if previous location is (not) available
    if (previousLocation == null) return true
    // check if distance between is large enough
    val accuracy = if (location.accuracy != 0.0f) location.accuracy else Keys.DEFAULT_THRESHOLD_DISTANCE
    val previousAccuracy = if (previousLocation.accuracy != 0.0f) previousLocation.accuracy else Keys.DEFAULT_THRESHOLD_DISTANCE
    val accuracyDelta = Math.sqrt((accuracy.pow(2) + previousAccuracy.pow(2)).toDouble())
    val distance = calculateDistance(previousLocation, location)
    // location is different when far enough away from previous location
    return distance > accuracyDelta
}

Of course, the question is how much confidence do we want? Too much confidence will result in fewer points being recorded, and too low confidence will make false points.

I have made an apk, however I expect you would have to uninstall the official app in order to install my mod. If you don't want to lose your tracks you probably shouldn't install mine. trackbook-voussoir-20210310-1731.apk. It's raining outside so I haven't tested it yet!

@y20k How do you feel about this? Would you be interested in a pull request soon?

y20k commented 3 years ago

@voussoir :+1: A pull request would be great. I will create and share an APK signed with the key used for the Play Store version. No data will be lost when upgrading from the Play Store production version. Anyone using the F-Droid version should wait for an official release.

y20k commented 3 years ago

... in the meantime I will manually drop in the revised function from your comment and give it a test later on. :-)

voussoir commented 3 years ago

Great! Once again I did not test that new function yet, I was just following the formula.

By the way, the rest of the diff so far is

diff --git a/app/src/main/java/org/y20k/trackbook/Keys.kt b/app/src/main/java/org/y20k/trackbook/Keys.kt
index 30a3609..4ac8433 100644
--- a/app/src/main/java/org/y20k/trackbook/Keys.kt
+++ b/app/src/main/java/org/y20k/trackbook/Keys.kt
@@ -96,7 +96,7 @@ object Keys {
     const val ONE_HOUR_IN_MILLISECONDS: Int = 3600000
     const val EMPTY_STRING_RESOURCE: Int = 0
     const val REQUEST_CURRENT_LOCATION_INTERVAL: Long = 1000L                   // 1 second in milliseconds
-    const val ADD_WAYPOINT_TO_TRACK_INTERVAL: Long = 15000L                     // 15 seconds in milliseconds
+    const val ADD_WAYPOINT_TO_TRACK_INTERVAL: Long = 1000L                      // 1 second in milliseconds
     const val SIGNIFICANT_TIME_DIFFERENCE: Long = 120000L                       // 2 minutes in milliseconds
     const val STOP_OVER_THRESHOLD: Long = 300000L                               // 5 minutes in milliseconds
     const val IMPLAUSIBLE_TRACK_START_SPEED: Double = 250.0                     // 250 km/h
diff --git a/app/src/main/java/org/y20k/trackbook/helpers/LocationHelper.kt b/app/src/main/java/org/y20k/trackbook/helpers/LocationHelper.kt
index 3a6584a..ae5c60a 100644
--- a/app/src/main/java/org/y20k/trackbook/helpers/LocationHelper.kt
+++ b/app/src/main/java/org/y20k/trackbook/helpers/LocationHelper.kt
@@ -27,6 +27,7 @@ import androidx.core.content.ContextCompat
 import org.y20k.trackbook.Keys
 import org.y20k.trackbook.core.Track
 import java.util.*
+import kotlin.math.pow

 /*
@@ -148,7 +149,7 @@ object LocationHelper {
     fun isAccurateEnough(location: Location, locationAccuracyThreshold: Int): Boolean {
         val isAccurate: Boolean
         when (location.provider) {
-            LocationManager.GPS_PROVIDER -> isAccurate = location.accuracy < locationAccuracyThreshold
+            LocationManager.GPS_PROVIDER -> isAccurate = (location.accuracy * 2) < locationAccuracyThreshold
             else -> isAccurate = location.accuracy < locationAccuracyThreshold + 10 // a bit more relaxed when location comes from network provider
         }
         return isAccurate
@@ -180,16 +181,12 @@ object LocationHelper {
         // check if previous location is (not) available
         if (previousLocation == null) return true
         // check if distance between is large enough
-        val distanceThreshold: Float
-        val averageAccuracy: Float = (previousLocation.accuracy + location.accuracy) / 2
-        // increase the distance threshold if one or both locations are
-        if (averageAccuracy > Keys.DEFAULT_THRESHOLD_DISTANCE) {
-            distanceThreshold = averageAccuracy
-        } else {
-            distanceThreshold = Keys.DEFAULT_THRESHOLD_DISTANCE
-        }
+        val accuracy = if (location.accuracy != 0.0f) location.accuracy else Keys.DEFAULT_THRESHOLD_DISTANCE
+        val previousAccuracy = if (previousLocation.accuracy != 0.0f) previousLocation.accuracy else Keys.DEFAULT_THRESHOLD_DISTANCE
+        val accuracyDelta = Math.sqrt((accuracy.pow(2) + previousAccuracy.pow(2)).toDouble())
+        val distance = calculateDistance(previousLocation, location)
         // location is different when far enough away from previous location
-        return calculateDistance(previousLocation, location) > distanceThreshold
+        return distance > accuracyDelta
     }

diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml
index 2048088..134ee30 100644
--- a/app/src/main/res/values/strings.xml
+++ b/app/src/main/res/values/strings.xml
@@ -76,7 +76,7 @@
     <string name="pref_about_title">About</string>
     <string name="pref_app_version_summary">Version</string>
     <string name="pref_app_version_title">App Version</string>
-    <string name="pref_accuracy_threshold_summary">Discard location fixes with an accuracy larger than:</string>
+    <string name="pref_accuracy_threshold_summary">Discard location fixes with an accuracy larger than (meters):</string>
     <string name="pref_accuracy_threshold_title">Accuracy Threshold</string>
     <string name="pref_advanced_title">Advanced</string>
     <string name="pref_delete_non_starred_summary">Delete all recordings in \"Tracks\" that are not starred.</string>
Hob111 commented 3 years ago

@voussoir That was quick, I'll take your word that the code does what you say :confused:.

Regarding the confidence level to adopt, I have 2 apps on my phone which both give my location in different formats and both report a location error which I assume is derived in the same way as location.accuracy, The error is usually around 3 to 4m, rising to about 12m under unfavourable conditions, so we can do some "back of an envelope" calculations.

For the best case, assuming a brisk walking speed of 1.5 m/s (5.4km/h), and a location.accuracy of 3m then:

and the worse case, walking speed of 1.5 m/s (5.4km/h), and a location.accuracy of 12m:-

To accommodate the two extremes I'd suggest adopting the middle value as the default but allowing the user to choose the higher or lower value as a setting in place of the "Accuracy Threshhold" setting. No need for a detailed explanation. Simply set the default as 2 with the option to change it to 1 or 3 with the a comment like "1 - more waypoints, less accurate. 3 - fewer waypoints, more accurate" or something along those lines. What do you both think?

Hob111 commented 3 years ago

@y20k If I back up the trackbook data files to a safe location before uninstalling the current version and installing the apk produced by @voussoir will it be possible to copy them back without losing any data? I know I did that some years ago when you published an apk for testing but things may have changed since then. I should add I'm using the F-Droid version.

y20k commented 3 years ago

@Hob111 Yes. This is possible since the data structure is not changed. The relevant files are here: /Android/data/org.y20k.trackbook/files/

Hob111 commented 3 years ago

I have made an apk, however I expect you would have to uninstall the official app in order to install my mod. If you don't want to lose your tracks you probably shouldn't install mine. trackbook-voussoir-20210310-1731.apk. It's raining outside so I haven't tested it yet!

I can't install this. I get the usual Play Protect warning so I click "Install anyway" and the installation fails, almost as though the over-ride hasn't worked. Running Android 8. any ideas? I tried an apk for another app that I'd used in the past and it works.

voussoir commented 3 years ago

@Hob111 Sorry about the apk. I'm not very good at Android Studio, I learned just barely enough to make changes onto my device. I guess that apk isn't shareable.

Pull request is open

voussoir commented 3 years ago

@Hob111 I like your idea of replacing the accuracy threshold setting with a confidence level setting. Most people probably don't know the meter accuracy of their device, so your setting would make it easier for the user to pick their compromise.

However I did not include that in my pull request because I am inexperienced and don't want to remove existing settings from the app. Let's talk about that as a separate pull request idea with @y20k's input.

y20k commented 3 years ago

I compiled a test-APK using the normal / official key for signing. It is based on the code up to this commit: 7d690dc.

(If you are using the F-Droid version, you might need to uninstall the app first. Please make copy of the files directory first: /Android/data/org.y20k.trackbook/files/)

y20k commented 3 years ago

@voussoir Thank you for the pull request (#97). I am looking forward to testing it. My first test yesterday went well so far, I did not test idle detection yet though.

(1) We should keep in mind @Hob111's suggestion to implement a simple setting

Simply set the default as 2 with the option to change it to 1 or 3 with the a comment like "1 - more waypoints, less accurate. 3 - fewer waypoints, more accurate" or something along those lines.

Is this still desirable?

(2) We also should have a look - as you suggested - on the function isAccurateEnough

    fun isAccurateEnough(location: Location, locationAccuracyThreshold: Int): Boolean {
        val isAccurate: Boolean
        when (location.provider) {
            LocationManager.GPS_PROVIDER -> isAccurate = location.accuracy < locationAccuracyThreshold
            else -> isAccurate = location.accuracy < locationAccuracyThreshold + 10 // a bit more relaxed when location comes from network provider
        }
        return isAccurate
    }

This function is used to determine whether a location fix is added to a track or not. Users can set the locationAccuracyThreshold in Settings. I added it because my last phone had notoriously bad GPS reception and I needed a way to increase this threshold for me without making the app worse for others.

y20k commented 3 years ago

Sorry I am a bit overworked. Friday evening ...

like your idea of replacing the accuracy threshold setting with a confidence level setting.

I oversaw this comment. My opinion: Let's get rid of the Accuracy Threshold setting, if possible. It is very cryptic. But can we really? Sometimes the location fixes Trackbook gets are VERY bad. There still should be a way to drop them, I think. But that way does not need to be configurable.

Hob111 commented 3 years ago

@y20k Thanks for the apk, will download and install.

I'm delighted the statistical formula worked - Another triumph for theory :grin:. Now we have a situation where the waypoint separation is controlled by the quality of the device, the quality of the gps signal and the velocity of the user. Thank you both.

I feel the "accuracy threshold" setting is rather arbitrary and unnecessary. If the GPS on the phone is poor surely the accuracy reported by function GetAccuracy should deal with that when using the statistical formula, the waypoint will be rejected if the accuracy is too low. This suggest to me that function isAccurateEnough is redundant. Am I missing something or now @y20k latest post has just appeared are we in agreement?

If we allow the user to tweak the statistical formula by changing the multiplier used in the calculation of accuracyDelta (hope I've read the code correctly and got the name right), with 2 (95%) as the default but allowing any value between 1 and 3 (not necessarily integer) a user can then adjust the setting to suit their device and their requirements. A value is needed if function GetAccuracy returns 0 but that could be hard coded at a reasonable and not over optimistic value of 12 to 15m (say).

What I'm "in the dark about" is how the starting point is selected, not being able to read the code I need this explained to me.

Have a good weekend, no work for either of you I hope, I'm retired :smirk:

voussoir commented 3 years ago

@y20k Thank you for merging the PR and for your patience as this issue has progressed. If you are feeling overworked, do take a break. I'm not in a rush :)

@Hob111 We can't remove the isAccurateEnough function entirely. You said "the waypoint will be rejected if the accuracy is too low", but that's what this function is for. Although we can multiply location.accuracy to change our confidence, we still have to have a radius for our confidence circle. "I am 95% confident that my position is inside this ___ meter circle". The changes we made to isDifferentEnough do not substitute for this, especially on the first point of the track for example. So the function needs to stay and we need to have some number to put there.

I like the idea of using a confidence setting and letting the user pick the value of x in location.accuracy * x < threshold. Perhaps the user can select from x=1, 1.5, 2, 3.

However, I hesitate to use a fixed value for the threshold itself. If someone has a device capable of sub-5 meter accuracy for example, why should we limit them to only adjusting their confidence under a 15 meter threshold? Suppose someone wants to record points with 95% confidence under 5m? I think we should find a way to make the Accuracy Threshold setting easier to understand, but not remove it yet.

else -> isAccurate = location.accuracy < locationAccuracyThreshold + 10 // a bit more relaxed when location comes from network provider

I don't use network-assisted GPS myself, but perhaps instead of adding a constant +10 you can use another value of x to multiply the confidence in this case.

y20k commented 3 years ago

I will do some testing today. In the meantime I wrote up a short description on how Trackbook currently handles location recording. Hope that might help here.

Wiki page "How does Trackbook process location information?"

Hob111 commented 3 years ago

@y20k The apk installed with no problems and I've reinstated my data. I was also delighted to find a complete set of gpx tracks which I didn't know about. They'll allow me to share tracks which currently isn't possible through Trackbook.

In the meantime I wrote up a short description on how Trackbook currently handles location recording. Hope that might help here.

A brilliant idea. Thanks for finding the time to do this, with it's aid I should be able to find my way around a bit more easily.

@voussoir My suggestion about removing isAccurateEnough came from a misunderstanding of @y20k's explanation

This function is used to determine whether a location fix is added to a track or not. Users can set the locationAccuracyThreshold in Settings. I added it because my last phone had notoriously bad GPS reception ...

I interpreted this to mean that isAccurateEnough was added to overcome poor GPS, not, as I now realise, locationAccuracyThreshold. My apologies for the confusion and thanks for your explanation.

I like the idea of using a confidence setting and letting the user pick the value of x in location.accuracy * x < threshold. Perhaps the user can select from x=1, 1.5, 2, 3.

What do you have against 2.5? Although the % probability changes little between 2 and 3, 2.5 is no less important than the other values. If it is decided to go this way won't it be easier to set up a slider to change between 1 and 3 in .5 increments rather than missing out one value?

However, I hesitate to use a fixed value for the threshold itself.

I'm against any artificially imposed value if there's a better way, let the quality of the data speak for itself. My suggestion, above, of using 12-15m was to cope with the location.accuracy being returned as zero (which I don't understand, if the fix is obtained from a cluster of satellites there ought to be a computable error!). I think I need to see if I can figure out how this is dealt with at present with the aid of @y20k's description.

Finally, can someone humour me and explain why we're posting to a closed issue? It confused me at first when I couldn't find it.

y20k commented 3 years ago

Just some short feedback regarding my tests today,

I was recording two walks and a car ride using two phones. I had good weather conditions. The recorded accuracy was often around 5. (Trackbook will show that when long pressing on one of the blue waypoints.)

I was very pleased with the results. The recorded tracks did not contain any waypoints that were wrong/off, nor did they contain gaps. For my personal taste the waypoints seem to be a bit too close to each other. But that seems to be me over the years being accustomed to have waypoints a bit more spaced out in the UI of the app. Perhaps the app should start drawing a line instead of dots.

Another thought. Dialing the frequency down to 1 second with which Trackbook stores waypoints can probably result in performance problems. Just an example: Trackbook then can potentially store 10.800 waypoints in a three hour car ride. Trackbook stores tracks in JSON. I am not sure how the app will handle so much data.

That's all for today. More testing on my part in the upcoming week.

PS. I just took a look at one of the recorded track files. I copied its content into a JSON formatter to make it better readable. This track - a 90 minute walk - had ~4000 lines.

Hob111 commented 3 years ago

I'm delighted things seem to be working OK, I've not had the opportunity to test yet having missed Friday because I couldn't get the install to work. It doesn't surprise me that a lot a waypoints are recorded on good quality devices and I suspect file size could become a problem on long trips: - an option to increase the confidence level should help to deal with this but maybe experience will show a configurable time interval will also be needed?

I was going to ask one of you to help me locate the relevant code so I could try to follow the details @y20k had posted in the Wiki, However, I see he's now added links to the relevant sections, many thanks.

y20k commented 3 years ago

I today found a new feature in the osmdroid library: SimpleFastPointOverlay. The documentation states that it should be able to handle over 100.000 points. That leaves worries about the file size. Trackbooks currently saves a temp.json each time a new point is added - currently every second. I should probably increase that to every ten seconds.

Hob111 commented 3 years ago

@y20k

The documentation states that it should be able to handle over 100.000 points.

The next time I'm planning to walk for more than 27 hours I'll let you know :smile:

On a more serious note, I assume temp.json stores the current track before it's saved by the user. An upper time limit for a walk is unlikely to be more than 8 hours and using the back of my trusty envelope I make that 28.800 seconds. A more practical time limit would be 6 hours (excluding stops), or 21,600s. Do you have any feel for the file size that Trackbook / Android will support without it impinging on performance? I assume you're concerned about the number of records rather than the physical size in bytes.

Presumably saving the track every 9 seconds reduces the number of waypoints Trackbook records, thereby defeating the original aim set out by @voussoir. (Or have I misunderstood?) Would a sensible approach be to "test it to destruction" without any restrictions? If restrictions are needed for longer tracks maybe the time interval should be user configurable? (caveat emptor)

y20k commented 3 years ago

Presumably saving the track every 9 seconds reduces the number of waypoints Trackbook records, thereby defeating the original aim

@Hob111 Don't worry. Trackbook keeps ALL the points in memory. And it saves all waypoints to a track, when the users taps "Save". The temp file is just a backup in case the app is being killed. It too stores all waypoints. It is now updated every 9 seconds, instead of every second.

y20k commented 3 years ago

Just a note: some of the potential performance bottlenecks are removed. The temp-save interval is now longer and the app uses the SimpleFastPointOverlay. Again... I need to test this a bit before the changes can be rolled out.

But we are not in hurry, right? 😄

Issues like possible changes to the Settings screen need to be discussed. And we need to test the cool new isDifferentEnough a bit longer.

Hob111 commented 3 years ago

You've been busy, I wonder where you find the time. Presumably you have no concerns about storing up to 20,000 waypoints in memory. I'm looking forward to lockdown restrictions being relaxed a bit here so I can get out and do a few 12-15km walks to put the app through its paces.

When you retire nothing is a hurry :smile:

If you'd like to roll out the latest changes as an apk I'd be happy to test it. Thanks for all your hard work.

y20k commented 3 years ago

When you retire nothing is a hurry

Sounds great.

roll out the latest changes as an apk

Okay. Sure here is an updated test APK. It is based on the code up to this commit: 7ad12a2.

PS. As always: Be sure to keep a copy of your tracks somewhere safe :-)

Hob111 commented 3 years ago

Thanks for the apk, it's sensible that we should all be testing the same version of the code.

y20k commented 3 years ago

After we are done here, I am interested on what you two think on how the calculation of altitude changes can be improved. :smiley:

The elevation info Trackbook calculates is currently pretty unrealistic. The increased waypoint density resulting from the changes introduced by the cool new isDifferentEnough increases the unreliability of the current way that elevation is calculated.

Example: A hilly route in Stuttgart that I am often choosing for a run.

A. with the previous density of waypoints Elevation (uphill): 158m Elevation (downhill): -300m Highest Waypoint: 503m Lowest Waypoint: 354m

B. with the new improved density of waypoints Elevation (uphill): 461m Elevation (downhill): -575m Highest Waypoint: 506m Lowest Waypoint: 353m

So if any of you is interested, take a look at "Realistic Calculation of Elevation" (#99)

y20k commented 3 years ago

Hi @voussoir @Hob111

To accommodate the two extremes I'd suggest adopting the middle value as the default but allowing the user to choose the higher or lower value as a setting in place of the "Accuracy Threshhold" setting. No need for a detailed explanation. Simply set the default as 2 with the option to change it to 1 or 3 with the a comment like "1 - more waypoints, less accurate. 3 - fewer waypoints, more accurate" or something along those lines. What do you both think?

I added a Recording Accuracy setting. Since initial testing has shown (at least for me) there is no need to for more waypoints. I opted for a simple switch, that increases the required "different-ness" and will result in less waypoints recorded.

I have two questions:

1. What would be n correct and easy to understand wording of this setting? Title = "Recording Accuracy" Off (default) = "Default accuracy." On = High accuracy. Less waypoints are being recorded.

2. What do you think: How high should the accuracyMultiplier be for the setting "High"? Relevant code: https://github.com/y20k/trackbook/blob/3fa589e21c45edca51ff5ca9bed3b46f248122d0/app/src/main/java/org/y20k/trackbook/helpers/LocationHelper.kt#L180-L198

voussoir commented 3 years ago
  1. What would be a correct and easy to understand wording of this setting?

How about

  1. How high should the accuracyMultiplier be for the setting "High"?
multiplier %
1 68%
1.5 87%
2 95%

I think 2 would be worth a try, and if it feels way too sparse then reduce it to 1.5. If a typical Android device has ~10m accuracy, I think 1 point per 20 meters would be acceptable to a person who has chosen high accuracy mode.

voussoir commented 3 years ago

Wait, I just realized something. Should high accuracy mode also affect the isAccurateEnough calculations? For example, if you are in a situation with really poor GPS accuracy, you can eventually pass isDifferentEnough by getting far away -- but that doesn't mean your next waypoint will actually be more accurate.

The accuracyMultiplier is adjusting how far away you must get to register a new point, but not necessarily the accuracy of the new point. This was the effect of the old accuracy threshold slider, you could limit your track to only containing really accurate waypoints -- but if your device is not capable of getting below that threshold it would never record a point. I like the simplicity of the new switch but the behavior is different.

y20k commented 3 years ago

Hi @voussoir

I think there is still a need for the isAccurateEnough check. Some devices will sometime provide Trackbook with location fixes that are extremely inaccurate - with GPS, but especially if you use Network location. When I was "investigating" that more closely that happened for the first couple of locations you get, when you exit a building or tunnel, or when you try to record a train ride.

Anyway just from a practical viewpoint the current solution is already pretty good. I uploaded a test-built including a High Accuracy switch over at Issue 99.

I used this setting (Factor 2) on two runs ... And at least for me (my running trail & my device) the High setting provided enough waypoints. I am thinking of making it the default.