transistorsoft / cordova-background-geolocation-lt

The most sophisticated background location-tracking & geofencing module with battery-conscious motion-detection intelligence for iOS and Android.
http://www.transistorsoft.com/shop/products/cordova-background-geolocation
Other
655 stars 277 forks source link

IOS Iphone SE - Crash on stopWatchPosition every 5km during tracking #280

Closed jbfrequence closed 7 years ago

jbfrequence commented 7 years ago

Hi,

Environment

Expected Behavior

My ios runners should not expect any crash and resume of their smartphone during their 15km or more jogger runs

Actual Behavior

Stack trace (source : crashlytics) 3 crashs like this (one every 5 km) Crashed: com.apple.root.default-qos 0 libsystem_kernel.dylib 0x249cec5c pthread_kill + 8 1 libsystem_pthread.dylib 0x24a78733 pthread_kill + 62 2 libsystem_c.dylib 0x249630ad abort + 108 3 libsystem_malloc.dylib 0x249f8ef7 free + 438 4 libobjc.A.dylib 0x244e7e09 object_dispose + 20 5 CoreFoundation 0x24c5bdc7 -[NSDictionaryM dealloc] + 190 6 libobjc.A.dylib 0x244f8f67 objc_object::sidetable_release(bool) + 150 7 CoreFoundation 0x24d36523 -[__NSArrayM removeAllObjects] + 266 8 FREQUENCE 0x550a33 -[TSLocationManager stopWatchPosition] (TSLocationManager.m:812) 9 libdispatch.dylib 0x248b1823 _dispatch_call_block_and_release + 10 10 libdispatch.dylib 0x248c05e9 _dispatch_root_queue_drain + 1560 11 libdispatch.dylib 0x248bffcd _dispatch_worker_thread3 + 96 12 libsystem_pthread.dylib 0x24a75b29 _pthread_wqthread + 1024 13 libsystem_pthread.dylib 0x24a75718 start_wqthread + 8

Thanks for any helps,

JB

jbfrequence commented 7 years ago

I've got also a crash like this on IPhone 6S and 7 (IOS >= 10) related to watchPosition function

Crashed: com.apple.root.default-qos 0 libobjc.A.dylib 0x18116af70 objc_msgSend + 16 1 FREQUENCE 0x10062c088 -[TSLocationManager watchPosition:success:failure:] (TSLocationManager.m:807) 2 FREQUENCE 0x10020de8c __42-[CDVBackgroundGeolocation watchPosition:]_block_invoke (CDVBackgroundGeolocation.m:459) 3 libdispatch.dylib 0x1815aa1fc _dispatch_call_block_and_release + 24 4 libdispatch.dylib 0x1815aa1bc _dispatch_client_callout + 16 5 libdispatch.dylib 0x1815b8a4c _dispatch_queue_override_invoke + 732 6 libdispatch.dylib 0x1815ba34c _dispatch_root_queue_drain + 572 7 libdispatch.dylib 0x1815ba0ac _dispatch_worker_thread3 + 124 8 libsystem_pthread.dylib 0x1817b32a0 _pthread_wqthread + 1288 9 libsystem_pthread.dylib 0x1817b2d8c start_wqthread + 4

christocracy commented 7 years ago

I'll look into the crashes but are you 100% sure you need to use #watchPosition?

The plugin will track a moving device without using #watchPosition. #watchPosition is not the primary tracking mechanism; it's a relatively new addition and generally not required. the plugin existed for nearly its first 3 years without it.

Do you understand the plugin's Philosophy of Operation?

jbfrequence commented 7 years ago

Hi,

I used this functions since I already used a similar approach when I was using the cordova-geolocation-plugin before refactoring my strategy with your plugin. Yes, I already read your plugin documentation (2 or 3 times ;-) and I think I have the same objective : get the most accurate precision with the less battery consumption. I need to get the first accurate position (<10m) with a timeout of 10 seconds. I also skip the first position reterieved since sometimes on android devices, I observed that the first returned position is a cached value. If no accurate position (or no position at all) are retrieved during this period, I call a fallback method to get an extrapolation of distance / speed using core motion data and runner training data.

christocracy commented 7 years ago

You need to forget what you know based upon cordova-plugin-geolocation.

When the plugin does motionchange, it requests 5 samples, selecting the most accurate before firing the motionchange event. You'll see all these "samples" in the location event where location.sample === true.

bgGeo.on('location', function(location, taskId) {
  if (location.sample) {
    console.log('- Sample: ', location);
  }
});

#getCurrentPosition requests 3 samples by default (configurable)

