Create Calorie Tracking app w/ health integration#4142
Create Calorie Tracking app w/ health integration#4142RKBoss6 wants to merge 79 commits intoespruino:masterfrom
Conversation
Add metadata for Calorie Tracker application
Expanded the README to include the calorie calculation formula and its components, enhancing user understanding of the app's functionality.
…date boot.js to match
…nshot for health, add gender for myprofile
Refactor logic for setting resting heart rate and round max heart rate calculation.
Added instructions for taking Resting Heart Rate (RHR) measurements, including tips for optimal conditions and app navigation.
…nto caloriesApp
…nto caloriesApp
|
Alright, this is good to go at long last!! Thanks for being patient as I finalized the last few things, @thyttan! |
|
Any update on this? |
|
Ah! I can try it out now :) |
|
Sorry, I just put it on my app loader right now |
thyttan
left a comment
There was a problem hiding this comment.
Thanks! Here's my initial review :)
There was a problem hiding this comment.
Please add white space in line with the preferred code style: https://www.espruino.com/Code+Style
Maybe easiest to use some auto formatter for this initial contribution to the repo?
And same for the other js files in the calories directory.
For the changes to the health and myprofile apps please follow the style there as well, but don't auto format the whole files.
There was a problem hiding this comment.
Thanks! Just changed this app.js, does that look better?
There was a problem hiding this comment.
Yes, thanks! I think so. But you lost all semi-colons at the same time? I think that will make e.g. the web ide editor complain.
There was a problem hiding this comment.
Oh no! I didn't even see that 😂, just fixed it!
There was a problem hiding this comment.
Unresolved since the other files in the 'calories' directory is yet to be updated wrt white space.
There was a problem hiding this comment.
Unresolved since the other files in the 'calories' directory is yet to be updated wrt whitespace.
| @@ -0,0 +1,127 @@ | |||
| // Takes object with bpm, movement (in duration), steps (in duration), and duration in minutes | |||
| let calcAge=function(rawBday){ | |||
| let birth = new Date(rawBday); | |||
There was a problem hiding this comment.
I get this error, probably because I have not done any setup in the myprofile app?:
Uncaught TypeError: Variables of type undefined are not supported in date constructor
at calcAge (calories:2:41)
let birth=new Date(rawBday);let now=new Date();let diffInDays=(now-birth)/8...
^
at calcBMR (calories:3:93)
...=calcAge(myProfile.birthday);if(age<18){if(myProfile.gender!=undefined&&...
^
at intermittentBMRUpdate (.boot0:30:140)
...calModule.calcBMR(myProfile)*((now-lastBMRWrite)/60000));cal...
^
at .boot0 (.boot0:32:512)
...()});intermittentBMRUpdate();;
^
If so we should make this not happen some way. Either by using some default value somewhere or something. Not sure what's the best solution.
There was a problem hiding this comment.
Should we catch the error and display a prompt about entering info in myprofile? From the prompt the user could be directed to the myprofile app via a positive action button.
There was a problem hiding this comment.
Should we catch the error and display a prompt about entering info in myprofile. From the prompt the user could be directed to the myprofile app via a positive action button.
There was a problem hiding this comment.
That sounds really nice actually - I'll get on that
| // returns cals/minute | ||
| exports.calcBMR=function(myProfile){ | ||
| let bmr=0; | ||
| let weight = myProfile.weight; |
There was a problem hiding this comment.
I get this error, probably since I haven't added any info to myprofile. Either we try to make sure the user have already entered info there. Or we nudge the user to add their weight. And/or add some default average value. What do you think?
>
{"t":"act","stp":0,"hrm":0,"mov":175,"act":"UNKNOWN"}
Uncaught Error: Calories: Not enough myProfile data to calculate!
at calcCalories (calories:10:234)
...yProfile data to calculate!);}let weight=myProfile.weight;let age=...
^
at .boot0:32:13
}),myProfile)if(!cd)return;calData.activeCaloriesBurned+=cd.activeC...
^
There was a problem hiding this comment.
I think this may be the best solution as of now? I feel like nudging the user to add certain elements means we need several checks that are separate for each element. I've added a table in the README for all the required fields, so if the user sees it, they can quickly check the readme/update the fields that are missing that way. Wdyt?
There was a problem hiding this comment.
Good idea adding to the readme!
I think we can also do the same thing with catching the error and presenting the user with a prompt like suggested for that other error.
There was a problem hiding this comment.
Yep, sounds good! We'll just have one prompt then, that says Not enough myprofile data, and then prompts to open the app settings?
There was a problem hiding this comment.
Yeah I think that's good :)
There was a problem hiding this comment.
Just committed, check it out!
| }); | ||
| }; | ||
|
|
||
| menu[/*LANG*/`Resting HR: ${myprofile.restingHrm?myprofile.restingHrm:"--"}`]=RHRReading; |
There was a problem hiding this comment.
Not sure how best to handle but just looking in the settings page it looks weird having a "HR min" entry and a "Resting HR" entry. Even more so when they don't show the same number.
There was a problem hiding this comment.
Would it be better if we do the full names like Minimum HR, Resting HR, Maximum HR?
There was a problem hiding this comment.
I think you elaborated on this somewhere - could you link me to that thread? 🙏🙂
There was a problem hiding this comment.
Huh, I don't think I did at all... But will that make it any better in your eyes if we did do the full name?
There was a problem hiding this comment.
It was actually on this PR, sorry :p
Could we change this a bit? I think it would be better if we just kept the one field 'HR min' and then have the resting heart rate measurement set the 'HR min' value once it finishes. That way we don't introduce a new store of the same data we already keep in the 'HR min' entry.
My suggestion in points:
- Rename the 'Resting HR: xx' entry to 'Take resting HR' or similar.
- Move that entry up so it's just below the 'HR min' entry.
- Make the now renamed 'Take resting HR' entry update the 'HR min' fields value.
- Update other files and readme to accommodate this change.
| } else { | ||
| const age = (new Date()).getFullYear() - date.getFullYear(); | ||
| const newMaxHRM = 220-age; | ||
| const newMaxHRM = Math.round(208-0.7*age); |
There was a problem hiding this comment.
Do you have a source saying the new calculation is actually better/more accurate?
There was a problem hiding this comment.
Ok, thanks. That page does not say explicitly that it's proven to be better. But it's good enough I think. Maybe we put that web adress in a comment at the end of the line?
Added prompt to set MyProfile data if missing for calorie calculations.
|
Should we try to get this one over the finish line? I'll probably have some time to look closer again sunday morning :) |
|
Yes, that sounds good! I won't have that much time to do it for a while, but I can take a look at it in bits and pieces and get it done gradually :) |
|
Ok, no stress 🙂 |
This is just a draft right now, but creates an app for calorie tracking based on #4092, that counts calories burned in a day and splits it by BMR, Active, and total calories. Uses MyProfile to get user health data, and hooks onto health events for new data. Also modifies myprofile to use RHR readings as an additional piece of data, with its own measurement UI.
Check it out at my app loader