[Bug]: IPeripheral's connection state is not updated after .Scan() running again

sleushunou commented 8 months ago

Component/Nuget

BluetoothLE Client (Shiny.BluetoothLE)

What operating system(s) are effected?

[X] iOS (13+ supported)
[ ] Mac Catalyst
[ ] Android (8+ supported)
[ ] Windows (.NET 7 Target - only Core is currently supported, BLE is coming)

Version(s) of Operation Systems

Tested on iOS 16 iPhone 12

Hosting Model

[ ] MAUI
[X] Native/Classic Xamarin
[ ] Manual

Steps To Reproduce

Run .Scan()
Receive IPeripheral from result of .Scan()
Run .Scan() again
Try to connect to received at step 2 IPeripheral and observe IPeripheral::WhenStatusChanged()

Expected Behavior

IPeripheral::WhenStatusChanged() - status changed when peripheral is connected

Actual Behavior

IPeripheral::WhenStatusChanged() - status not changed when peripheral is connected, subscription handler is not called when BUT IBleDelegate::OnPeripheralStateChanged() - state changed when peripheral is connected ✔️

Exception or Log output

instead of logs. seems like problem is here: this.Clear(); List of cached IPeripheral's on BleManager cleared every time after starting a new scan, and when connection status is changed, BleManager calls p.ReceiveStateChange(status);, where p - another, newly created, instance of IPeripheral that refers on the same physical device, and first instance of IPeripheral does not receive state change

public IObservable<ScanResult> Scan(ScanConfig? scanConfig = null) => Observable.Create<ScanResult>(ob =>
{
    if (this.IsScanning)
        throw new InvalidOperationException("There is already an existing scan");

    this.Clear();
    ......
}

void Clear() => this.peripherals
    .Where(x => x.Value.Status != ConnectionState.Connected)
    .ToList()
    .ForEach(x => this.peripherals.TryRemove(x.Key, out var device));

void RunStateChange(CBPeripheral peripheral, bool connected, NSError? error)
{
    this.logger.PeripheralStateChange(peripheral.Identifier, connected, error?.LocalizedDescription ?? "None");

    var p = this.GetPeripheral(peripheral);
    var status = connected ? ConnectionState.Connected : ConnectionState.Disconnected;
    p.ReceiveStateChange(status);

    this.services.RunDelegates<IBleDelegate>(x => x.OnPeripheralStateChanged(p), this.logger);
}

readonly ConcurrentDictionary<string, Peripheral> peripherals = new();
Peripheral GetPeripheral(CBPeripheral peripheral) => this.peripherals.GetOrAdd(
    peripheral.Identifier.ToString(),
    x => new Peripheral(this, peripheral, this.operations, this.peripheralLogger)
);

Code Sample

    private  IDisposable _subscription;

    private async Task Test(IBleManager bleManager)
    {
        var scanResult = await bleManager
            .Scan(new ScanConfig())
            .FirstAsync()
            .ToTask()
            .ConfigureAwait(false);

        var peripheral = scanResult.Peripheral;

        var scanResult2 = await bleManager
            .Scan(new ScanConfig())
            .FirstAsync()
            .ToTask()
            .ConfigureAwait(false);

        _subscription = peripheral.WhenStatusChanged().Subscribe(OnStatusChanged);
        peripheral.Connect(new ConnectionConfig(false));
    }

    private void OnStatusChanged(ConnectionState state)
    {

    }

Code of Conduct

[X] I have supplied a reproducible sample that is NOT FROM THE SHINY SAMPLES!
[X] I am a Sponsor OR I am using the LATEST stable/beta version from nuget (v3.0 stable - ALPHAS are not taking issues - Sponsors can still send v2 issues)
[X] I am Sponsor OR My GitHub account is 30+ days old
[X] I understand that if I am checking these boxes and I am not actually following what they are saying, I will be removed from this repository!

aritchie commented 8 months ago

Do you have a good business case for this? This potential fix may seem "easy" here, but it definitely is not?

Most peripherals stop advertising when they have connections
Rescanning with the same cb manager is not recommended as any good practice

sleushunou commented 8 months ago

https://developer.apple.com/library/archive/documentation/NetworkingInternetWeb/Conceptual/CoreBluetooth_concepts/BestPracticesForInteractingWithARemotePeripheralDevice/BestPracticesForInteractingWithARemotePeripheralDevice.html#//apple_ref/doc/uid/TP40013257-CH6-SW1

According to the Apple guide, scanning should be active only when it is really needed to save battery power.

business case: Once we have found all of our expected devices, we stop scanning. But, if we want to add another device to the device list, we run the scan again.

aritchie commented 8 months ago

Are you actually connected to the devices while running a scan though?

aritchie commented 8 months ago

No response from user.

You can't keep a reference to an unconnected device and then run a scan. iOS emits new peripheral.

shinyorg / shiny