bgGeo.getCurrentPosition(callback, failure, {
  samples: 10,  // <-- 10 samples!!!
  desiredAccuracy: 10  // <-- keep sampling until a location having accuracy <= desiredAccuracy
});

In the above, if the first location that arrives has `accuracy == 10`, the function will stop sampling and immediately return that first location.
christocracy commented 7 years ago

In the XCode logs, you'll see these samples like this:

╔═══════════════════════════════════════════════════════════
║ -[LocationManager locationManager:didUpdateLocations:] Sample 1 of 3
╚═══════════════════════════════════════════════════════════
jbfrequence commented 7 years ago

With calling getCurrentPosition as suggested, how can we ignore the first sample (even if it is accurate) ? On android devices. as I said, I need to ignore the first sample position returned

christocracy commented 7 years ago

The plugin already ignores samples. It does not persist samples to its database.

Only the final sample is provided to #getCurrentPosition.

bgGeo.on('location', function(location, taskId) {
  if (location.sample) {
    console.log('- Sample');
  } else {
    console.log('- Final');
  }
});
bgGeo.getCurrentPosition(function(location, taskId) {
  console.log('- getCurrentPosition: ', location);
  bgGeo.finish(taskId);
});

In logs, you'd see:

>- Sample
>- Sample
>- Final
>- getCurrentPosition
jbfrequence commented 7 years ago

Ok you said "In the above, if the first location that arrives has accuracy == 10, the function will stop sampling and immediately return that first location." So if the first sample has accuracy == 10, it will return it right ? I don't care about database persistance (I don't use it) , I only need that the first sample (even accurate) MUST be skipped

christocracy commented 7 years ago

if you want to "skip" these locations, just ignore it.

if (location.sample) {
  console.log('- Skip');
}
christocracy commented 7 years ago

The callback to #getCurrentPosition is called only once, with the final location. It never receives samples.

christocracy commented 7 years ago

You'll hear samples with the debug sound tick.....tick.....tick

jbfrequence commented 7 years ago

OK I understand that I cannot use the function bgGeo.getCurrentPosition alone to skip the final samples that occur first. Using watchPosition & stopPosition was more convenient to handle this particular case. I'd rather wait for the stopPosition crash fix resolution

christocracy commented 7 years ago

What is your use-case that you think you need to use #watchPosition instead of the regular tracking mechanism recording a location each distanceFilter meters?

#watchPosition was designed for use in the foreground, when you might want to show rapid updates to the user's current position on the map. It wasn't designed for long-term use in the background.

jbfrequence commented 7 years ago

My use-case is about tracking (cross foreground/background) the current runner distance & speed as accurate and fast as I can with the less battery consumption. I don't need to show any map at real time I'm just dealing with distance and temporal concerns (speed ..). If the getCurrentPosition was ignoring the "first" final location when it occurs first, I would use it

christocracy commented 7 years ago

You do not need #watchPosition at all. Stop using it.

You just need the standard tracking mechanism. Execute changePace(true) to "start tracking" immediately. Set your distanceFilter as desired. Execute changePace(false) to stop tracking.

If you really want to cut down on battery power, do not attempt to send each location to your server. HTTP requests consume far more energy / s than GPS.

jbfrequence commented 7 years ago

About changePace -> Yes i used it like you said (start->true, stop->false) I don't send any HTTPS request to server. All what I do is computing distance and speed internally every time I got an accurate position. In case, I have no accurate position after 10 seconds, I should call a fallback function. That's why I used watchPosition & stopPosition for convenience. But why exposing this API if it is only foreground compliant ?

christocracy commented 7 years ago

it's not only foreground compliant, nothing in the plugin is only so. It was designed with foreground use in mind. It wasn't designed for long-term tracking. I imagined it being turned on in the Cordova pause event and off on pause.

Also, you'll get far better accuracy using the regular tracking mechanism since it keeps the GPS radio on constantly.

#watchPosition toggles the GPS radio on/off based upon a timer configured to your interval.

You should forget about using #watchPosition in your case. I don't recommend it.

jbfrequence commented 7 years ago

So as a conclusion, do you recommand me to use getCurrentPosition with "on" function to ignore some location I decided to skipp ? Is the getCurrentPosition() toggling the GPS on/off everytime it is called/finished ?

christocracy commented 7 years ago

So as a conclusion, do you recommand me to use getCurrentPosition

Do not even think about doing this:

setInterval(function() {
  bgGeo.getCurrentPosition(success, failure);
}, 1000);

#watchPosition was created to prevent people doing this.

What I recommend you do is simply:

// Start tracking immediately
bgGeo.changePace(true);
jbfrequence commented 7 years ago

