StasDoskalenko / react-native-google-fit

A React Native bridge module for interacting with Google Fit
MIT License
333 stars 209 forks source link

Add device info to the data when available #279

Closed kristerkari closed 2 years ago

kristerkari commented 2 years ago

When trying this library with Google Fit and my smart watch, I noticed that the returned steps data does not have any device info available.

Depending on where the data comes from, the device information can be found by calling dp.getOriginalDataSource().getDevice(). This PR adds that information to the response when available.

To me it looks like the Typescript types in the current version are not up-to-date, so I tried updating them, but I'm not sure if I did it correctly. Please let me know if you think that they are not correct.

P.S.

There are also other fields that could be added with a separate Pull Request, e.g. dp.getOriginalDataSource().getAppPackageName() and the appName when the data comes from a third-party app, such as Withings Health Mate or Polar Flow.

@aboveyunhai

aboveyunhai commented 2 years ago

@kristerkari thx for the info. I may not be able to quickly respond to the exact details. Based on your description and my code skimming. I believe you can just extract the switch case into a single function (for example, getDeviceInfo()) function in helper file, and then return the device string.

I believe this function is valuable to be used in other health data.

kristerkari commented 2 years ago

I believe you can just extract the switch case into a single function (for example, getDeviceInfo()) function in helper file, and then return the device string.

Yeah, I can do that. I was just using inline switch because the existing code was already using switch. :)

kristerkari commented 2 years ago

@aboveyunhai I extracted the switch statement into a function, which definitely looks a lot cleaner.

What I still would like to further discuss is the naming conventions, mainly the use of camelCase for values.

aboveyunhai commented 2 years ago

probably one last thing to check, the device info might be universal for different type of data. we already extract it out into a function, shall we also make a individual type or interface for it inside the typescript file.

kristerkari commented 2 years ago

probably one last thing to check, the device info might be universal for different type of data.

Looks like it is: https://developers.google.com/android/reference/com/google/android/gms/fitness/data/Device

we already extract it out into a function, shall we also make a individual type for it inside the typescript file.

Sorry, I did not quite get it, what do you want to add the type definition for? Do you want to make the function part of the public API of the library?

aboveyunhai commented 2 years ago

probably one last thing to check, the device info might be universal for different type of data.

Looks like it is: https://developers.google.com/android/reference/com/google/android/gms/fitness/data/Device

we already extract it out into a function, shall we also make a individual type for it inside the typescript file.

Sorry, I did not quite get it, what do you want to add the type definition for? Do you want to make the function part of the public API of the library?

check the review.

kristerkari commented 2 years ago

check the review.

Hmm.. what review are you referring to? I'm still a bit confused about what is the change that you want me to make.

aboveyunhai commented 2 years ago

check the review.

Hmm.. what review are you referring to? I'm still a bit confused about what is the change that you want me to make.

something like type DeviceInfo = { dataTypeName, dataSourceId ... }, then type rawSteps = Array<{startDate: string, endDate: string, steps: number}> & DeviceInfo, basically extract the device typescript out of rawStep

Do you might also capitalize rawSteps to RawSteps, it looks like my past mistake.

kristerkari commented 2 years ago

Ok, now I got it, thanks!

aboveyunhai commented 2 years ago

npm@0.18.3 is up.