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

we don't need to *also* store the boot time #18

Closed
kaushikgopal opened this issue Sep 19, 2016 · 9 comments
Closed

we don't need to *also* store the boot time #18

kaushikgopal opened this issue Sep 19, 2016 · 9 comments

Comments

@kaushikgopal
Copy link
Collaborator

boot time = sntp time - device uptime. we shouldn't have 3 flags

https://github.com/instacart/truetime-android/blob/master/library/src/main/java/com/instacart/library/truetime/DiskCacheClient.java#L12

@montanalow
Copy link

montanalow commented Sep 19, 2016

We do need to cache boot time right? It may change when the device is
rebooted, and we need to check if boot time (cache) + uptime (read) is no
longer consistent with sntp time (cache) on every app resume.

On Mon, Sep 19, 2016 at 3:37 PM, Kaushik Gopal notifications@github.com
wrote:

boot time = sntp time - device uptime. we shouldn't have 3 flags

https://github.com/instacart/truetime-android/blob/master/
library/src/main/java/com/instacart/library/truetime/
DiskCacheClient.java#L12


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
#18, or mute the
thread
https://github.com/notifications/unsubscribe-auth/AAQ2nfoq4ep7cI9cJmELFe7N__zLph7Lks5qrvJtgaJpZM4KA77L
.

@montanalow
Copy link

I think we don't need to cache uptime instead, since it should be cheap to read, and we'll want the latest version every second.

@kaushikgopal
Copy link
Collaborator Author

We do need to cache boot time right?

That's right, we would need to know the last boot time but this can easily be derived by sntpTime - deviceUptime. Currently we're caching all three values but one of those is anyway derived from the other two.

It may change when the device is rebooted

yup, that's right too but rather than checking on every app resume (which would include after app kills for e.g.), we can listen to "boot" events, which is what the boot broadcast receiver does. On every boot we clear the disk cache, so the case you mentioned is accounted for. But we get the advantage of not having to check this on every app resume. just device boots.

@kaushikgopal
Copy link
Collaborator Author

I think we don't need to cache uptime instead, since it should be cheap to read, and we'll want the latest version every second.

we'll need the uptime at the point that sntp was generated. Or alternatively, we could store the boot time (either would work)

@montanalow
Copy link

Cool. Does the boot broadcast still get delivered even if the app is not running, and then processed after app resume?

@kaushikgopal
Copy link
Collaborator Author

kaushikgopal commented Sep 20, 2016

so my understanding is that the boot broadcast would be received if the app was ever launched atleast once. i.e. if it was never launched then it wouldn't receive the broadcast. but if it was launched once, then we should be getting these. i tested this with a basic boot case adb shell start; adb shell stop and it looks like it does the trick

@Bakhrom
Copy link

Bakhrom commented Feb 25, 2017

Probably does not always work this receiver, error log reports show similar:

Device time: 2017-02-23T21:12:56.630-05:00
Location time: 2017-02-23T21:12:56.613-05:00 
NTP time: 2017-02-17T13:55:57.835-05:00

@kaushikgopal
Copy link
Collaborator Author

@Bakhrom : i would encourage you to open a different issue on this. (might be a faulty NTP server ? In that new issue please give me details like which ntp pool server your using and if you've pulled the latest code)

@kaushikgopal
Copy link
Collaborator Author

i'm closing this issue out, cause i'm not enthused at the moment to change it. i'm open to PRs: :P

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

3 participants