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

Add History of cigarettes smoked #24

Closed
amaury1093 opened this issue May 18, 2018 · 21 comments · Fixed by #121
Closed

Add History of cigarettes smoked #24

amaury1093 opened this issue May 18, 2018 · 21 comments · Fixed by #121
Labels
F-planned New feature or request good first issue Good for newcomers
Milestone

Comments

@amaury1093
Copy link
Member

Right now, the AQI project doesn't give any historical data. There is apparently some platform here: http:https://aqicn.org/data-platform/register/, I registered, no answer yet. But it looks more like a one time thing, e.g. "Tell me the air quality data in Winter 2014 in Beijing", and not a online API we could use.

I wouldn't put much hope into the above solution. Which means, we cannot have the past history of cigarettes smoked.


Instead, we could start storing data. I see 2 solutions:

Solution 1: Store the data locally on the user's phone

We start storing from the time the user installs the app. We would store the current AQI at the user's location every x hours (x tbd), and there would be a "Past months summary" feature somewhere where we do an average calculation.

Solution 2: Store the data on a server of ours

A server that we maintain would poll & store historical data of all AQI. We could have a lot of data. But this requires a some effort, so maybe later.

@amaury1093 amaury1093 added the F-planned New feature or request label May 18, 2018
@amaury1093 amaury1093 added this to To do in Sh**t! I Smoke via automation May 18, 2018
@marceloscoelho
Copy link
Collaborator

This seems to be the most useful feature in my view. People are not asking so much as I thought they would, but to really have an accurate notion of the air quality, an average is necessary.

Solution 1 seems better considering Privacy and even savings for us (owning a fast server with everyone's data wouldn't be so cheap). Do you think this would be something that would occupy a lot of space on the user's HD?

@amaury1093
Copy link
Member Author

Agreed, 1 seems the way to go.

After some quick estimates, it's maximum 5kb per day. 1.5Mb per year. It's totally not an issue.

@amaury1093
Copy link
Member Author

amaury1093 commented Apr 17, 2019

Use https://docs.expo.io/versions/v32.0.0/sdk/location/#locationstartlocationupdatesasync to track background location. A ping every 1h-2h sounds reasonable.

I would say we store at each ping:

  1. datetime of the ping
  2. user geolocation
  3. PM2.5 raw concentration at the ping.

@amaury1093 amaury1093 added the good first issue Good for newcomers label Apr 17, 2019
@lucienbl
Copy link
Contributor

Wouldn't letting the app constantly running in the background drain the battery, especially for these non-native apps which means they aren't particularly optimized for such background tasks?

@amaury1093
Copy link
Member Author

I was thinking of something like: every 3 hours, fetch the current location, fetch the PM2.5 at this location, and store it locally. I'm not 100% sure this is technically feasible with Expo's TaskManager, but if it is, I don't think it'll drain the battery, since it's not a permanent background task?

@lucienbl
Copy link
Contributor

Hm but to fire the worker every 3 hours your app need something which will constantly run in background, which is feasible with services / broadcast receivers through native apps but I know I already had issues working with rn background tasks or am I wrong? 🤔

If there's a background task manager provided by expo they probably work with native implementations, then the battery drain should not be that important, I honestly don't really know how expo works since I mostly worked without it 😄

@amaury1093
Copy link
Member Author

Expo does have a TaskManager: https://docs.expo.io/versions/v32.0.0/sdk/task-manager/. Tbh I'm not sure either how it works, we can always implement something, test it for ~1 week to see how the battery behaves, and then reconsider if necessary.

If Expo doesn't provide any good solution, we can eject.

I'll be able to have a look at this next week. @lucienbl in the meantime if you're interested, feel free to explore with TaskManager!

@lucienbl
Copy link
Contributor

Sounds good! I'll have a look for sure!

@lucienbl
Copy link
Contributor

Hi there! I started looking a bit around the way how this can be stored and did this 0ab3cb9 ! It seems to work, except the API call which doesn't seem to work in background 🤔

@amaury1093
Copy link
Member Author

Nice start! Is the callback called? I.e. if you put a console.log here, does it show something?

@lucienbl
Copy link
Contributor

Normally data contains the basic response coming from the location manager. What do you mean is it supposed to show?

This part works perfectly in foreground, but the API call seems to have some trouble when calling from background mode.

@amaury1093
Copy link
Member Author

@lucienbl Have a look at https://blog.expo.io/expo-sdk-v33-0-0-is-now-available-52d1c99dfe4c#8b09, they apparently fixed BackgroundFetch on Android!

Do you still want to work on it?

@lucienbl
Copy link
Contributor

lucienbl commented Jun 8, 2019

Hey! Sure, I'll have a look into it. Do you update the project to the newer version, if it's not already done? Otherwise I'll do it if you tell me what dependencies should be updated 😄

@amaury1093
Copy link
Member Author

Otherwise I'll do it if you tell me what dependencies should be updated

Let's do like that, I'll let you update to Expo@33. In one PR, or in 2 separate PRs, I'll let you decide.

@lucienbl
Copy link
Contributor

Hey! I let you take a look at this which should now work with Expo 33. Let me know about it

@amaury1093
Copy link
Member Author

amaury1093 commented Jun 23, 2019

This is super cool, thanks! Didn't try to play with it yet, but could you create a PR, I like looking at the code diff

@lucienbl
Copy link
Contributor

Sure, it's here #109 😉

@marceloscoelho
Copy link
Collaborator

Hey @amaurymartiny and @lucienbl ! I've worked on the design for the history feature and it would be nice to get your opinions. Here's a quick prototype of how it would work:
shoot-history

The idea would be to keep the structure of the app as similar as the current one, just adding a button behind the "Daily" and the other options (Weekly, Monthly) to make it clear to the user it's clickable.
I've done other tests that are available for you to see/comment on Figma:
https://www.figma.com/file/vwJbDYQwH3SzpjIZKHOq1V/screens-v6-history-Copy?node-id=300%3A0

You can also play with the Flinto prototype on your phones by downloading the app and this file:
shoot-history-flinto.zip

@marceloscoelho
Copy link
Collaborator

Maybe a small delay is necessary on loading the cigarettes information:
shoot-history-delay

@amaury1093
Copy link
Member Author

I'm reopening this issue, because I removed all the history features (mostly done by Lucien) in this PR: #202.

The reason is this upstream issue: expo/expo#3582. After some thorough testing, BackgroundFetch just never gets called...

Note: Putting a Hacktoberfest label here, but it would be finding a way to fix the Expo upstream issue.

@amaury1093 amaury1093 removed this from the v1.4 milestone Oct 1, 2019
@amaury1093 amaury1093 added this to the v1.5 milestone Feb 3, 2020
@amaury1093
Copy link
Member Author

Closed in #425, we're using OpenAQ's historical data for this feature.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
F-planned New feature or request good first issue Good for newcomers
Projects
No open projects
Sh**t! I Smoke
  
Ideas
Development

Successfully merging a pull request may close this issue.

3 participants