StartScanningForDevicesAsync always times out

[JSchaenzle]

Looking at the code in AdapterBase.cs, on both the master branch and the 1.3.0 branch, the StartScanningForDevicesAsync function looks like this:

   public async Task StartScanningForDevicesAsync(Guid[] serviceUuids = null, Func<IDevice, bool> deviceFilter = null, bool allowDuplicatesKey = false, CancellationToken cancellationToken = default(CancellationToken))
    {
      if (IsScanning) {
        Trace.Message("Adapter: Already scanning!");
        return;
      }

      IsScanning = true;
      serviceUuids = serviceUuids ?? new Guid[0];
      _currentScanDeviceFilter = deviceFilter ?? (d => true);
      _scanCancellationTokenSource = new CancellationTokenSource();

      try {
        using (cancellationToken.Register(() => _scanCancellationTokenSource?.Cancel())) {
          await StartScanningForDevicesNativeAsync(serviceUuids, allowDuplicatesKey, _scanCancellationTokenSource.Token);
          await Task.Delay(ScanTimeout, _scanCancellationTokenSource.Token);
          Trace.Message("Adapter: Scan timeout has elapsed.");
          CleanupScan();
          ScanTimeoutElapsed(this, new System.EventArgs());
        }
      } catch (TaskCanceledException) {
        CleanupScan();
        Trace.Message("Adapter: Scan was cancelled.");
      }
    }

I don't understand the intended usage of this method. The name of the method is StartScanningForDevicesAsync which makes it sound like it should return (complete) once scanning has been started. However, from what I can tell it just starts scanning, then delays for a configurable amount of time, and then stops scanning and calls the timeout elapsed callback.

In my application, I need to start scanning and then scan indefinitely until a certain device is found, at which point I would call StopScanningForDevicesAsync in order to connect. I don't really want a scan timeout.

Is the intended usage of this method that it not return until the cancellation token is cancelled? That seems a bit unusual to me but maybe I'm missing something.

[smsissuechecker]

Hi @JSchaenzle,

I'm the friendly issue checker. It seems like (100.00 %) you haven't used our issue template :cry: I think it is very frustrating for the repository owners, if you ignore them.

If you think it's fine to make an exception, just ignore this message. But if you think it was a mistake to delete the template, please close the issue and create a new one.

Thanks!

[JSchaenzle]

I found that I can get this to work by setting ScanTimeout to int.MaxValue, and then remove the await from the call to StartScanningForDevicesAsync that just makes me a little nervous because by removing the await that also means that if any exception is thrown by StartScanningForDevicesAsync (other than the TimeoutException) they will get silently ignored.

[bed007]

Scanning for devices for a long time is very hard on the battery. Maybe you should perform some kind of scanning duty cycle to preserve battery a little bit.

[JSchaenzle]

@bed007 I definitely agree with you in most cases. I am a fan of saving user's battery life. However, there are going to be use cases where that's not feasible. For example, if you're making an app that searches for a device to come into range, you probably want to keep scanning as long as the user still has the app in the foreground until the device is found.

It just seems to me like this method is assuming too much. I think that incorporating a ScanTimeout really should be the responsibility of the application that uses this library, not the library itself. Having a method that only returns by way of being cancelled doesn't seem right either. If it's going to stay that way I would propose changing the name to something like ScanForDevicesUntilCancelledAsync.

If I'm the only one that feels this way I'll shut up and you can close the issue. :) But if others agree then I don't mind spiking out the change and submitting a PR.

[bed007]

You are right, it is possible that in some applications, you want to perform ongoing scanning to detect any new device in range asap.

[JSchaenzle]

Any other thoughts on this? I'm still up for doing some refactoring of this method if others are on board.

[xabre]

I would recommend using ConnectToKnownDevice async if you did connect to a device at some point in time and whant to connect now as soon as it is in range again, it's much more efficient than scanning forever. The iOS implementation does exactly what you want by just calling it once and awaiting (no timeout), in adroid there is an implicit timeout from the sdk.

[xabre]

We could add the timeout as a parameter with a default value. This way the user is aware of this functionality :)