and do you recommand a recursive call like this ?

var track = function(){

 bgGeo.getCurrentPosition(function(location, taskId){
       // do my track stuff, computing distance speed etc..
       doMyTrackStuff(location);

       // finish background task
       bgGeo.finish(taskId);

       // next track
       track();

   }, failure);

}
christocracy commented 7 years ago

HELL NO. Forget about using #getCurrentPosition and #watchPosition to track a device!

Let the plugin do it's job. Just changePace(true).

christocracy commented 7 years ago

You already get speed from GPS. And the plugin is calculating the distance attaching odometer to each location.

You need to forget about what you've learned before using this plugin.

christocracy commented 7 years ago

You can configure the accuracy used for odometer calculations with desiredOdometerAccuracy

jbfrequence commented 7 years ago

? speed from GPS is not very accurate if you don't smooth the data yourself. For a car, I understand than 10 meters acuracy is enough but not for a runner that runs between 10 and 20km/h..

Thanks for you help

christocracy commented 7 years ago

speed from GPS is not very accurate

Yet you're going to calculate your Δd, Δt of two GPS locations and d/t = v?

jbfrequence commented 7 years ago

Yes, delta time & delta speed & delta distance. It's not the main stuff, these computations are trivial.

jbfrequence commented 7 years ago

For a foreground tracking with android device, this approach consumes less than 13% per hour

jbfrequence commented 7 years ago

I am trying to refactor my code to follow your guidelines (using the standard tracking mechanism with my own stuff computation relying on my runner business rules) but I need some fallback function to call when no GPS location updates arrive after a certain amount of time (10s, for any reason, one of my runner is under a tunnel for instance, we have a lot like this on the parisian marathon road..). Maybe i am wrong but I don't see any timeout fallback mechanism in the location change "on" function.

christocracy commented 7 years ago

location event.

If an error occurs while fetching the location, the failureFn will be executed with an Integer Error Code as the first argument. Ie:

bgGeo.on('location', function(location, taskId) {
  console.log('- Location', location);
  bgGeo.finish(taskId);
}, function(error) {
  console.log('- Location error: ', error);
  if (error == 408) {
    console.log('- Location timeout');
  }
});
christocracy commented 7 years ago

I'm hammering my device repeatedly running #watchPosition / #stopWatchPosition, #stop / #start, #changePace, all while running #getCurrentPosition in a setInterval:

setInterval(function() {
  bgGeo.getCurrentPosition(function(location, taskId) {
    console.log('- getCurrentPosition: ', location);
    bgGeo.finish(taskId);
  });
}, 1000);

I cannot make it crash. Can you reproduce this crash on your own device?

jbfrequence commented 7 years ago

I can't reproduce the original bug on my own device (Iphone 5, IOS 10, no M7 chip) I ran with it more than 1 hour and i didn't get any crash Only my runners with M7 chip have reproduce this issue and only on IOS devices (Iphone >= 5S) Their app crashed after around the 5fth kilometer of run.

jbfrequence commented 7 years ago

If you own several iphone devices and want to test the tracking with our app, i am able to invite you as a new IOS beta tester (the app is not yet published in US stores but the beta testing with TestFlight may be a workaround). If so, you will have access to the latest dev version of app, I don't know if you really need a debug one to investigate.

Other thing, strange, a collegue tested the tracking (with watch/stop position implementation) on Iphone yesterday and after his run (1hours), no crash but his IOS document and storage dramatically increased from 6.7Mo to 23.6Mo. He has desinstalled and re-install the app before the run. Before the refactoring with your plugin, it was not the case. I figure out if there is something related to the SQLite database that I don't want to use. Does the "persist" flag in configuration works in all case ? Maybe i need to clear the database after each tracking.

christocracy commented 7 years ago

The plugin logs a lot of data, controlled with logLevel and logMaxDays

This data is to help you during development.

jbfrequence commented 7 years ago

OK Thanks, I will use LOG_LEVEL_OFF in the production config and keep them during the development phase

christocracy commented 7 years ago

That is correct.

christocracy commented 7 years ago

What's the status of this issue?

jbfrequence commented 7 years ago

Hi,

I don't use anymore the watch/stop functions and followed your guidelines. It seems to work correctly (no crash during 2 hour running sessions on IOS on different devices), accurate measures and efficient battery consumption on all devices. Maybe just a detailed documentation on when to use these functions (watch/stop) would be appreciated by other developpers.

christocracy commented 7 years ago

The plugin is designed around #start, #stop

It's in the Philosophy of Operation doc.

watchPosition is merely a recent addition a few months ago.

-- Snet form Gmail Mobile