Enhance: Fix current time by RealClock #288
Conversation
|
@jaydeetay Please have a look. |
app/src/main/java/com/google/android/stardroid/control/RealClock.java
Outdated
Show resolved
Hide resolved
|
@jaydeetay @barbeau I have updated it. Please review. |
|
@jaydeetay will need to weigh in here as he's more familiar with the code and history of project, but I don't think any change is needed to the code - I'm not sure I understand the TODO comment |
|
@barbeau Yes even though the TODO comment is not appropriate. I have updated the code for accurate time according to UTC, the user be in any time zone. |
|
@jaydeetay @barbeau I also think that we could use two methods here, one for returning time in UTC and other for the local time. |
| // For getting the time in UTC | ||
| long offset = currentTime.get(Calendar.ZONE_OFFSET) + | ||
| currentTime.get(Calendar.DST_OFFSET); | ||
| return currentTime.getTimeInMillis() + offset; |
There was a problem hiding this comment.
Why are the offsets needed? The API docs say getTimeInMillis already returns UTC. I'm a bit rusty on this stuff. It's not obvious to me why this is better than the existing code.
There was a problem hiding this comment.
I don't think it is - the existing code seems right to me.
There was a problem hiding this comment.
Well not that it is completely true. I have seen time differences using the SystemClock . The offsets are there for confirmation on the accuracy (if ever there is a time zone issue on any device) . @jaydeetay
Fixes #287
Run the unit tests with
gradlew app:connectedGmsDebugAndroidTestto make sure you didn't break anything (with an emulator running or phone connected)If you have multiple commits please combine them into one commit by squashing them.