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

Migrate WebSockets to new SocketApi from SDK #1456

Merged
merged 3 commits into from
Mar 28, 2022

Conversation

nielsvanvelzen
Copy link
Member

@nielsvanvelzen nielsvanvelzen commented Feb 19, 2022

The new WebSocket code from the SDK is still in beta. The API should be finished so no changes are expected in the app. Some issues in de SDK right now include unnecessary reconnects on subscription changes and the lack of automatic reconnection logic. These will be fixed before SDK 1.2.0 is released as stable and app 0.14 is released in beta/stable. The current issues are minor and shouldn't prevent us contributors from using the master branch on our devices.

Changes

  • Add new SocketHandler to replace the TvApiEventListener
    • Used the new WebSocket code from the SDK
    • Fully rewritten in Kotlin, more readable and less code now
    • Doesn't support screen mirroring as the ItemLauncher does not support the SDK
  • Add endPlayback overload that can close the activity
    • It's a hack, but we don't want to keep track of activities in the SocketHandler
    • Still way better then it was before
  • Move session capability announcement from ApiBinder to new SocketHandler
  • Remove currentActivity stuff from TvApp.java

Documentation for the WebSocket API is available here: https://github.com/jellyfin/jellyfin-sdk-kotlin/blob/master/docs/websockets.md

Issues

@jellyfin-bot jellyfin-bot added the merge conflict Conflicts prevent merging label Feb 19, 2022
@jellyfin-bot jellyfin-bot removed the merge conflict Conflicts prevent merging label Feb 19, 2022
@nielsvanvelzen nielsvanvelzen force-pushed the sdk-websockets branch 2 times, most recently from 49c9ab6 to 59c10c4 Compare February 20, 2022 13:16
@nielsvanvelzen nielsvanvelzen force-pushed the sdk-websockets branch 2 times, most recently from 573ec37 to 5c4236e Compare March 3, 2022 13:38
@nielsvanvelzen nielsvanvelzen added the next-release Pull request related to a future release which is not being worked on yet label Mar 4, 2022
@nielsvanvelzen nielsvanvelzen removed the next-release Pull request related to a future release which is not being worked on yet label Mar 17, 2022
@nielsvanvelzen nielsvanvelzen force-pushed the sdk-websockets branch 2 times, most recently from 73a464d to 773cd07 Compare March 26, 2022 18:30
Copy link
Contributor

@DavidFair DavidFair left a comment

Choose a reason for hiding this comment

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

Some general design points whilst this is a draft PR (so I'm hoping open to change).
I'm looking forward as the project gradually migrates to the new SDK though, nice work 👍

@nielsvanvelzen nielsvanvelzen merged commit 0a423e8 into jellyfin:master Mar 28, 2022
@nielsvanvelzen nielsvanvelzen deleted the sdk-websockets branch March 28, 2022 19:44
@nielsvanvelzen nielsvanvelzen added the release-highlight Pull request might be suitable for mentioning in the release blog post label May 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-highlight Pull request might be suitable for mentioning in the release blog post
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants