mapbox / ios-sdk-examples

Live example code for the Mapbox iOS SDK
https://www.mapbox.com/ios-sdk/examples/
Other
95 stars 35 forks source link

Completed Resources test for Offline Pack fires more than once in `OfflinePackExample.swift` #419

Open roblabs opened 3 years ago

roblabs commented 3 years ago

In OfflinePackExample.swift, when downloading an offline pack there is a test to determine whether the Completed Resources is equal to the Expected Resources.

The test that determines download complete fires more than once, and I'm looking for suggestions on how to improve handling this.

https://github.com/mapbox/ios-sdk-examples/blob/5fff10393e0d48f0fdc2b98e1b0be199feb8e017/Examples/Swift/OfflinePackExample.swift#L101,L103

if completedResources == expectedResources {
  let byteCount = ByteCountFormatter.string(fromByteCount: Int64(pack.progress.countOfBytesCompleted), countStyle: ByteCountFormatter.CountStyle.memory)
  print("Offline pack “\(userInfo["name"] ?? "unknown")” completed: \(byteCount), \(completedResources) resources")
}

In practice, we want to use the state that all expected resources have downloaded, and update a user interface. In our app, the test for completedResources == expectedResources is true more than twice.

This issue occurs when downloading from the following apps:


Log from Examples, where the completed state is logged twice.

Offline pack “My Offline Pack” has 4 of 545 resources — 0.73%.
Offline pack “My Offline Pack” has 69 of 545 resources — 12.66%.
... <snip a bunch>
Offline pack “My Offline Pack” has 544 of 545 resources — 99.82%.
Offline pack “My Offline Pack” completed: 7 MB, 545 resources
Offline pack “My Offline Pack” completed: 7 MB, 545 resources

Our Apps' offline packs support downloading non-contiguous grids. In our case, our UI reports that 4 out of 2 grids have been downloaded.

image

captainbarbosa commented 3 years ago

Thanks for the feedback. Tagging in my colleague @ZiZasaurus who has been working close to this example/use case lately.

ZiZasaurus commented 3 years ago

@roblabs, thank you for providing this feedback. I'm still looking into why completedResources == expectedResources is called twice, however, in the meantime, you can replace this with pack.state == .completed, which will only be called once.

roblabs commented 3 years ago

@ZiZasaurus — thank you for the tip on handling status via pack.state. I tried your suggestion, but I am still seeing the complete state report being reported more than once. I think the proper fix in our case would be to:

  1. check for pack.state == MGLOfflinePackStateComplete
  2. exit out of the method offlinePackProgressDidChange()
    1. by an instance variable in the class or some other structure to track

Log after deleting app and restarting (on simulator)

Offline pack “My Offline Pack” has 527 of 531 resources — 99.25%. Offline pack “My Offline Pack” has 528 of 531 resources — 99.44%. Offline pack “My Offline Pack” completed: 6.8 MB, 531 resources Offline pack “My Offline Pack” completed: 6.8 MB, 531 resources


Swift change

// If this pack has finished, print its size and resource count.
if pack.state == .complete {
    let byteCount = ByteCountFormatter.string(fromByteCount: Int64(pack.progress.countOfBytesCompleted), countStyle: ByteCountFormatter.CountStyle.memory)
    print("Offline pack “\(userInfo["name"] ?? "unknown")” completed: \(byteCount), \(completedResources) resources")
}

Objective-C Change

// If this pack has finished, print its size and resource count.
if (pack.state == MGLOfflinePackStateComplete) {
    NSString *byteCount = [NSByteCountFormatter stringFromByteCount:progress.countOfBytesCompleted countStyle:NSByteCountFormatterCountStyleMemory];
    NSLog(@"Offline pack “%@” completed: %@, %llu resources", userInfo[@"name"], byteCount, completedResources);
}