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

Not being able to use signalclient after migrate from v1 to v2 #534

Open
UladzimirTrehubenka opened this issue Sep 26, 2024 · 6 comments · May be fixed by #540
Open

Not being able to use signalclient after migrate from v1 to v2 #534

UladzimirTrehubenka opened this issue Sep 26, 2024 · 6 comments · May be fixed by #540

Comments

@UladzimirTrehubenka
Copy link

Observed during migrate from v1.1.8 to v2.2.1:

import lksdk "github.com/livekit/server-sdk-go"
// v1.1.8 code
client := lksdk.NewSignalClient()
res, err := client.Join(url, token, &lksdk.ConnectParams{AutoSubscribe: true})

in #392 we can see that

func (c *SignalClient) Join(urlPrefix string, token string, params *ConnectParams) (*livekit.JoinResponse, error)

became

func (c *SignalClient) Join(urlPrefix string, token string, params connectParams) (*livekit.JoinResponse, error)

So for now function Join() cannot be used outside (only inside the SDK itself),
actually this is broken design - how exported method would use private structure?!

@davidzhao
Copy link
Member

we don't recommend using SignalClient by itself outside of the SDK itself. what is your use case?

@UladzimirTrehubenka
Copy link
Author

Custom app server: mobile -> app server -> livekit instead direct mobile -> livekit

@UladzimirTrehubenka
Copy link
Author

UladzimirTrehubenka commented Sep 27, 2024

Is it possible to add something like

func (c *SignalClient) JoinWithAutoSubscribe(urlPrefix string, token string, autoSubscribe bool) (*livekit.JoinResponse, error) {
	params := connectParams{AutoSubscribe: autoSubscribe}
	return c.Join(urlPrefix, token, params)
}

This is how we are using v2 for now with patch vendor folder during build.

@davidzhao
Copy link
Member

in that case, I think it's ok to make connectParams public. would you like to open a PR renaming it to SignalClientConnectParams ?

@UladzimirTrehubenka
Copy link
Author

UladzimirTrehubenka commented Oct 7, 2024

Created #540

@UladzimirTrehubenka
Copy link
Author

@davidzhao could you please add reviewers?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants