Skip to content

fixes random ForegroundService crash#2818

Closed
AbhinandAK350 wants to merge 1 commit intoMetrolistGroup:mainfrom
AbhinandAK350:main
Closed

fixes random ForegroundService crash#2818
AbhinandAK350 wants to merge 1 commit intoMetrolistGroup:mainfrom
AbhinandAK350:main

Conversation

@AbhinandAK350
Copy link
Copy Markdown
Contributor

Removes the conditional startForegroundService calls in widget receivers, simplifying them to just startService. This is safe because the service already calls startForeground itself when appropriate.

Removes the conditional `startForegroundService` calls in widget receivers, simplifying them to just `startService`. This is safe because the service already calls `startForeground` itself when appropriate.
@adrielGGmotion
Copy link
Copy Markdown
Collaborator

@AbhinandAK350 Build faied because of unresolved references, make sure the app builds next time before you create an PR and that your fix indeed works

Copy link
Copy Markdown
Contributor

@maximillions-dev maximillions-dev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@AbhinandAK350 Hey, thanks for looking into the ForegroundService crash — it's a real issue and the core idea of switching to startService in the widget receivers (since they already check MusicService.isRunning) is actually sound.
That said, I don't think we can merge this as-is. A few things:

  1. The build is broken. There are unresolved references because some Player.* constants were unqualified without adding the corresponding static imports (e.g. STATE_ENDED, EVENT_MEDIA_ITEM_TRANSITION). Please always build before opening a PR.
  2. There's a bug introduced. The val previousShuffleEnabled = player.shuffleModeEnabled removal left a bare player.shuffleModeEnabled expression that does nothing. That line should just be deleted entirely.
  3. Duplicate isRunning = false. onDestroy already sets isRunning = false at the top — the PR adds a second one before super.onDestroy(). Pick one spot, don't have both.
  4. Too much scope creep. About half the diff is unrelated cleanup — FQN shortening, property access syntax for shuffleOrder, unused destructuring, a lint suppression on MediaButtonReceiver. These aren't bad changes on their own, but they don't belong in a PR titled "fixes random ForegroundService crash". It makes it harder to review and harder to bisect if something breaks.
  5. Commit message says "remove redundant receiver" but no receiver is removed.
    The actual fix (3 startForegroundServicestartService swaps in the widget receivers) is like 6 lines. I'd suggest stripping this down to just that, making sure it builds, and resubmitting. The cleanup stuff can go in a separate PR if you want.

@AbhinandAK350
Copy link
Copy Markdown
Contributor Author

Sure

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants