Windows: More than one lost connection does not work correctly

[CodeMusher]

Windows: More than one lost connection does not work correctly

[CodeMusher]

@AskBojesen Sorry, I'll do that of course.

[smsissuechecker]

Hi @CodeMusher,

I'm the friendly issue checker.
Thanks for using the issue template 🌟
I appreciate it very much. I'm sure, the maintainers of this repository will answer, soon.

[janusw]

Version of the Plugin: 3..0-beta-2.

This is supposed to be 3.1.0-beta.2, I assume?

[CodeMusher]

Sorry, my bad! 3.1.0-beta.2 is correct

[CodeMusher]

This could be solved by adding this line
disconnectingRegistry.Remove(id);
in the Device_ConnectionStatusChanged method of the Adapter class

[janusw]

This could be solved by adding this line disconnectingRegistry.Remove(id); in the Device_ConnectionStatusChanged method of the Adapter class

Hm, looks like that line is there already ...

dotnet-bluetooth-le/Source/Plugin.BLE/Windows/Adapter.cs

Lines 148 to 151 in 04a5d27

	if (nativeDevice.ConnectionStatus == BluetoothConnectionStatus.Disconnected
	&& ConnectedDeviceRegistry.TryRemove(id, out var disconnectedDevice))
	{
	bool disconnectRequested = disconnectingRegistry.Remove(id);

... but maybe it's not always called?

@AskBojesen This seems to be your code (see #809). Care to have a look?

[janusw]

@CodeMusher So this fails with 3.1.0-beta.2. Does it work with any prior version, like 3.1.0-beta.1 or 3.0.0? (Then it would be regression.) Or a later version like 3.1.0-rc.1? (Probably not?)

[AskBojesen]

I will try to reproduce with latest later today or tomorrow

[CodeMusher]

@janusw, @AskBojesen
Sounds good.
One more thing I need is the possibility to register for updates on a characteristic that is only INDICATE, not NOTIFY.
That would be nice.

[AskBojesen]

@janusw, @AskBojesen Sounds good. One more thing I need is the possibility to register for updates on a characteristic that is only INDICATE, not NOTIFY. That would be nice.

@CodeMusher: Please register a new issue, so we don't get more confused...
I will ignore your comment in this issue

[AskBojesen]

IMO this is not a bug in the plugin, but wrong usage of the API in the example code, which I have made myself to test something....
Take a look here: https://github.com/dotnet-bluetooth-le/dotnet-bluetooth-le/blob/master/Source/BLE.Client/BLE.Client.WinConsole/PluginDemos.cs line 166

This method does in fact not test connection lost > reconnect... correctly as I believe the API should be used

In our application, where we use the plugin in both Android + iOS + Windows. We listen for connection lost and handles connection lost -> reconnect,.. and it works on all three platforms :-)
BUT my colleague has spend quite some work to make it work ...

If this is to be "fixed" the WinConsole demo method must be fixed to show how it can work. But it is not related to the API and the functionality itself of the plugin - I can fix the demo method within a week - but remember it is only the winconsole demo...

The WinConsole demo is now used - and I appreciate that :-), but I also see it has consequences: All test methods must be valid with respect to how how a client must use the API....

@janusw: IMO the other client implementations are not even close to the winconsole client with respect to correct usage and functionality... I vote fixing up the clients with functionality and correct usage (whatever that is) for some next release.

What do you say @janusw : Should I use the effort to show how this can work with the existing API for connection lost -> reconnect now (within a week)?

[CodeMusher]

@AskBojesen Does the WinConsole demo work the same when you tried it or have I missed something?
I think it would be a great thing to describe how the connection lost/reconnect should be implemented for this to work as designed.

[CodeMusher]

@AskBojesen, @janusw In my application I also listen for lost connection event and start a reconnection process. Since I only managed to get the lost connection event once I tried to find any errors.
I used your WinConsole Demo try to locate the error. I came to the conclusion that the first lost connection invoked the lost connection event but it also put the device in the disconnectingRegistry. That caused the next lost connection to be handled as if the user disconnected and no lost connection event was invoked. Added a line in the source code, removing the device from the disconnectingRegistry every time we got connected kind of solved it.

If this is not a bug and it works for you on all platforms, then something must be missing in my use of the API.
Maybe you could provide some pseudocode to to do this properly?

[AskBojesen]

Well now I have improved the demo in the winconsole.
PR: #849
And I actually see that there is a bug!
The second time I disconnect the plugin states: DisconnectedPeripheral by user, and I get no connectionlost event
I will investigate.

[AskBojesen]

I found the bug and fixed the issue - sorry for my stubbornness from believing there was a bug.
Fixed now in PR: #849

[CodeMusher]

@AskBojesen good to hear that I wasn’t wrong about this😉
I checked your log file and noticed that the second lost connection differs from the others. There is no gatt closed messages. Should the disconnect not be identical?

[AskBojesen]

@AskBojesen good to hear that I wasn’t wrong about this😉 I checked your log file and noticed that the second lost connection differs from the others. There is no gatt closed messages. Should the disconnect not be identical?

No I don't see it as an error with the GattSession_SessionStatusChanged - we could leave it out of the log.
I guess it is some timeng when the GattSession is disposed