nikeee / TeamSpeak3QueryApi

.NET wrapper for the TeamSpeak 3 Query API
https://nikeee.github.io/TeamSpeak3QueryAPI
GNU General Public License v3.0
60 stars 16 forks source link

Added keep alive logic for the QueryClient and TeamSpeakClient #67

Closed Arefu closed 3 years ago

Arefu commented 4 years ago

Like you say here https://github.com/nikeee/TeamSpeak3QueryAPI/issues/65 I think this should do the trick nicely, I can't be sure that the queue will process, it'll fire every 5 minutes, I don't like the loop there all that much, but I can't think of a better way to do it without upping the .NET standard so we could use Timers on events instead.

What do you think?

lgund commented 4 years ago

Hey well done. Great idea I write my keep alive by myself but it´s better with this solution.

Btw if you create the request can you also update the readme file and add for example the keep alive parameter?

nikeee commented 4 years ago

Cool work.

Does this poll the stop watch value? Seems like it should use a lot of resources. Maybe we can just use Task.Delay() in the KeepAlive loop.

@Igund mentions here that there is a command called alive. Can we use that?

aleksanderpsuj commented 4 years ago

I've used this solution for my project and its works so far so good.

TeamSpeakClient.cs

public async Task KeepAlive(int delay)
        {
            while (Client.IsConnected)
            {
                await Task.Delay(TimeSpan.FromSeconds(delay)).ConfigureAwait(false);
                await Client.Send("version").ConfigureAwait(false);
            }
        }

Program.cs rc.KeepAlive(100);

Arefu commented 4 years ago

Hey well done. Great idea I write my keep alive by myself but it´s better with this solution.

Btw if you create the request can you also update the readme file and add for example the keep alive parameter?

Yeah, I might as well do that a bit later tonight, I kinda forgot to that side of things lel.

Cool work.

Does this poll the stop watch value? Seems like it should use a lot of resources. Maybe we can just use Task.Delay() in the KeepAlive loop.

@igund mentions here that there is a command called alive. Can we use that?

We could use that, I chose whoami as I wasn't aware of a 'alive' command taht we could send, if you'd like me to swap it around I can, as for the Task.Delay that seems fair since then it's not just polling constantly checking, I'll do some research on the alive command when I am home later tonight.

Edit: I've logged into my server, and it says:

error id=256 msg=command\snot\sfound when I run 'alive' so, I guess that's not the command, I still think either whoami or version is a good command to send since the info isn't really all that intense.

I've used this solution for my project and its works so far so good.

TeamSpeakClient.cs

public async Task KeepAlive(int delay)
        {
            while (Client.IsConnected)
            {
                await Task.Delay(TimeSpan.FromSeconds(delay)).ConfigureAwait(false);
                await Client.Send("version").ConfigureAwait(false);
            }
        }

Program.cs rc.KeepAlive(100);

That actually looks a lot cleaner lol, maybe we should just throw that in there

Arefu commented 4 years ago

Not to sure if I am doing it right with the Task.Delay since it's my first time actually using it 😅, but that should make it wait a minute if it's more a minute away from the check, so it won't do it so often.

nikeee commented 4 years ago

Task.Delay also takes a TimeSpan.

I'd suggest the following API:

class TeamSpeakClient {
    public TimeSpan? KeepAliveInterval { get; }
    TeamSpeakClient(TimeSpan? keepAliveInterval = null) {
        KeepAliveInterval = keepAliveInterval;
    }
    // ...
    async Task KeepAliveLoop() {
        if (KeepAliveInterval == null)
            return Task.CompletedTask;
        while (this.Client.IsConnected) {
            await Task.Delay(KeepAliveInterval.Value).ConfigureAwait(false);
            await KeepAliveAsync();
        }
    }
}
// (with KeepAliveAsync being some operation)

Task.Delay also takes a CancellationToken, which we should also provide. We have to maintain a CancellationTookenSource at class-level, so we can cancel the KeepAliveLoop when the user disconnects (otherwise, we'd have a dangling thread for at least one minute). Something like this:


private CancellationTokenSource _keepAliveCancellationTokenSource;

async Task KeepAliveLoop() {
    if (KeepAliveInterval == null)
        return Task.CompletedTask;

    _keepAliveCancellationTokenSource = new CancellationTokenSource();

    while (this.Client.IsConnected) {
        try {
            await Task.Delay(KeepAliveInterval.Value, _keepAliveCancellationTokenSource.Token).ConfigureAwait(false);
        } catch(OperationCanceledException) {
            return;
        }
        await KeepAliveAsync();
    }
}
// Call _keepAliveCancellationTokenSource?.Cancel() when disconnecting / disposing

We don't need any timer if we do it using Task.Delay().

Edit: When using Task.Delay() this way, we don't consider requests that the user has sent hinself. We then send the keep-alive every X seconds, regardless if it is actually needed.

We can work around this by saving the timestamp of the last sent command. If it exceeds a threshold (Now - LastCommandSentTimeStamp > KeepAliveInterval), we send a KeepAlive. If not, we Task.Delay(LastCommandSentTimeStamp + KeepAliveInterval).

Arefu commented 4 years ago

Is this more what you'd be after?

This is quite out of scope for what my original project was, so I won't be pursuing it much longer sorry.

nikeee commented 3 years ago

Thanks!