-
Notifications
You must be signed in to change notification settings - Fork 177
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
Added fitness trend options to prefer using PSS/RSS over HRSS #954
Conversation
This reverts commit e152426.
Update the note about the configuration being stored locally
…tats for others sports than running & cycling
Thanks for the PR ! I'm bit late to handle it sorry. Looking at this now |
<div fxFlex> | ||
<mat-slide-toggle | ||
[(ngModel)]="fitnessTrendConfigDialogData.fitnessTrendConfigModel.preferEstimatedPowerStressScore" | ||
[disabled]="!isEstimatedPowerStressScoreToggleEnabled"> |
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.
Toggle auto disable seems not working.
(i can handle it if you need)
<div fxFlex> | ||
<mat-slide-toggle | ||
[(ngModel)]="fitnessTrendConfigDialogData.fitnessTrendConfigModel.preferEstimatedRunningStressScore" | ||
[disabled]="!isEstimatedRunningStressScoreToggleEnabled"> |
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.
Toggle auto disable seems not working.
(i can handle it if you need)
// - HRSS | ||
// - TRIMP | ||
// - PSS without Power meter | ||
// - RSS | ||
// - SSS | ||
if (activity.powerStressScore && activity.hasPowerMeter) { | ||
if (activity.powerStressScore && (activity.hasPowerMeter || activity.preferEstimatedPowerStressScore)) { |
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.
Could you add some unit tests in fitness.service.spec.ts which could validate that your changes are working properly without regressions on legacy code?
You will find examples you can use inside this describe:
describe("prepare fitness activities", () => {
....
});
@@ -39,6 +39,8 @@ export class FitnessTrendComponent implements OnInit { | |||
initializedFitnessTrendModel: {ctl: null, atl: null}, | |||
allowEstimatedPowerStressScore: false, | |||
allowEstimatedRunningStressScore: false, | |||
preferEstimatedPowerStressScore: false, |
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.
You might aloso update below FitnessTrendComponent method:
public onEstimatedPowerStressScoreToggleChange(enabled: boolean): void {
if (this.fitnessTrendConfigModel.allowEstimatedPowerStressScore !== enabled) {
this.fitnessTrendConfigModel.allowEstimatedPowerStressScore = enabled;
this.saveConfigAndReloadFitnessTrend();
}
}
To
public onEstimatedPowerStressScoreToggleChange(enabled: boolean): void {
if (this.fitnessTrendConfigModel.allowEstimatedPowerStressScore !== enabled) {
this.fitnessTrendConfigModel.allowEstimatedPowerStressScore = enabled;
if (!this.fitnessTrendConfigModel.allowEstimatedPowerStressScore) {
this.fitnessTrendConfigModel.preferEstimatedPowerStressScore = false;
}
this.saveConfigAndReloadFitnessTrend();
}
}
Same idea for onEstimatedRunningStressScoreChange method
@@ -9,6 +9,8 @@ export class FitnessPreparedActivityModel { | |||
public type: string; | |||
public name: string; | |||
public hasPowerMeter: boolean; | |||
public preferEstimatedPowerStressScore: boolean; |
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.
Both boolean should not be here.
Just pass the fitnessTrendConfigModel
object into FitnessService::dayStressOnDate
method.
Update the method to:
public dayStressOnDate(currentDay: moment.Moment, fitnessPreparedActivities: FitnessPreparedActivityModel[], fitnessTrendConfigModel: FitnessTrendConfigModel): DayStressModel
You will understand :)
// - HRSS | ||
// - TRIMP | ||
// - PSS without Power meter | ||
// - RSS | ||
// - SSS | ||
if (activity.powerStressScore && activity.hasPowerMeter) { | ||
if (activity.powerStressScore && (activity.hasPowerMeter || activity.preferEstimatedPowerStressScore)) { |
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.
Use instead:
if (activity.powerStressScore && (activity.hasPowerMeter || fitnessTrendConfigModel.preferEstimatedPowerStressScore))
dayActivity.finalStressScore += activity.powerStressScore; | ||
} else if (activity.runningStressScore && activity.preferEstimatedRunningStressScore) { |
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.
Use instead:
else if (activity.runningStressScore && fitnessTrendConfigModel.preferEstimatedRunningStressScore)
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
4a227a2
to
5073394
Compare
This issue has been automatically closed because it didn't had recent activity. |
I prefer using RSS instead of HRSS for Fitness Trend Stress Score calculations. I added the code so this can be turned on/off in 'Configure Fitness Trend' settings (the gear icon in the Fitness Trend) and also added similar for PSS. This was raised as Feature Request #736