1iveowl / WebsocketClientLite.PCL

Websocket client lite
MIT License
38 stars 14 forks source link

CancellationToken in develop branch #12

Closed cmisztur closed 7 years ago

cmisztur commented 7 years ago

Looks like you refactored some code in develop.

This no longer works:

 cts.Token.Register(() =>
            {
                _logger.Warn("Connection closed.");
                disposeSubscriber();
                onDisconnect?.Invoke();
            });
1iveowl commented 7 years ago

Yes this is right. After the refactoring this token doesn't really make sense anymore. In fact in v3.6.1-beta3 I've removed it altogether.

In stead I've added an instead it is not possible to monitor/log activities using an observable like this:

var websocketLoggerSubscriber = websocketClient.ObserveConnectionStatus.Subscribe(
                    s =>
                    {
                        System.Console.WriteLine(s.ToString());
                    });

What do you think. Makes sense to remove the Cancellation token?

cmisztur commented 7 years ago

That is much cleaner!

How do I unsubscribe? I dispose websocketLoggerSubscriber after disconnect and then before another connect I subscribe websocketLoggerSubscriber again. This yields in duplicate invocations somewhere.

cmisztur commented 7 years ago

I do not see an OnCompleted for connection status observable.

1iveowl commented 7 years ago

In Rx you don't unsubscribe, you Dispose. Look at the test. To unsubscribe here you would do like this:websocketLoggerSubscriber.Dispose(). However in the test the whole thing is inside a Using block and subscriptions will therefore be disposed automatically.

Anywhy, you don't have to unsubscribe the "logger" when you connect or disconnect to a Websocket. Again try and look at the test. I'm connecting and disconnecting to different Websocket servers.

Make sense?

I need to rewrite some of the documentation, but first let me know if this new paradigm makes sense.

Regarding your other post. What do you mean by "OnCompleted"? Did I just answer that or is this something different?

cmisztur commented 7 years ago

Looking at some RX documentation it looks like OnCompleted should be called when you're done with it?

It does make sense, but if you try

subscribe
openasync
closeasync
dispose
subscribe
openasync

You will see multiple 'Connecting', 'Connected', etc fired. This is not the case with the text message receiver, only with connection status receiver.

1iveowl commented 7 years ago

Ahh, now I understand.

Yes, a Rx sequence ends with an OnComplete or an Error. It is also ended when Disposed, so things should be ok in general. Anyway, I'll have a look at it. I want it to be consistent.

They way I've implemented it now is that the stream of status messages is alive as long as the MessageRx is alive no matter how many times you connect or disconnect. Same about the subscription to the data that is delived from any Websocket servers you connect to. You can however only connect to one server at the time.

Glad you like the other parts 👍

cmisztur commented 7 years ago

nevermind..... monday morning typo on my part. I was disposing the same thing twice, instead of two things once.

Thanks.