Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Throttle network requests for the same location. #45

Closed
EugeneEl opened this issue Sep 25, 2018 · 7 comments
Closed

Throttle network requests for the same location. #45

EugeneEl opened this issue Sep 25, 2018 · 7 comments

Comments

@EugeneEl
Copy link

I've played a little with SDK (latest version, master branch).
I'm testing it on my project which has NetworkLogger. (I use it to inspect network request, log specific info)
What I've found strange that my application using SDK executes tones of API requests for the same coordinates. I tried to change my location just by walking at the office.
So I don't have significant location changes, just stationary.
Also I use default accuracy options (30 meters).
It might be better to add some throttling mechanism in order to reduce number of network requests for the same location.

Image with debugger
Link for screenshot

iOS version: 11.4.1
Device: iPhone 6+
Xcode: 9.4.1
GPS/Wifi/CoreMotion is Enabled.

@sobri909
Copy link
Owner

Interesting!

I'm guessing you've got timeline.activityTypeClassifySamples set to true in the Demo App's main view controller?

The requests you're seeing are requests for activity type ML model data. For them to repeat like that would suggest that either the server isn't returning any models, or it's denied the request (unlikely, because the response code is 200), or there's a bug in that version of LocoKit that allows duplicate/redundant model requests.

Either way, the first step I'd recommend is switching to the develop or timelines branches, as the master branch is very far behind these days. I haven't had a spare moment to ship a new stable release in quite some months.

The timelines branch is the most up to date, and will be the code used in the next Arc App version (probably getting submitted to Apple tomorrow). So that'll be the one to go with.

Let me know if you see the same problem in that branch. Thanks 😄

@EugeneEl
Copy link
Author

EugeneEl commented Sep 25, 2018

Yes, I think you're right.
Data for my region is unavailable for now. I checked that via map on your website and even tried to fetch classifier models in demo by passing hardcoded coordinates😊
Because SDK has no classifier models it attempts to fetch them again and again so it confused me.
In any way this issue can be closed)

Also I downloaded timelines branch but unfortunately I got 2 errors:
ActivityTypesCache

   LocoKitService.fetchModelsFor(coordinate: depthCenter, depth: depth) { json in
            if let json = json { self.parseTypes(json: json) } 

Type 'LocoKitService' has no member 'fetchModelsFor'

LocomotionManager

        LocoKitService.requestWakeup(at: wakeupTime)

Type 'LocoKitService' has no member 'requestWakeup'

Of course I won't create a new issue for it because you're currently working on this branch😄
If you have a chance to update or answer for this it would be cool.
In any way I will wait for stable release
Thank you for your fast answer😄

@sobri909
Copy link
Owner

Data for my region is unavailable for now. I checked that via map on your website and even tried to fetch classifier models in demo by passing hardcoded coordinates😊

Hmm. Are you using Arc App in that area? As long as you've confirmed/corrected any activity types at all in that region, there should be model data on the server.

The LocoKit coverage page won't necessarily show the model data coordinates for that region, because the data has to pass various privacy thresholds before it can be shown publicly.

Type 'LocoKitService' has no member 'fetchModelsFor'

That's odd. I built a new LocoKitCore binary just a couple of days ago to fix that exact problem. And confirmed in the Demo App in that that it was working correctly.

It's probably worth deleting the Pods folder and doing a fresh pod install, to be sure you've got the latest LocoKitCore.

@sobri909
Copy link
Owner

I just realised this morning that the coverage map is actually a good indicator of what data will be available to LocoKit, because LocoKit has the same privacy restrictions enforced on it. The same privacy thresholds are being applied in both cases.

So yeah, if you're not seeing anything on the coverage map for that area, then there won't be any data available to LocoKit either. It would require multiple users training their models to completion, for that region, so that there's no risk of the model data representing only a single person.

@EugeneEl
Copy link
Author

Thank you for explanation. I'll try to test SDK in some another region.
Are you going to merge timelines branch into master?
So it can be considered as final?)
Or you're going to test it in Arc application first?
Sorry for too many questions)

@sobri909
Copy link
Owner

Are you going to merge timelines branch into master? So it can be considered as final?

Yep! And yep!

Or you're going to test it in Arc application first?

I submitted an updated Arc App to Apple last night, which is built using that branch of LocoKit. So that'll be going live on the App Store presumably later today.

The only thing holding it back from being merged into master is missing documentation. Well, and one ugly bit of API, but the workaround for that ugly API is a one liner, so it's not a major blocker.

I'll probably merge it into master next week. I doubt I'll get time to fill in all the missing documentation first though.

@sobri909
Copy link
Owner

sobri909 commented Oct 4, 2018

I'm going to close this issue for now, on the assumption that the timelines branch isn't doing repeated wasteful requests.

But @EugeneGoloboyar if you're able to reproduce the problem on that branch, definitely let me know! There's throttling code in there to stop it, but there's always a chance I missed something. Thanks 😄

@sobri909 sobri909 closed this as completed Oct 4, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants