docs: push notifications fcm android

[berendsliedrecht]

trafficstars

• The same PR basically as #699 but for fcm

[TelegramSam]

Why include android in the protocol name? Doesn't FCM work with iOS as well?

[berendsliedrecht]

Why include android in the protocol name? Doesn't FCM work with iOS as well?

Yes Fcm works for iOS as well. However, @TimoGlastra and I had a brief discussion about it yesterday and a holder can choose to use apns for iOS and fcm for android. Keeping the protocol like push-notifications-fcm would imply, during feature discovery, that fcm is supported for iOS and Android. Separating them into push-notifications-fcm-android and push-notifications-fcm-ios would fix that issue, but it would, with all services, create an insane amount of RFCs. push-notifications-apns only supports iOS so it does not have to mention iOS explicitly and push-notifications-expo, for example, is one protocol for both, so they can be combined.

Hopefully my explanation is clear enough.

I am very much open to better ideas to clean it up, but this is what we came up with if we wanted to do specific feature-discovery for each service.

[genaris]

Why include android in the protocol name? Doesn't FCM work with iOS as well?

Yes Fcm works for iOS as well. However, @TimoGlastra and I had a brief discussion about it yesterday and a holder can choose to use apns for iOS and fcm for android. Keeping the protocol like push-notifications-fcm would imply, during feature discovery, that fcm is supported for iOS and Android. Separating them into push-notifications-fcm-android and push-notifications-fcm-ios would fix that issue, but it would, with all services, create an insane amount of RFCs. push-notifications-apns only supports iOS so it does not have to mention iOS explicitly and push-notifications-expo, for example, is one protocol for both, so they can be combined.

Hopefully my explanation is clear enough.

I am very much open to better ideas to clean it up, but this is what we came up with if we wanted to do specific feature-discovery for each service.

I missed some parts from yesterday's WG call, so probably you already discussed it, but could it be possible to use "roles" field from Discover Features protocol to indicate target platforms supported (e.g. notification-sender-android, notification-sender-ios, notification-sender-web)?

[berendsliedrecht]

@TelegramSam I have resolved the feedback from the WG call where this was discussed.

[TimoGlastra]

This LGTM, it has been open for a while and is basically a clone of #699, but for Android.

I think the suggestion from @genaris makes sense, maybe you can integrate this @blu3beri? I'm also fine with the state as is

[berendsliedrecht]

Yeah the roles, like @genaris suggested, makes sense here. Are there some conventions for default roles or is it always explicit?

[genaris]

I have a few more comments about this after having some fun with FCM on Android and iOS devices:

• Notification handling is quite different in both platforms, so maybe it could be useful for the notification sender to know receiver's OS. There was a device_platform field in Set Native Device Info message of the good-old (!) push-notifications plug-in for AFJ, but this was dropped in APNs and FCM-specific protocols. Probably we can re-add this information by using roles for receiver as well (e.g. notification-receiver-android, notification-receiver-ios, notification-receiver-web)
• In the Push Notification body It could be useful to support arrays for message_id and message_tag, so the notification sender can send a single notification for several messages, to prevent flooding in case of multiple messages that arrive in a short timeframe (e.g. chat messaging).

[TimoGlastra]

Notification handling is quite different in both platforms, so maybe it could be useful for the notification sender to know receiver's OS. There was a device_platform field in Set Native Device Info message of the good-old (!) push-notifications plug-in for AFJ, but this was dropped in APNs and FCM-specific protocols. Probably we can re-add this information by using roles for receiver as well (e.g. notification-receiver-android, notification-receiver-ios, notification-receiver-web)

In this case it feels we're starting to misuse the roles. In the case of the notification sender including the service makes more sense to me as it's about discoverability (what services do you support?), while in the mobile case it's not "What Operating systems do you support?". So in this case I'd vote for adding the device_platform back in.

In the Push Notification body It could be useful to support arrays for message_id and message_tag, so the notification sender can send a single notification for several messages, to prevent flooding in case of multiple messages that arrive in a short timeframe (e.g. chat messaging).

Agreed! We should also change it in the APNS RFC. Although the APNS protocols not used a lot yet, we should bump it to 1.1 and make the message_id and message_tag properties accept both a string (single) or an array value. ~This way we don't create breaking changes.~ @blu3beri can you pick that up?

Edit: That's not true, it will create a breaking change. Should we add message_ids and message_tags properties?

[rodolfomiranda]

How does the client know if the mediator has the app registered in firebase? Seems that having different protocols for apns and fcm (and why not AWS SNS) to discover the provider does not guarantee that the mediator will be able to send a push notification back to the agent application.

[genaris]

How does the client know if the mediator has the app registered in firebase? Seems that having different protocols for apns and fcm (and why not AWS SNS) to discover the provider does not guarantee that the mediator will be able to send a push notification back to the agent application.

I think @rodolfomiranda has a good point here. AFAIK in FCM it's possible for the server to send a dry run API call to check whether a Registration Token is valid or not (this works for both the cases where token is invalid and token is valid but the app not registered for mediator), so maybe it could be enough to send an ack for confirmation or error reporting.

[berendsliedrecht]

@genaris @TelegramSam @TimoGlastra

I have resolved the feedback, if this is approved I will do the same for the apns RFC.

[TelegramSam]

Discussed WG 20221109. No objections to merge. James will add comment about a typo.