Skip to content

[WIP] WebSocket #46

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

Open
wants to merge 57 commits into
base: main
Choose a base branch
from
Open

[WIP] WebSocket #46

wants to merge 57 commits into from

Conversation

JPKribs
Copy link
Member

@JPKribs JPKribs commented May 10, 2025

Summary

Create a JellyfinSocket item for WebSocket activity. Right now, this is in a fully functional, but messy state but I thought I would make a draft PR just to signal this was something I was working on. The goal is to get this working, add it to the SDK, and eventually be able to work on this: jellyfin/Swiftfin#408

WIP

This is currently a work in progress. The current version is functional but the following still need work:


  • Either remove logging or add a more permanent logging solution

  • Resolve all the Swift 6 Sendable violations (likely threading) - I think I did this?
  • Code Cleanup (potential threading changes, redundant functions, socket folder for enums, etc)
  • Comment Cleanup
  • String Typing / Remove hard-coded strings
  • Figure out the initial connection issues - Initial connection is finicky unless connecting directly from JellyfinClient initialization
  • Figure out the best, most standard way to initialize and use the SDK
  • Cleanup / Structure of all Websocket file(s) in the SDK
  • Allow subscriptions to higher volume endpoints
  • Add Usage Documentation in the README

Why Not NIO?

Not against it. Just getting this error:

PendingDatagramWritesManager.swift:291:33 Property 'msg_len' is not available due to missing import of defining module 'CNIODarwin'

I think the structure, or at least what I learned from doing this, should be reusable if we decide NIO is the best route. I would just need to get past that CNIODarwin dependency that only shows up on iOS.


As a note - WebSocket (at least for Jellyfin) are named from the Server's perspective. Inbound means inbound into Server.

@JPKribs JPKribs changed the title [WIP] Web Socket [WIP] WebSocket May 12, 2025
@JPKribs
Copy link
Member Author

JPKribs commented May 12, 2025

Hey @LePips no rush at all! I'm getting really close to being ready. I think the only thing that isn't working is the subscription for things like live tracking ActivityLogs and Sessions. I think I figured this one out on my flight. At this point, I think it's just polish. I have some weird threading that solved some issues for me that I need to lock down if they're necessary or not.

I wanted to check in and make sure this structure/usage made sense to you. I've never build something like this before so I just wanted to be sure this structure wasn't going to cause headaches.

Let me know what you think!

/// Create a WebSocket instance with all available parameters
let socket = JellyfinSocket(
    client: client,
    userID: user.id,
    isSupportsMediaControl: true,
    supportedCommands: [.displayMessage, .play, .pause],
    logLevel: .debug
)

/// Observe socket state changes
let stateSubscription = socket.$state
    .receive(on: DispatchQueue.main)
    .sink { state in
        switch state {
        case .idle:
            print("Socket is idle")
        case .connecting:
            print("Connecting...")
        case .connected(let url):
            print("Connected to: \(url)")
        case .disconnecting:
            print("Disconnecting...")
        case .closed(let error):
            print("Closed: \(String(describing: error))")
        case .error(let error):
            print("Socket error: \(error)")
        }
    }

/// Observe parsed server messages
let messageSubscription = socket.messages
    .receive(on: DispatchQueue.main)
    .sink { message in
        switch message {
        case .sessionsMessage(let msg):
            print("Received session update: \(msg)")
        case .outboundKeepAliveMessage:
            print("Received keep-alive pong")
        default:
            break
        }
    }

/// Connect the socket
socket.connect()

/// Subscribe to sessions feed immediately with updates every 2 seconds
socket.subscribe(.sessions(initialDelayMs: 0, intervalMs: 2000))

/// Later, unsubscribe
socket.unsubscribe(.sessions())

/// Gracefully disconnect (optional; also triggered by deinit)
socket.disconnect()

The primary things that I want to confirm look good:

  • Is the sink method a good way of handling incoming messages?
  • Right now, everything is an OutboundWebSocketMessage. Which means, in order to get to the actual message data and details, you would need to unwrap it. It's easy enough to deal with casting, but it's not super intuitive IMO. Is there a different/better way to handle this?
  • I handle all of the keep alive messages as part of the socket class. Do we want to do that there or should the user handle their own keep alive?

@JPKribs JPKribs marked this pull request as ready for review May 13, 2025 07:35
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.

1 participant