Skip to content

Conversation

@nift4
Copy link
Contributor

@nift4 nift4 commented Oct 9, 2025

I was debugging start commands I shouldn't be recieving, and the startSelfIntent with empty action was sort of confusing because it wasn't immediately obvious where it came from. Adding a unique action string doesn't have any ill effects and allows easily identifying these intents in onStartCommand().

mainHandler = Util.createHandler(Looper.getMainLooper(), /* callback= */ this);
mainExecutor = (runnable) -> Util.postOrRun(mainHandler, runnable);
startSelfIntent = new Intent(mediaSessionService, mediaSessionService.getClass());
startSelfIntent.setAction("androidx.media3.session.MediaNotificationManager.START_SELF_INTENT");
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's call this MediaSessionService. Agreed the origin is the MediaNotificationManager.

And the I think it would make sense to add this as a public constant to MediaSessionService, so that an app would be able to compare the action easily and then discard the intent as a no-op.

WDYT?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi, thanks for the feedback. I agree with your idea and did it for this and the other missing Intent action.

@nift4 nift4 force-pushed the notifstartaction branch 2 times, most recently from 7edcad7 to b32a5ba Compare October 15, 2025 16:16
@marcbaechinger
Copy link
Contributor

Thanks. I will take that.

"Override this method if this service also needs to handle actions other than those mentioned above."

Just as an aside. Above API 31 the only action that should arrive in onStartCommand is the start self action. From API 31 and below they are additional actions coming from the notification as a pending intent that is sent to the service as well. It's API 31 because of [1].

As a developer I would actually never need to override this method I think. If you do that you can, but you would be responsible for doing this in a way that doesn't break the state handling of the service.

If an app is sending such Intent it's not just sending messages but it may change the state of the service. It would be beneficial if the library is managing this by default and an app is not interfereing with this. Sending an Intent to the service is not a means of sending messages. An app can use custom actions for that instead. The advantage of this is that the library is able to handle the bound/started state which is why Intent should never be used by an app.

If it was about me, startCommand would be final. But that ship has sailed I think as we released that like this with 1.0.

I'm going to adjust the documentation you added accordingly. Please refrain from pushing further changes.

[1] https://developer.android.com/about/versions/12/behavior-changes-12#foreground-service-launch-restrictions

I was debugging start commands I shouldn't be recieving, and the
startSelfIntent with empty action was sort of confusing because
it wasn't immediately obvious where it came from. Adding a unique
action string doesn't have any ill effects and allows easily
identifying these intents in onStartCommand().
@marcbaechinger
Copy link
Contributor

I'm going to send this for internal review now. You may see some more commits being added as I make changes in response to review feedback. Please refrain from pushing any more substantive changes as it will complicate the internal review - thanks!

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants