Conversation
There was a problem hiding this comment.
Code Review
This pull request integrates the Ongoing Activity API into the ForegroundOnlyWalkingWorkoutService for both the start and finished modules. Key changes include refactoring notification updates into a centralized updateOngoingActivity method and adding required permission annotations. Feedback focuses on a functional regression in the start module where the new update method is left as a TODO, effectively disabling notification updates that previously worked. There is also a request to restore an accidentally deleted log statement to maintain consistency between the project modules.
| val updatedStatus = getString(R.string.walking_points_text, walkingPoints) | ||
| updateOngoingActivity(updatedStatus) |
There was a problem hiding this comment.
This change replaces the notification update logic with a call to updateOngoingActivity, which is currently an empty method with a TODO. This results in a functional regression where the walking points are no longer updated in the notification for the start version of the codelab. While this might be to set up an exercise for the user, it leaves the app in a non-working state. Consider preserving the old notification update logic within the new updateOngoingActivity method to ensure the app remains functional at the beginning of this codelab step.
| // NOTE: If this method is called due to a configuration change in MainActivity, | ||
| // we do nothing. | ||
| if (!configurationChange && walkingWorkoutActive) { | ||
| Log.d(TAG, "Start foreground service") |
There was a problem hiding this comment.
Update codelab to avoid recreating notification and ongoing activity for each status update
Instead, maintain a reference to the ongoing activity and just update using
ongoingActivity.update(...).Codelab documentation to be updated once these changes land.