-
Notifications
You must be signed in to change notification settings - Fork 0
Fix/4 Disable samsung health #5
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
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Small doubt, but it looks good @panjiyudasetya !!
@@ -18,7 +18,8 @@ | |||
android:vmSafeMode="true"> | |||
> | |||
<!-- Play service metadata --> | |||
<meta-data | |||
<!-- TODO: Enable this once we integrate with Samsung Health --> | |||
<!-- <meta-data | |||
android:name="com.google.android.gms.version; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not really sure what this metadata is used for, but we don't need this line android:name="com.google.android.gms.version"
? Or is it somehow just used for the Samsung Health?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's only be used for Samsung Health. If I compare the manifest file with with GoogleFit example here:
https://github.com/android/fit-samples/blob/master/BasicHistoryApiKotlin/app/src/main/AndroidManifest.xml
meta-data is not required at all for GoogleFit. But let me double check it by run the example project to ensure I'm not mess anything up. 😥
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
@xaviraol sorry for the super late reply, I have try to run the example project under You could also do the same thing to ensure everything works well 😉 |
bufff so old, do we still need this? @panjiyudasetya What's your opinion on that @jagamypriera ? |
@xaviraol it's better to getting rid any codes that are not necessary 🙂 |
This PR resolvs #4.
Description
I have reported from Syahmia that our App keep sending
SamsungHealth
request check update when app in background.According to the
HealthDataStore
documentation, crash would only occur when we provide incorrect parameters in the constructor, which is something that we did properly while we setting up SamsungHealth sdk.See:
https://img-developer.samsung.com/onlinedocs/health/android/data/com/samsung/android/sdk/healthdata/HealthDataStore.html
So I assume the log that we see are came from
exception.printStackTrace()
and it's only for logging purpose. Then, what I did in the PR is to ensure we disable any services that potentially connected toSamsungHealth
and reduce any false alarm to the team.