Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add recommendations for mobile clients #1915

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

agx
Copy link

@agx agx commented Aug 4, 2024

This came out of a discussion at https://matrix.to/#/!XXSJTvRPInupfUgQVb:matrix.org/$IUx1flZV7iLWIviEeJWrmIw8LHgv4DhKGDKnO1hU688?via=pixie.town&via=matrix.org&via=element.io

When looking into adding UnifiedPush support to a client I was looking around for RFC style "Best Comon Practice" implementation hints but couldn't find any. Maybe it makes sense to add something to the documentation? I couldn't find a more sensible place than the spec if there is one please point me to it?

Pull Request Checklist

  • Pull request includes a changelog file (left out as this repo maybe isn't the right location to add this?)
  • Pull request includes a sign off
  • Pull request is classified as 'other changes'

@agx agx requested a review from a team as a code owner August 4, 2024 12:10
Copy link
Member

@anoadragon453 anoadragon453 left a comment

Choose a reason for hiding this comment

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

I think this may go better under https://spec.matrix.org/v1.11/client-server-api/#receiving-notifications.

@matrix-org/rust could I confirm whether this is the best practice for mobile developers to receive (non-)encrypted events?


In order to save bandwidth and battery mobile clients should:

- Use [push notifications][/client-server-api##push-notifications] to get notified about new events
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
- Use [push notifications][/client-server-api##push-notifications] to get notified about new events
- Use [push notifications][/client-server-api#push-notifications] to get notified about new events


- Use [push notifications][/client-server-api##push-notifications] to get notified about new events
- Disable continuous `/sync` calls
- Fetch the events indicted in the push notifications via the ``GET /rooms/{roomId}/event/{eventId}`` API call
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
- Fetch the events indicted in the push notifications via the ``GET /rooms/{roomId}/event/{eventId}`` API call
- Fetch the events included in the push notifications via the ``GET /rooms/{roomId}/event/{eventId}`` API call

Copy link
Member

Choose a reason for hiding this comment

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

Also, could you linkify GET /rooms/{roomId}/event/{eventId}? Likewise with /sync?

@bnjbvr
Copy link
Member

bnjbvr commented Aug 12, 2024

@matrix-org/rust could I confirm whether this is the best practice for mobile developers to receive (non-)encrypted events?

Yep, something like this. For what it's worth, instead of using /context or /event for retrieving the event, we're doing a limited sliding sync, because the two former endpoints don't work for stripped events (coming from invited rooms). And if the event was encrypted, we're indeed trying to run a sliding sync limited to the to-device and encryption extensions; this may change with the new evolution of the sliding sync protocol.

@richvdh
Copy link
Member

richvdh commented Nov 26, 2024

This feels like it has stalled. @agx: are you able to make the requested changes?

@agx
Copy link
Author

agx commented Nov 28, 2024

This feels like it has stalled. @agx: are you able to make the requested changes?

Yes, I hope to get to this in the not too distant future.

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.

4 participants