Revert audio focus duck attempt (#423) — restore audio output for #421#428
Conversation
There was a problem hiding this comment.
Code Review
This pull request simplifies the Android haptic feedback implementation by removing unnecessary audio focus management logic. The review identified a potential resource leak in the MediaPlayer initialization and suggested refactoring the inlined sound name lists into constants to improve code efficiency and maintainability.
| try { | ||
| // Fire OS: Use USAGE_MEDIA to work around SoundPool volume bug | ||
| // Standard Android: Use USAGE_GAME to mix with music without interrupting | ||
| val audioAttributes = AudioAttributes.Builder() | ||
| .setUsage(playbackUsage) | ||
| .setUsage( | ||
| if (DeviceInfo.isFireOS()) { | ||
| AudioAttributes.USAGE_MEDIA | ||
| } else { | ||
| AudioAttributes.USAGE_GAME | ||
| }, | ||
| ) | ||
| .setContentType(AudioAttributes.CONTENT_TYPE_SONIFICATION) | ||
| .build() | ||
|
|
||
| val mediaPlayer = MediaPlayer.create(context, resId, audioAttributes, 0) | ||
| if (mediaPlayer == null) { | ||
| audioFocusSession.abandon() | ||
| return | ||
| } | ||
| val mediaPlayer = MediaPlayer.create(context, resId, audioAttributes, 0) ?: return | ||
| mediaPlayer.setVolume(1.0f, 1.0f) | ||
| mediaPlayer.setOnCompletionListener { | ||
| it.release() | ||
| audioFocusSession.abandon() | ||
| } | ||
| mediaPlayer.setOnCompletionListener { it.release() } | ||
| mediaPlayer.start() | ||
| } catch (_: Exception) { | ||
| // Silently fail - sound is not critical | ||
| audioFocusSession.abandon() | ||
| } |
There was a problem hiding this comment.
The MediaPlayer instance created via MediaPlayer.create is not released if an exception occurs during setVolume, setOnCompletionListener, or start. Since mediaPlayer is declared inside the try block, it is not accessible in the catch block, leading to a potential resource leak of native audio resources. Declaring the variable outside the try block allows for proper cleanup in the catch block.
var mediaPlayer: MediaPlayer? = null
try {
val audioAttributes = AudioAttributes.Builder()
.setUsage(
if (DeviceInfo.isFireOS()) {
AudioAttributes.USAGE_MEDIA
} else {
AudioAttributes.USAGE_GAME
},
)
.setContentType(AudioAttributes.CONTENT_TYPE_SONIFICATION)
.build()
mediaPlayer = MediaPlayer.create(context, resId, audioAttributes, 0) ?: return
mediaPlayer.setVolume(1.0f, 1.0f)
mediaPlayer.setOnCompletionListener { it.release() }
mediaPlayer.start()
} catch (_: Exception) {
mediaPlayer?.release()
}| /** | ||
| * Get a random badge celebration sound name. | ||
| */ | ||
| private fun getRandomBadgeSound(): String { | ||
| val badgeSoundNames = getBadgeSoundNames() | ||
| val badgeSoundNames = listOf( | ||
| "absolute_domination", "absolute_unit", "another_milestone_crushed", | ||
| "beast_mode", "insane_performance", "maxed_out", "new_peak_achieved", | ||
| "new_record_secured", "no_ones_stopping_you_now", "power", "pr", | ||
| "pressure_create_greatness", "record", "shattered", "strenght_unlocked", | ||
| "that_bar_never_stood_a_chance", "that_was_a_demolition", "that_was_god_mode", | ||
| "that_was_monster_level", "that_was_next_tier_strenght", "that_was_pure_savagery", | ||
| "the_grind_continues", "the_grind_is_real", "this_is_what_champions_are_made", | ||
| "unchained_power", "unstoppable", "victory", "you_crushed_that", | ||
| "you_dominated_that_set", "you_just_broke_your_limits", "you_just_destroyed_that_weight", | ||
| "you_just_levelled_up", "you_went_full_throttle", | ||
| ) | ||
| return badgeSoundNames[Random.nextInt(badgeSoundNames.size)] | ||
| } | ||
|
|
||
| /** | ||
| * Get a random PR celebration sound name. | ||
| */ | ||
| private fun getRandomPRSound(): String { | ||
| val prSoundNames = getPRSoundNames() | ||
| val prSoundNames = listOf("new_personal_record", "new_personal_record_2") | ||
| return prSoundNames[Random.nextInt(prSoundNames.size)] | ||
| } |
There was a problem hiding this comment.
Inlining the lists of sound names in getRandomBadgeSound and getRandomPRSound introduces duplication (as these lists are also defined in the remember block at line 90) and results in unnecessary object allocations on every function call. Moving these to private constants improves maintainability and efficiency. Note that the remember block at line 90 should also be updated to reference these constants.
private val BADGE_SOUND_NAMES = listOf(
"absolute_domination", "absolute_unit", "another_milestone_crushed",
"beast_mode", "insane_performance", "maxed_out", "new_peak_achieved",
"new_record_secured", "no_ones_stopping_you_now", "power", "pr",
"pressure_create_greatness", "record", "shattered", "strenght_unlocked",
"that_bar_never_stood_a_chance", "that_was_a_demolition", "that_was_god_mode",
"that_was_monster_level", "that_was_next_tier_strenght", "that_was_pure_savagery",
"the_grind_continues", "the_grind_is_real", "this_is_what_champions_are_made",
"unchained_power", "unstoppable", "victory", "you_crushed_that",
"you_dominated_that_set", "you_just_broke_your_limits", "you_just_destroyed_that_weight",
"you_just_levelled_up", "you_went_full_throttle",
)
private val PR_SOUND_NAMES = listOf("new_personal_record", "new_personal_record_2")
/**
* Get a random badge celebration sound name.
*/
private fun getRandomBadgeSound(): String {
return BADGE_SOUND_NAMES[Random.nextInt(BADGE_SOUND_NAMES.size)]
}
/**
* Get a random PR celebration sound name.
*/
private fun getRandomPRSound(): String {
return PR_SOUND_NAMES[Random.nextInt(PR_SOUND_NAMES.size)]
}Address gemini-code-assist review on PR #428: - playWithMediaPlayer: hoist `mediaPlayer` declaration out of the `try` block and release it from `catch`. Previous structure leaked the native player if setVolume/setOnCompletionListener/start threw. - Extract BADGE_SOUND_NAMES and PR_SOUND_NAMES to private top-level vals. Eliminates duplication between the SoundPool-loading `remember` block and getRandomBadgeSound/getRandomPRSound, and removes per-call list allocations on the random helpers.
There was a problem hiding this comment.
Pull request overview
Reverts the Android “transient duck audio focus” implementation introduced in #423 in order to restore reliable in-app audio cue playback (regression reported in #421), returning to the previously working SoundPool/MediaPlayer behavior while keeping earlier diagnostics and mappings.
Changes:
- Removed transient-duck audio focus request/abandon logic and associated duration-measurement/caching utilities.
- Simplified
playSoundandplayWithMediaPlayerto play without acquiring audio focus, relying on existing SoundPool + MediaPlayer fallback behavior. - Inlined badge/PR sound-name lists into the random-selection helpers after removing the shared helper functions.
Comments suppressed due to low confidence (1)
shared/src/androidMain/kotlin/com/devil/phoenixproject/presentation/components/HapticFeedbackEffect.android.kt:360
prSoundNamesis duplicated here and also in theprSoundIdsloader above. To avoid future mismatches between the sounds loaded into SoundPool vs those selected for playback, consider extracting a shared constant/list and referencing it in both places.
val effect = when (event) {
is HapticEvent.REP_COMPLETED -> {
// Light, quick click for each rep
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| /** | ||
| * Get a random badge celebration sound name. | ||
| */ | ||
| private fun getRandomBadgeSound(): String { | ||
| val badgeSoundNames = getBadgeSoundNames() | ||
| return badgeSoundNames[Random.nextInt(badgeSoundNames.size)] | ||
| } | ||
| private fun getRandomBadgeSound(): String = | ||
| BADGE_SOUND_NAMES[Random.nextInt(BADGE_SOUND_NAMES.size)] |
There was a problem hiding this comment.
Implemented in commit 09d3b42. HapticFeedbackEffect.android.kt now uses shared BADGE_SOUND_NAMES/PR_SOUND_NAMES constants for both SoundPool loading and random selection, so the badge/PR lists are no longer duplicated.
|
Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details. Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
Code Review SummaryStatus: 2 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
Other Observations (not in diff)Issues found in unchanged code that cannot receive inline comments: The typos at lines 328 and 330 were pre-existing in Files Reviewed (1 files)
Reviewed by laguna-m.1-20260312:free · 1,967,425 tokens |
* Refresh Smart Insights now timestamp on fetch (#412) * Refresh time-sensitive Smart Insights and recompute on clock updates (#418) * Align Smart Insights nowMs refresh with fetch and timer * Stabilize Smart Insights timer effect lifecycle * Remove periodic SmartInsights timer refresh loop * Harden SmartInsights loading state during refresh * Avoid mutating compose state from IO dispatcher * Align SmartInsights query and weekly-volume computation to shared nowMs (Option A) (#413) * Align insights query and weekly volume with shared nowMs * Harden insights anchor timestamp lifecycle * Fix SmartInsights loading guard for null anchor timestamp * Reset insights anchor before each profile refresh * Expand SmartSuggestions muscle-group taxonomy and add unknown-group fallback tracking (#415) * Expand muscle-group aliases and track unknown fallback usage * Refine unknown muscle-group fallback tracking * Keep SmartSuggestions fallback tracking stateless * Resolve review nits in muscle-group fallback docs/tests * Standardize inclusive cutoff policy (>=) in ReadinessEngine and add boundary tests (#416) * Define inclusive readiness cutoff policy and add boundary tests * Refine readiness boundary tests and add exclusion coverage * Address review cleanup in readiness boundary tests * Centralize inclusive readiness cutoff check * Unify insights into Snapshot→Trends→Diagnostics→Actions and add per-card metadata (#417) * Refactor analytics insights into unified hierarchy * Address review feedback on insight timeframe badges * Resolve review feedback on insights cards and cleanup * Resolve follow-up comments on readiness insight presentation * Use average intensity for Time-of-Day insights with confidence gating (#414) * Refine time-of-day insight to average intensity with confidence gating * Align time insight session-threshold copy with confidence gating * Add time-of-day tests for distinct-day gating and intensity scoring * Generalize time-window confidence copy across locales * Codex/fix high priority bug in insightstab.kt (#420) * Refactor analytics insights into unified hierarchy * Address review feedback on insight timeframe badges * Resolve review feedback on insights cards and cleanup * Resolve follow-up comments on readiness insight presentation * Fix missing RoundedCornerShape import in InsightsTab * Refresh Smart Insights now timestamp on fetch (#419) * Refresh Smart Insights now timestamp on fetch * Fix Compose state mutation in IO dispatcher and reset loading state on profile change Agent-Logs-Url: https://github.com/9thLevelSoftware/Project-Phoenix-MP/sessions/23e4c8a0-93e7-428f-8d00-b84a01429a58 Co-authored-by: 9thLevelSoftware <69057727+9thLevelSoftware@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> * fix(rep-counter): ignore firmware warmup sync for variable warmup sets (#422) * Request transient duck audio focus for Android workout cues (#423) * Fix Android cue audio precedence over background music * Fix audio focus release timing for workout cues * Align audio focus attributes with playback usage * Add focusChangeListener as identity token in requestTransientDuckAudioFocus Agent-Logs-Url: https://github.com/9thLevelSoftware/Project-Phoenix-MP/sessions/1d2923b8-cdaf-4080-bc22-8839a6940e94 Co-authored-by: 9thLevelSoftware <69057727+9thLevelSoftware@users.noreply.github.com> * Add focusHoldDurationMs helper and use event-specific delay in abandonAfterDelay Agent-Logs-Url: https://github.com/9thLevelSoftware/Project-Phoenix-MP/sessions/cd565f28-356a-4acd-9102-45b2602e7e02 Co-authored-by: 9thLevelSoftware <69057727+9thLevelSoftware@users.noreply.github.com> * Tie ducking focus hold to cue duration metadata --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> * build: update dependencies and add device metadata to Health Connect Update Health Connect to the stable 1.1.0 release and include device attribution in synced exercise records. - **Health**: Update `androidx-health-connect` to `1.1.0` stable and remove alpha-pinning workarounds in `shared/build.gradle.kts`. - **Health**: Implement `Metadata.activelyRecorded` for `ExerciseSessionRecord` and `TotalCaloriesBurnedRecord` to identify the "Vitruvian Trainer" device. - **Versions**: Upgrade Kotlin to `2.3.21`, AGP to `9.2.1`, and Gradle to `9.5.0`. - **Versions**: Bump Compose BOM to `2026.05.00`, Compose Navigation to `2.9.8`, and Ktor to `3.4.3`. * Revert audio focus duck attempt (#423) — restore audio output for #421 (#428) * Revert "Request transient duck audio focus for Android workout cues (#423)" This reverts commit 6371274. * fix(audio): release MediaPlayer on exception, dedupe sound name lists Address gemini-code-assist review on PR #428: - playWithMediaPlayer: hoist `mediaPlayer` declaration out of the `try` block and release it from `catch`. Previous structure leaked the native player if setVolume/setOnCompletionListener/start threw. - Extract BADGE_SOUND_NAMES and PR_SOUND_NAMES to private top-level vals. Eliminates duplication between the SoundPool-loading `remember` block and getRandomBadgeSound/getRandomPRSound, and removes per-call list allocations on the random helpers. --------- Co-authored-by: Claude <noreply@anthropic.com> * Restore Android workout cue audio routing and ignore code-review artifacts (#429) * Address PR #429 review feedback: route workout cues to STREAM_MUSIC (#430) * Address PR #429 review feedback: route cues to STREAM_MUSIC P1 (chatgpt-codex-connector / gemini-code-assist): the previous change to USAGE_ASSISTANCE_SONIFICATION routed cues to STREAM_SYSTEM, breaking the existing volumeControlStream = STREAM_MUSIC contract. Hardware volume buttons would no longer adjust workout cue loudness, leaving users with silent or unbalanced cues. - Switch STANDARD_ANDROID_CUE_USAGE to USAGE_MEDIA so SoundPool and MediaPlayer both route through STREAM_MUSIC, matching the activity's volumeControlStream and the existing media-volume warning logic. - Update warnIfMediaVolumeMuted comment/message to reflect USAGE_MEDIA. - Reword MainActivity volumeControlStream comment to reflect the actual scope (entire activity lifetime, not just the workout UI). - Update structural test assertions to match USAGE_MEDIA and add a guard preventing accidental reintroduction of USAGE_ASSISTANCE_SONIFICATION. - Replace silent fallbacks in HapticFeedbackAudioRoutingGuardTest with explicit checks so a missing project root, source file, or raw resource directory fails with an actionable message instead of an opaque FileNotFoundException or "every cue missing" report. * Address PR #430 review feedback: harden cue source parser - gemini-code-assist (high) / chatgpt-codex-connector (P2): the multi-line branch in soundNamesInConstantList searched for `\n)`, which only matches a closing paren at column 0. Replace with a simple `indexOf(")", start)`. Sound names only contain `[a-z0-9_]` so the first `)` after `start` is reliably the listOf(...) closer regardless of single-line, multi-line, or indented formatting. - gemini-code-assist (medium): extract the rep-count range (`(1..N).mapNotNull`) from the source instead of duplicating `1..25`, so the guard stays in sync if the upper bound changes. Skipped: the suggestion to skip SoundPool initialization on Fire OS (pre-existing behavior, out of scope for this PR's audio-routing focus). --------- Co-authored-by: Claude <noreply@anthropic.com> * Fix top-level superset reorder persistence and restore audio routing * Fix Android cue packaging and audio routing (#431) * Harden Compose layouts for iOS accessibility scaling (#432) * Harden iOS accessibility layouts for issue 389 * Fix cue audio resource lookup in release builds * Fix per-cable weight display and softMax handling (#433) * fix: preserve stall detection in portal sync * chore: remove stale generated artifacts * Codex/stu experiment (#436) * Fix per-cable weight display and softMax handling * Fix BLE force target to keep progression separate * Replace Just Lift auto-detection with post-set exercise tagging (#435) * Fix Just Lift retag PR tracking and completed set sync * fix: prevent Just Lift retag PR duplication * test: align AMRAP target weight expectation --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: Claude <noreply@anthropic.com>
* Refresh Smart Insights now timestamp on fetch (#412) * Refresh time-sensitive Smart Insights and recompute on clock updates (#418) * Align Smart Insights nowMs refresh with fetch and timer * Stabilize Smart Insights timer effect lifecycle * Remove periodic SmartInsights timer refresh loop * Harden SmartInsights loading state during refresh * Avoid mutating compose state from IO dispatcher * Align SmartInsights query and weekly-volume computation to shared nowMs (Option A) (#413) * Align insights query and weekly volume with shared nowMs * Harden insights anchor timestamp lifecycle * Fix SmartInsights loading guard for null anchor timestamp * Reset insights anchor before each profile refresh * Expand SmartSuggestions muscle-group taxonomy and add unknown-group fallback tracking (#415) * Expand muscle-group aliases and track unknown fallback usage * Refine unknown muscle-group fallback tracking * Keep SmartSuggestions fallback tracking stateless * Resolve review nits in muscle-group fallback docs/tests * Standardize inclusive cutoff policy (>=) in ReadinessEngine and add boundary tests (#416) * Define inclusive readiness cutoff policy and add boundary tests * Refine readiness boundary tests and add exclusion coverage * Address review cleanup in readiness boundary tests * Centralize inclusive readiness cutoff check * Unify insights into Snapshot→Trends→Diagnostics→Actions and add per-card metadata (#417) * Refactor analytics insights into unified hierarchy * Address review feedback on insight timeframe badges * Resolve review feedback on insights cards and cleanup * Resolve follow-up comments on readiness insight presentation * Use average intensity for Time-of-Day insights with confidence gating (#414) * Refine time-of-day insight to average intensity with confidence gating * Align time insight session-threshold copy with confidence gating * Add time-of-day tests for distinct-day gating and intensity scoring * Generalize time-window confidence copy across locales * Codex/fix high priority bug in insightstab.kt (#420) * Refactor analytics insights into unified hierarchy * Address review feedback on insight timeframe badges * Resolve review feedback on insights cards and cleanup * Resolve follow-up comments on readiness insight presentation * Fix missing RoundedCornerShape import in InsightsTab * Refresh Smart Insights now timestamp on fetch (#419) * Refresh Smart Insights now timestamp on fetch * Fix Compose state mutation in IO dispatcher and reset loading state on profile change Agent-Logs-Url: https://github.com/9thLevelSoftware/Project-Phoenix-MP/sessions/23e4c8a0-93e7-428f-8d00-b84a01429a58 Co-authored-by: 9thLevelSoftware <69057727+9thLevelSoftware@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> * fix(rep-counter): ignore firmware warmup sync for variable warmup sets (#422) * Request transient duck audio focus for Android workout cues (#423) * Fix Android cue audio precedence over background music * Fix audio focus release timing for workout cues * Align audio focus attributes with playback usage * Add focusChangeListener as identity token in requestTransientDuckAudioFocus Agent-Logs-Url: https://github.com/9thLevelSoftware/Project-Phoenix-MP/sessions/1d2923b8-cdaf-4080-bc22-8839a6940e94 Co-authored-by: 9thLevelSoftware <69057727+9thLevelSoftware@users.noreply.github.com> * Add focusHoldDurationMs helper and use event-specific delay in abandonAfterDelay Agent-Logs-Url: https://github.com/9thLevelSoftware/Project-Phoenix-MP/sessions/cd565f28-356a-4acd-9102-45b2602e7e02 Co-authored-by: 9thLevelSoftware <69057727+9thLevelSoftware@users.noreply.github.com> * Tie ducking focus hold to cue duration metadata --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> * build: update dependencies and add device metadata to Health Connect Update Health Connect to the stable 1.1.0 release and include device attribution in synced exercise records. - **Health**: Update `androidx-health-connect` to `1.1.0` stable and remove alpha-pinning workarounds in `shared/build.gradle.kts`. - **Health**: Implement `Metadata.activelyRecorded` for `ExerciseSessionRecord` and `TotalCaloriesBurnedRecord` to identify the "Vitruvian Trainer" device. - **Versions**: Upgrade Kotlin to `2.3.21`, AGP to `9.2.1`, and Gradle to `9.5.0`. - **Versions**: Bump Compose BOM to `2026.05.00`, Compose Navigation to `2.9.8`, and Ktor to `3.4.3`. * Revert audio focus duck attempt (#423) — restore audio output for #421 (#428) * Revert "Request transient duck audio focus for Android workout cues (#423)" This reverts commit 6371274. * fix(audio): release MediaPlayer on exception, dedupe sound name lists Address gemini-code-assist review on PR #428: - playWithMediaPlayer: hoist `mediaPlayer` declaration out of the `try` block and release it from `catch`. Previous structure leaked the native player if setVolume/setOnCompletionListener/start threw. - Extract BADGE_SOUND_NAMES and PR_SOUND_NAMES to private top-level vals. Eliminates duplication between the SoundPool-loading `remember` block and getRandomBadgeSound/getRandomPRSound, and removes per-call list allocations on the random helpers. --------- Co-authored-by: Claude <noreply@anthropic.com> * Restore Android workout cue audio routing and ignore code-review artifacts (#429) * Address PR #429 review feedback: route workout cues to STREAM_MUSIC (#430) * Address PR #429 review feedback: route cues to STREAM_MUSIC P1 (chatgpt-codex-connector / gemini-code-assist): the previous change to USAGE_ASSISTANCE_SONIFICATION routed cues to STREAM_SYSTEM, breaking the existing volumeControlStream = STREAM_MUSIC contract. Hardware volume buttons would no longer adjust workout cue loudness, leaving users with silent or unbalanced cues. - Switch STANDARD_ANDROID_CUE_USAGE to USAGE_MEDIA so SoundPool and MediaPlayer both route through STREAM_MUSIC, matching the activity's volumeControlStream and the existing media-volume warning logic. - Update warnIfMediaVolumeMuted comment/message to reflect USAGE_MEDIA. - Reword MainActivity volumeControlStream comment to reflect the actual scope (entire activity lifetime, not just the workout UI). - Update structural test assertions to match USAGE_MEDIA and add a guard preventing accidental reintroduction of USAGE_ASSISTANCE_SONIFICATION. - Replace silent fallbacks in HapticFeedbackAudioRoutingGuardTest with explicit checks so a missing project root, source file, or raw resource directory fails with an actionable message instead of an opaque FileNotFoundException or "every cue missing" report. * Address PR #430 review feedback: harden cue source parser - gemini-code-assist (high) / chatgpt-codex-connector (P2): the multi-line branch in soundNamesInConstantList searched for `\n)`, which only matches a closing paren at column 0. Replace with a simple `indexOf(")", start)`. Sound names only contain `[a-z0-9_]` so the first `)` after `start` is reliably the listOf(...) closer regardless of single-line, multi-line, or indented formatting. - gemini-code-assist (medium): extract the rep-count range (`(1..N).mapNotNull`) from the source instead of duplicating `1..25`, so the guard stays in sync if the upper bound changes. Skipped: the suggestion to skip SoundPool initialization on Fire OS (pre-existing behavior, out of scope for this PR's audio-routing focus). --------- Co-authored-by: Claude <noreply@anthropic.com> * Fix top-level superset reorder persistence and restore audio routing * Fix Android cue packaging and audio routing (#431) * Harden Compose layouts for iOS accessibility scaling (#432) * Harden iOS accessibility layouts for issue 389 * Fix cue audio resource lookup in release builds * Fix per-cable weight display and softMax handling * Fix per-cable weight display and softMax handling (#433) * Fix BLE force target to keep progression separate * fix: preserve stall detection in portal sync * chore: remove stale generated artifacts * Replace Just Lift auto-detection with post-set exercise tagging (#435) * Fix Just Lift retag PR tracking and completed set sync * fix: prevent Just Lift retag PR duplication * test: align AMRAP target weight expectation * fix: test legacy Kable GATT writes for Pixel starts * Codex/issue 333 ready gate test (#438) * fix: gate BLE ready state for Pixel diagnostics * fix(ble): optimize connection flow and improve Pixel device compatibility Refine BLE communication by introducing device-specific timing adjustments for Google Pixel hardware and streamlining the command observation sequence. - **DeviceInfo**: Add `isPixel()` and `isSamsung()` to the common `DeviceInfo` interface to support platform-specific hardware workarounds. - **BLE**: Implement observation for the `SAMPLE` characteristic to support raw monitor packets without opcode prefixes. - **BLE**: Update `parseMetricsPacket` in `KableBleRepository` to handle both opcoded (0x01) and raw monitor data payloads. - **BLE**: Introduce a 500ms `preDelayMs` on Pixel devices after sending program configurations to improve diagnostic read reliability. - **Session**: Remove the redundant manual `START` command (0x03) from `ActiveSessionEngine` prior to starting workout polling. - **Kable Patch**: Optimize `Connection.kt` by removing an unnecessary `async`/`await` wrapper when receiving GATT responses, reducing coroutine overhead. --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: Claude <noreply@anthropic.com>
Summary
Reverts PR #423 (
6371274) — the "request transient duck audio focus" attempt at fixing #421.After #423 shipped, #421 was retested and reopened with a worse symptom: the in-app Test Sounds button produces haptic feedback but no audio at all across multiple different sound files (
chirpchirp,beepboop,rep_05,boopbeepbeep). Owner's diagnosis on the issue: "refactoring broke something with the audio prioritization."Diagnostic findings that motivated the revert
res/raw/audit: all 66 .ogg files exist; every name referenced inHapticFeedbackEffect.android.ktresolves. Not a missing-resource bug.MainViewModel.testSounds()emits 4 events through the same globalHapticFeedbackEffecthosted inEnhancedMainScreen. Three of four have haptics; haptics fire, audio doesn't. Failure is downstream of dispatch — in the SoundPool/MediaPlayer/AudioFocus stack.SoundPool.playandplayWithMediaPlayercall in a transient-duck audio focus request and ties focus-hold duration to a freshly-measured cue duration. It is also the most invasive (+163/-22).What this PR keeps
ec0f673) —volumeControlStream = STREAM_MUSICinMainActivity,warnIfMediaVolumeMuteddiagnostic,loadedSoundsasync-load gate, improved error logging. All independently useful.FINAL_REP,chirpchirpforREP_COMPLETED,VELOCITY_THRESHOLD_REACHED,HapticEventAudioTest.What this PR removes
requestTransientDuckAudioFocushelper +AudioFocusSessioninterfacefocusHoldDurationMs/getSoundDurationMs/soundDurationCacheMsabandonAfterDelay/useDurationOrZeroplaySoundandplayWithMediaPlayerSingle file changed:
shared/src/androidMain/.../HapticFeedbackEffect.android.kt(-163/+22).Expected effect
Test plan
chirpchirp,beepboop,rep_05,boopbeepbeep)../gradlew :androidApp:assembleDebugbuilds clean../gradlew :shared:testDebugUnitTest—HapticEventAudioTestshould still pass (only Android playback path was touched).Closes/relates: #421 (regression rollback, not full fix).
Generated by Claude Code
Perform an AI-assisted review on