Add action to startSelfIntent to easily distinguish it as app

[nift4]

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]

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?

[nift4]

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

[marcbaechinger]

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

[marcbaechinger]

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!

[nift4]

Hi @marcbaechinger, is there anything blocking the merge of this change in internal review? Thanks.

[marcbaechinger]

Sorry for not giving you an update earlier.

We have quite a few changes in onStartCommand which will enable apps to more easily reuse the logic in there to send playback commands by Intent. We will continue discourage apps to override onStartCommand as we aim for extending the public API to use the existing Intents.

I will see that there is an API which will let you detect start self Intents, but I don't think we will exactly follow this PR here. I suggest we keep the PR until we have submitted the changes I'm talking about.

Thanks for your patience and sorry for the wait.