Add StopScan method.

Add StopScan and refactor StartScan to return a sesssion id.
Move handling of StartScan, StopScan and NotifyFoundBle to the same thread to avoid race conditions.
Implement start advertising in service controller.

PiperOrigin-RevId: 492016697

presence/data_types.h

@@ -26,37 +26,8 @@
 namespace nearby {
 namespace presence {
 
-// Holds the callback of stop scan for client to invoke later.
-// TODO(b/254895067) Rework status for absl::Status
-class ScanSession {
- public:
-  ScanSession()
-      : stop_scan_callback_(
-            []() { return Status{Status::Value::kNotImplemented}; }) {}
-  explicit ScanSession(absl::AnyInvocable<Status(void) &&> stop_scan_callback)
-      : stop_scan_callback_(std::move(stop_scan_callback)) {}
-
-  ~ScanSession() { StopScan(); }
-
-  Status StopScan() {
-    if (stop_called_) {
-      NEARBY_LOGS(WARNING) << "StopScan already called.";
-      return Status{Status::Value::kError};
-    }
-    stop_called_ = true;
-    if (stop_scan_callback_) {
-      return std::move(stop_scan_callback_)();
-    }
-    return Status{Status::Value::kError};
-  }
-
- private:
-  // Nearby library would provide the implementation of this callback in
-  // runtime. Assigning with a default value NotImplemented to surface potential
-  // issue where library failed to provide the implementation.
-  absl::AnyInvocable<Status(void) &&> stop_scan_callback_;
-  bool stop_called_ = false;
-};
+// Unique Scan Session Identifier.
+using ScanSessionId = uint64_t;
 
 // Callers would provide the implementation of these callbacks. If callers
 // don't need these signal updates, they can skip with the provided default

presence/implementation/mock_service_controller.h

@@ -31,7 +31,7 @@ class MockServiceController : public ServiceController {
   MockServiceController() = default;
   ~MockServiceController() override = default;
 
-  MOCK_METHOD(std::unique_ptr<ScanSession>, StartScan,
+  MOCK_METHOD(absl::StatusOr<ScanSessionId>, StartScan,
               (ScanRequest scan_request, ScanCallback callback), (override));
   MOCK_METHOD(absl::StatusOr<BroadcastSessionId>, StartBroadcast,
               (BroadcastRequest broadcast_request, BroadcastCallback callback),

presence/implementation/scan_manager.cc

@@ -22,8 +22,8 @@
 #include <vector>
 
 #include "absl/random/random.h"
-#include "absl/random/uniform_int_distribution.h"
 #include "absl/types/variant.h"
+#include "internal/platform/future.h"
 #include "internal/platform/implementation/ble_v2.h"
 #include "internal/platform/uuid.h"
 #include "presence/data_types.h"
@@ -47,86 +47,93 @@ using ScanningCallback =
 namespace nearby {
 namespace presence {
 
-std::unique_ptr<ScanSession> ScanManager::StartScan(ScanRequest scan_request,
-                                                    ScanCallback cb) {
-  absl::BitGen gen;
-  uint64_t id = absl::uniform_int_distribution<uint64_t>(0, UINT64_MAX)(gen);
-  ScanningCallback callback = ScanningCallback{
-      .start_scanning_result =
-          [start_scan_client =
-               std::move(cb.start_scan_cb)](BleOperationStatus ble_status) {
-            Status status;
-            if (ble_status == BleOperationStatus::kSucceeded) {
-              status = Status{.value = Status::Value::kSuccess};
-            } else {
-              status = Status{.value = Status::Value::kError};
-            }
-            start_scan_client(status);
-          },
-      // TODO(b/256686710): Track known devices
-      .advertisement_found_cb =
-          [this](BlePeripheral& peripheral, BleAdvertisementData data) {
-            NotifyFoundBle(data, peripheral);
-          }};
-  std::unique_ptr<ScanningSession> scanning_session =
-      mediums_->GetBle().StartScanning(scan_request, std::move(callback));
-  if (scanning_session == nullptr) {
-    return nullptr;
-  }
-  // We will not be needing the start_scan_cb anymore, so cb is ok to use here.
-  AddScanCallback(id, MapElement{
-                          .request = scan_request,
-                          .callback = cb,
-                          .decoder = AdvertisementDecoder(credential_manager_,
-                                                          scan_request),
-                      });
-  return std::make_unique<ScanSession>(
-      /*stop_scan_callback=*/[scanning_session_internal =
-                                  std::move(scanning_session),
-                              this, id]() {
-        {
-          absl::MutexLock lock(&mutex_);
-          int erased = absl::erase_if(
-              scanning_callbacks_,
-              [id](const auto& entry) { return id == entry.first; });
-          if (erased == 0) return Status{.value = Status::Value::kError};
-          // Unlock mutex since we don't need to access the list anymore.
+ScanSessionId ScanManager::StartScan(ScanRequest scan_request,
+                                     ScanCallback cb) {
+  ScanSessionId id = absl::Uniform<BroadcastSessionId>(bit_gen_);
+  RunOnServiceControllerThread(
+      "start-scan",
+      [this, id, scan_request, scan_callback = std::move(cb)]()
+          ABSL_EXCLUSIVE_LOCKS_REQUIRED(*executor_) {
+            ScanningCallback callback = ScanningCallback{
+                .start_scanning_result =
+                    [start_scan_client =
+                         std::move(scan_callback.start_scan_cb)](
+                        BleOperationStatus ble_status) {
+                      Status status;
+                      if (ble_status == BleOperationStatus::kSucceeded) {
+                        status = Status{.value = Status::Value::kSuccess};
+                      } else {
+                        status = Status{.value = Status::Value::kError};
+                      }
+                      start_scan_client(status);
+                    },
+                // TODO(b/256686710): Track known devices
+                .advertisement_found_cb =
+                    [this, id](BlePeripheral& peripheral,
+                               BleAdvertisementData data) {
+                      RunOnServiceControllerThread(
+                          "notify-found-ble",
+                          [this, id, data = std::move(data),
+                           address = peripheral.GetAddress()]()
+                              ABSL_EXCLUSIVE_LOCKS_REQUIRED(*executor_) {
+                                NotifyFoundBle(id, data, address);
+                              });
+                    }};
+            scan_sessions_.insert(
+                {id, ScanSessionState{
+                         .request = scan_request,
+                         .callback = std::move(scan_callback),
+                         .decoder = AdvertisementDecoder(credential_manager_,
+                                                         scan_request),
+                         .scanning_session = mediums_->GetBle().StartScanning(
+                             scan_request, std::move(callback))}});
+          });
+  return id;
+}
+
+void ScanManager::StopScan(ScanSessionId id) {
+  RunOnServiceControllerThread(
+      "stop-scan", [this, id]() ABSL_EXCLUSIVE_LOCKS_REQUIRED(*executor_) {
+        auto it = scan_sessions_.find(id);
+        if (it == scan_sessions_.end()) {
+          return;
         }
-        BleOperationStatus st = scanning_session_internal->stop_scanning();
-        if (st != BleOperationStatus::kSucceeded) {
-          return Status{.value = Status::Value::kError};
+        if (it->second.scanning_session) {
+          it->second.scanning_session->stop_scanning();
         }
-        return Status{.value = Status::Value::kSuccess};
+        scan_sessions_.erase(it);
       });
 }
 
-void ScanManager::NotifyFoundBle(BleAdvertisementData data,
-                                 const BlePeripheral& peripheral) {
-  std::vector<std::pair<ScanCallback, internal::DeviceMetadata>> callbacks;
-  {
-    absl::MutexLock lock(&mutex_);
-    auto advertisement_data =
-        data.service_data[kPresenceServiceUuid].AsStringView();
-    for (const auto& entry : scanning_callbacks_) {
-      auto candidate = entry.second;
-      auto advert = candidate.decoder.DecodeAdvertisement(advertisement_data);
-      if (!advert.ok()) {
-        // This advertisement is not relevant to the current element, skip.
-        continue;
-      }
-      if (candidate.decoder.MatchesScanFilter(advert.value())) {
-        std::string bt_addr = peripheral.GetAddress();
-        internal::DeviceMetadata metadata;
-        metadata.set_bluetooth_mac_address(bt_addr);
-        callbacks.push_back({candidate.callback, metadata});
-      }
-    }
+void ScanManager::NotifyFoundBle(ScanSessionId id, BleAdvertisementData data,
+                                 absl::string_view remote_address) {
+  auto advertisement_data =
+      data.service_data[kPresenceServiceUuid].AsStringView();
+  auto it = scan_sessions_.find(id);
+  if (it == scan_sessions_.end()) {
+    return;
   }
-  // TODO(b/256913915): Provide more information in PresenceDevice once fully
-  // implemented
-  for (const auto& callback : callbacks) {
-    callback.first.on_discovered_cb(PresenceDevice(callback.second));
+  auto advert = it->second.decoder.DecodeAdvertisement(advertisement_data);
+  if (!advert.ok()) {
+    // This advertisement is not relevant to the current element, skip.
+    return;
   }
+  if (it->second.decoder.MatchesScanFilter(advert.value())) {
+    // TODO(b/256913915): Provide more information in PresenceDevice once
+    // fully implemented
+    internal::DeviceMetadata metadata;
+    metadata.set_bluetooth_mac_address(remote_address);
+    it->second.callback.on_discovered_cb(PresenceDevice(metadata));
+  }
+}
+
+int ScanManager::ScanningCallbacksLengthForTest() {
+  ::location::nearby::Future<int> count;
+  RunOnServiceControllerThread("callbacks-size",
+                               [&]() ABSL_EXCLUSIVE_LOCKS_REQUIRED(*executor_) {
+                                 count.Set(scan_sessions_.size());
+                               });
+  return count.Get().GetResult();
 }
 
 }  // namespace presence

presence/implementation/scan_manager.h

@@ -21,7 +21,8 @@
 #include <utility>
 
 #include "absl/container/flat_hash_map.h"
-#include "absl/synchronization/mutex.h"
+#include "absl/random/random.h"
+#include "internal/platform/single_thread_executor.h"
 #include "presence/data_types.h"
 #include "presence/implementation/advertisement_decoder.h"
 #include "presence/implementation/credential_manager.h"
@@ -31,45 +32,51 @@
 namespace nearby {
 namespace presence {
 
-/*
- * The instance of ScanManager is owned by {@code ServiceControllerImpl}.
- * Helping service controller to manage scan requests and callbacks.
- */
+// The instance of ScanManager is owned by `ServiceControllerImpl`.
+// Helping service controller to manage scan requests and callbacks.
 class ScanManager {
  public:
-  ScanManager(Mediums& mediums, CredentialManager& credential_manager) {
+  using SingleThreadExecutor = ::location::nearby::SingleThreadExecutor;
+  using Mutex = ::location::nearby::Mutex;
+  using MutexLock = ::location::nearby::MutexLock;
+  using ScanningSession =
+      ::location::nearby::api::ble_v2::BleMedium::ScanningSession;
+  using Runnable = ::location::nearby::Runnable;
+  using BleAdvertisementData =
+      ::location::nearby::api::ble_v2::BleAdvertisementData;
+
+  ScanManager(Mediums& mediums, CredentialManager& credential_manager,
+              SingleThreadExecutor& executor) {
     mediums_ = &mediums, credential_manager_ = &credential_manager;
+    executor_ = &executor;
   }
   ~ScanManager() = default;
 
-  std::unique_ptr<ScanSession> StartScan(ScanRequest scan_request,
-                                         ScanCallback cb)
-      ABSL_LOCKS_EXCLUDED(mutex_);
+  ScanSessionId StartScan(ScanRequest scan_request, ScanCallback cb);
+  void StopScan(ScanSessionId session_id);
   // Below functions are test only.
   // Reference: go/totw/135#augmenting-the-public-api-for-tests
-  int ScanningCallbacksLengthForTest() ABSL_LOCKS_EXCLUDED(mutex_) {
-    absl::MutexLock lock(&mutex_);
-    return scanning_callbacks_.size();
-  }
+  int ScanningCallbacksLengthForTest();
 
  private:
-  struct MapElement {
+  struct ScanSessionState {
     ScanRequest request;
     ScanCallback callback;
     AdvertisementDecoder decoder;
+    std::unique_ptr<ScanningSession> scanning_session;
   };
-  void AddScanCallback(uint64_t id, MapElement element) {
-    absl::MutexLock lock(&mutex_);
-    scanning_callbacks_.insert({id, element});
+  void NotifyFoundBle(ScanSessionId id, BleAdvertisementData data,
+                      absl::string_view remote_address)
+      ABSL_EXCLUSIVE_LOCKS_REQUIRED(*executor_);
+  void RunOnServiceControllerThread(absl::string_view name, Runnable runnable) {
+    executor_->Execute(std::string(name), std::move(runnable));
   }
-  mutable absl::Mutex mutex_;
   Mediums* mediums_;
   CredentialManager* credential_manager_;
-  absl::flat_hash_map<uint64_t, MapElement> scanning_callbacks_
-      ABSL_GUARDED_BY(mutex_);
-  void NotifyFoundBle(
-      location::nearby::api::ble_v2::BleAdvertisementData data,
-      const location::nearby::api::ble_v2::BlePeripheral& peripheral);
+  absl::flat_hash_map<ScanSessionId, ScanSessionState> scan_sessions_
+      ABSL_GUARDED_BY(*executor_);
+  SingleThreadExecutor* executor_;
+  absl::BitGen bit_gen_;
 };
 
 }  // namespace presence

presence/implementation/scan_manager_test.cc

@@ -31,6 +31,7 @@
 #include "internal/platform/count_down_latch.h"
 #include "internal/platform/logging.h"
 #include "internal/platform/medium_environment.h"
+#include "internal/platform/single_thread_executor.h"
 #include "presence/implementation/advertisement_factory.h"
 #include "presence/implementation/base_broadcast_request.h"
 #include "presence/implementation/credential_manager_impl.h"
@@ -46,6 +47,7 @@ using AdvertisingSession =
     ::location::nearby::api::ble_v2::BleMedium::AdvertisingSession;
 using AdvertisingCallback =
     ::location::nearby::api::ble_v2::BleMedium::AdvertisingCallback;
+using ::location::nearby::SingleThreadExecutor;
 
 using CountDownLatch = ::location::nearby::CountDownLatch;
 
@@ -117,11 +119,12 @@ class ScanManagerTest : public testing::Test {
       location::nearby::MediumEnvironment::Instance()};
   CountDownLatch start_latch_{1};
   CountDownLatch found_latch_{1};
+  SingleThreadExecutor executor_;
 };
 
 TEST_F(ScanManagerTest, CanStartThenStopScanning) {
   Mediums mediums;
-  ScanManager manager(mediums, credential_manager_);
+  ScanManager manager(mediums, credential_manager_, executor_);
   // Set up advertiser
   location::nearby::BluetoothAdapter server_adapter;
   Ble ble2(server_adapter);
@@ -129,20 +132,20 @@ TEST_F(ScanManagerTest, CanStartThenStopScanning) {
       StartAdvertisingOn(ble2);
 
   // Start scanning
-  auto scan_session =
+  ScanSessionId scan_session =
       manager.StartScan(MakeDefaultScanRequest(), MakeDefaultScanCallback());
   EXPECT_EQ(manager.ScanningCallbacksLengthForTest(), 1);
   EXPECT_TRUE(start_latch_.Await().Ok());
   EXPECT_TRUE(found_latch_.Await().Ok());
-  EXPECT_TRUE(scan_session->StopScan().Ok());
+  manager.StopScan(scan_session);
   EXPECT_EQ(manager.ScanningCallbacksLengthForTest(), 0);
 }
 
 TEST_F(ScanManagerTest, CannotStopScanTwice) {
   Mediums mediums;
-  ScanManager manager(mediums, credential_manager_);
+  ScanManager manager(mediums, credential_manager_, executor_);
 
-  auto scan_session =
+  ScanSessionId scan_session =
       manager.StartScan(MakeDefaultScanRequest(), MakeDefaultScanCallback());
 
   NEARBY_LOGS(INFO) << "Start scan";
@@ -150,14 +153,16 @@ TEST_F(ScanManagerTest, CannotStopScanTwice) {
   // Ensure that we have started scanning before we try to stop.
   env_.Sync();
   NEARBY_LOGS(INFO) << "Stop scan";
-  EXPECT_TRUE(scan_session->StopScan().Ok());
+  manager.StopScan(scan_session);
+  EXPECT_EQ(manager.ScanningCallbacksLengthForTest(), 0);
   NEARBY_LOGS(INFO) << "Stop scan again";
-  EXPECT_FALSE(scan_session->StopScan().Ok());
+  manager.StopScan(scan_session);
+  EXPECT_EQ(manager.ScanningCallbacksLengthForTest(), 0);
 }
 
 TEST_F(ScanManagerTest, TestNoFilter) {
   Mediums mediums;
-  ScanManager manager(mediums, credential_manager_);
+  ScanManager manager(mediums, credential_manager_, executor_);
   // Set up advertiser
   location::nearby::BluetoothAdapter server_adapter;
   Ble ble2(server_adapter);
@@ -167,20 +172,20 @@ TEST_F(ScanManagerTest, TestNoFilter) {
   // Start scanning
   ScanRequest scan_request_no_filter = MakeDefaultScanRequest();
   scan_request_no_filter.scan_filters.clear();
-  auto scan_session =
+  ScanSessionId scan_session =
       manager.StartScan(scan_request_no_filter, MakeDefaultScanCallback());
 
   ASSERT_EQ(manager.ScanningCallbacksLengthForTest(), 1);
   ASSERT_TRUE(mediums.GetBle().IsAvailable());
   EXPECT_TRUE(start_latch_.Await().Ok());
   EXPECT_TRUE(found_latch_.Await().Ok());
-  EXPECT_TRUE(scan_session->StopScan().Ok());
+  manager.StopScan(scan_session);
   EXPECT_EQ(manager.ScanningCallbacksLengthForTest(), 0);
 }
 
 TEST_F(ScanManagerTest, PresenceDeviceMetadataIsRetained) {
   Mediums mediums;
-  ScanManager manager(mediums, credential_manager_);
+  ScanManager manager(mediums, credential_manager_, executor_);
   // Set up advertiser
   location::nearby::BluetoothAdapter server_adapter;
   Ble ble2(server_adapter);
@@ -204,20 +209,19 @@ TEST_F(ScanManagerTest, PresenceDeviceMetadataIsRetained) {
   // Start scanning
   ScanRequest scan_request_no_filter = MakeDefaultScanRequest();
   scan_request_no_filter.scan_filters.clear();
-  auto scan_session =
-      manager.StartScan(scan_request_no_filter, callback);
+  auto scan_session = manager.StartScan(scan_request_no_filter, callback);
 
   ASSERT_EQ(manager.ScanningCallbacksLengthForTest(), 1);
   ASSERT_TRUE(mediums.GetBle().IsAvailable());
   EXPECT_TRUE(start_latch_.Await().Ok());
-  EXPECT_TRUE(found_latch_.Await(absl::Milliseconds(1000)).result());
-  EXPECT_TRUE(scan_session->StopScan().Ok());
+  EXPECT_TRUE(found_latch_.Await().Ok());
+  manager.StopScan(scan_session);
   EXPECT_EQ(manager.ScanningCallbacksLengthForTest(), 0);
 }
 
 TEST_F(ScanManagerTest, StopOneSessionFromAnotherDeadlock) {
   Mediums mediums;
-  ScanManager manager(mediums, credential_manager_);
+  ScanManager manager(mediums, credential_manager_, executor_);
   CountDownLatch start_latch2{1};
   CountDownLatch found_latch2{1};
 
@@ -240,23 +244,23 @@ TEST_F(ScanManagerTest, StopOneSessionFromAnotherDeadlock) {
   };
   // we use scan_request_mismatch so this session's discovery doesn't get
   // triggered.
-  auto scan_session =
+  ScanSessionId scan_session =
       manager.StartScan(scan_request_mismatch, MakeDefaultScanCallback());
-  ScanCallback scanning_callback2 = {
-      .start_scan_cb =
-          [&start_latch2](Status status) {
-            if (status.Ok()) {
-              start_latch2.CountDown();
-            }
-          },
-      .on_discovered_cb =
-          [&found_latch2, &scan_session](PresenceDevice pd) {
-            NEARBY_LOGS(INFO) << "scansession2 found";
-            found_latch2.CountDown();
-            scan_session->StopScan();
-          }};
-  auto scan_session2 = manager.StartScan(MakeDefaultScanRequest(),
-                                         std::move(scanning_callback2));
+  ScanCallback scanning_callback2 = {.start_scan_cb =
+                                         [&](Status status) {
+                                           if (status.Ok()) {
+                                             start_latch2.CountDown();
+                                           }
+                                         },
+                                     .on_discovered_cb =
+                                         [&](PresenceDevice pd) {
+                                           NEARBY_LOGS(INFO)
+                                               << "scansession2 found";
+                                           found_latch2.CountDown();
+                                           manager.StopScan(scan_session);
+                                         }};
+  ScanSessionId scan_session2 = manager.StartScan(
+      MakeDefaultScanRequest(), std::move(scanning_callback2));
 
   ASSERT_EQ(manager.ScanningCallbacksLengthForTest(), 2);
 
@@ -266,56 +270,15 @@ TEST_F(ScanManagerTest, StopOneSessionFromAnotherDeadlock) {
   std::unique_ptr<AdvertisingSession> advertising_session =
       StartAdvertisingOn(ble2);
 
-  EXPECT_TRUE(found_latch2.Await(absl::Milliseconds(1500)).result());
-  EXPECT_FALSE(found_latch_.Await(absl::Milliseconds(1500)).result());
+  EXPECT_TRUE(found_latch2.Await().Ok());
   // Session was stopped before, this should not be able to stop successfully.
-  EXPECT_FALSE(scan_session->StopScan().Ok());
+  manager.StopScan(scan_session);
   EXPECT_EQ(manager.ScanningCallbacksLengthForTest(), 1);
-  EXPECT_TRUE(scan_session2->StopScan().Ok());
+  ASSERT_TRUE(mediums.GetBle().IsAvailable());
+  manager.StopScan(scan_session2);
   EXPECT_EQ(manager.ScanningCallbacksLengthForTest(), 0);
 }
 
-TEST_F(ScanManagerTest, StopWhenScopeEnds) {
-  Mediums mediums;
-  ScanManager manager(mediums, credential_manager_);
-  ScanCallback scanning_callback = ScanCallback{
-      .start_scan_cb =
-          [this](Status status) {
-            if (status.Ok()) {
-              start_latch_.CountDown();
-            }
-          },
-  };
-  {
-    auto scan_session = manager.StartScan(MakeDefaultScanRequest(),
-                                          std::move(scanning_callback));
-    EXPECT_EQ(manager.ScanningCallbacksLengthForTest(), 1);
-    // Ensure that we start scanning before we go out of scope.
-    env_.Sync();
-  }
-  EXPECT_EQ(manager.ScanningCallbacksLengthForTest(), 0);
-}
-
-TEST_F(ScanManagerTest, MoveDoesNotTriggerDestructor) {
-  Mediums mediums;
-  ScanManager manager(mediums, credential_manager_);
-  ScanCallback scanning_callback = ScanCallback{
-      .start_scan_cb =
-          [this](Status status) {
-            if (status.Ok()) {
-              start_latch_.CountDown();
-            }
-          },
-  };
-  auto scan_session =
-      manager.StartScan(MakeDefaultScanRequest(), std::move(scanning_callback));
-  EXPECT_EQ(manager.ScanningCallbacksLengthForTest(), 1);
-  env_.Sync();
-  auto scan_session_moved = std::move(scan_session);
-  // Make sure we don't trigger the destructor.
-  EXPECT_EQ(manager.ScanningCallbacksLengthForTest(), 1);
-}
-
 }  // namespace
 }  // namespace presence
 }  // namespace nearby

presence/implementation/service_controller.h

@@ -33,8 +33,9 @@ class ServiceController {
  public:
   ServiceController() = default;
   virtual ~ServiceController() = default;
-  virtual std::unique_ptr<ScanSession> StartScan(ScanRequest scan_request,
-                                                 ScanCallback callback) = 0;
+  virtual absl::StatusOr<ScanSessionId> StartScan(ScanRequest scan_request,
+                                                  ScanCallback callback) = 0;
+  virtual void StopScan(ScanSessionId session_id) = 0;
   virtual absl::StatusOr<BroadcastSessionId> StartBroadcast(
       BroadcastRequest broadcast_request, BroadcastCallback callback) = 0;
   virtual void StopBroadcast(BroadcastSessionId session_id) = 0;

presence/implementation/service_controller_impl.cc

@@ -46,10 +46,13 @@ Status ConvertBleStatus(BleOperationStatus status) {
 
 }  // namespace
 
-std::unique_ptr<ScanSession> ServiceControllerImpl::StartScan(
+absl::StatusOr<ScanSessionId> ServiceControllerImpl::StartScan(
     ScanRequest scan_request, ScanCallback callback) {
   return scan_manager_.StartScan(scan_request, callback);
 }
+void ServiceControllerImpl::StopScan(ScanSessionId id) {
+  scan_manager_.StopScan(id);
+}
 
 absl::StatusOr<BroadcastSessionId> ServiceControllerImpl::StartBroadcast(
     BroadcastRequest broadcast_request, BroadcastCallback callback) {

presence/implementation/service_controller_impl.h

@@ -49,8 +49,9 @@ class ServiceControllerImpl : public ServiceController {
   ServiceControllerImpl() = default;
   ~ServiceControllerImpl() override { executor_.Shutdown(); }
 
-  std::unique_ptr<ScanSession> StartScan(ScanRequest scan_request,
-                                         ScanCallback callback) override;
+  absl::StatusOr<ScanSessionId> StartScan(ScanRequest scan_request,
+                                          ScanCallback callback) override;
+  void StopScan(ScanSessionId session_id) override;
   absl::StatusOr<BroadcastSessionId> StartBroadcast(
       BroadcastRequest broadcast_request, BroadcastCallback callback) override;
   void StopBroadcast(BroadcastSessionId) override;
@@ -85,8 +86,8 @@ class ServiceControllerImpl : public ServiceController {
   Mediums mediums_;  // NOLINT: further impl will use it.
   CredentialManagerImpl
       credential_manager_;  // NOLINT: further impl will use it.
-  ScanManager scan_manager_{
-      mediums_, credential_manager_};  // NOLINT: further impl will use it.
+  ScanManager scan_manager_{mediums_, credential_manager_,
+                            executor_};  // NOLINT: further impl will use it.
   absl::flat_hash_map<BroadcastSessionId, BroadcastSessionState> sessions_
       ABSL_GUARDED_BY(executor_);
   absl::BitGen bit_gen_;

presence/presence_client.cc

@@ -26,14 +26,23 @@
 namespace nearby {
 namespace presence {
 
-std::unique_ptr<ScanSession> PresenceClient::StartScan(ScanRequest scan_request,
-                                                       ScanCallback callback) {
+absl::StatusOr<ScanSessionId> PresenceClient::StartScan(
+    ScanRequest scan_request, ScanCallback callback) {
   ::location::nearby::Borrowed<PresenceService*> borrowed = service_.Borrow();
   if (!borrowed) {
-    return nullptr;
+    return absl::FailedPreconditionError(
+        "Can't start scan, presence service is gone");
   }
   return (*borrowed)->StartScan(scan_request, callback);
 }
+
+void PresenceClient::StopScan(ScanSessionId id) {
+  ::location::nearby::Borrowed<PresenceService*> borrowed = service_.Borrow();
+  if (borrowed) {
+    (*borrowed)->StopScan(id);
+  }
+}
+
 absl::StatusOr<BroadcastSessionId> PresenceClient::StartBroadcast(
     BroadcastRequest broadcast_request, BroadcastCallback callback) {
   ::location::nearby::Borrowed<PresenceService*> borrowed = service_.Borrow();

presence/presence_client.h

@@ -44,21 +44,23 @@ class PresenceClient {
   PresenceClient(PresenceClient&&) = default;
   PresenceClient& operator=(const PresenceClient&) = delete;
 
-  /**
-   * Starts a Nearby Presence scan and registers {@link ScanCallback}
-   * which will be invoked when a matching {@link PresenceDevice} is detected,
-   * lost, and status changed.
-   *
-   * <p>The {@link ScanCallback} is kept at the Nearby Presence service.
-   * Returning unique_ptr of ScanSession including stop scan callback
-   * for client to invoke later.
-   *
-   * <p>The {@link ScanRequest} contains the options like scan power mode
-   * and type; the filters including credentials, actions and extended
-   * properties.
-   */
-  std::unique_ptr<ScanSession> StartScan(ScanRequest scan_request,
-                                         ScanCallback callback);
+  // Starts a Nearby Presence scan and registers `ScanCallback`
+  // which will be invoked when a matching `PresenceDevice` is detected,
+  // lost, and status changed.
+  // The session can be terminated with `StopScan()`.
+  //
+  // `ScanCallback` is kept in the Nearby Presence service until `StopScan()` is
+  // called.
+  //
+  // `ScanRequest` contains the options like scan power mode
+  // and type; the filters including credentials, actions and extended
+  // properties.
+  absl::StatusOr<ScanSessionId> StartScan(ScanRequest scan_request,
+                                          ScanCallback callback);
+
+  // Terminates the scan session. Does nothing if the session is already
+  // terminated.
+  void StopScan(ScanSessionId session_id);
 
   // Starts a Nearby Presence broadcast and registers `BroadcastCallback`
   // which will be invoked after broadcast is started.

presence/presence_client_test.cc

@@ -73,16 +73,17 @@ TEST_F(PresenceClientTest, StartBroadcastFailsWhenPresenceServiceIsGone) {
 
 TEST_F(PresenceClientTest, StartScanWithDefaultConstructor) {
   env_.Start();
-  Status scan_result = {Status::Value::kError};
+  ::location::nearby::Future<Status> scan_result;
   ScanCallback scan_callback = {
-      .start_scan_cb = [&](Status status) { scan_result = status; },
+      .start_scan_cb = [&](Status status) { scan_result.Set(status); },
   };
 
   PresenceService presence_service;
   PresenceClient presence_client = presence_service.CreatePresenceClient();
-  presence_client.StartScan({}, scan_callback);
+  EXPECT_OK(presence_client.StartScan({}, scan_callback));
 
-  EXPECT_TRUE(scan_result.Ok());
+  EXPECT_TRUE(scan_result.Get().ok());
+  EXPECT_TRUE(scan_result.Get().GetResult().Ok());
   env_.Stop();
 }
 
@@ -93,8 +94,10 @@ TEST_F(PresenceClientTest, StartScanFailsWhenPresenceServiceIsGone) {
       .start_scan_cb = [&](Status status) { scan_result = status; },
   };
 
-  CreateDefunctPresenceClient().StartScan({}, scan_callback);
+  absl::StatusOr<ScanSessionId> session_id =
+      CreateDefunctPresenceClient().StartScan({}, scan_callback);
 
+  EXPECT_THAT(session_id, StatusIs(absl::StatusCode::kFailedPrecondition));
   EXPECT_FALSE(scan_result.Ok());
   env_.Stop();
 }

presence/presence_service.cc

@@ -30,10 +30,13 @@ PresenceClient PresenceService::CreatePresenceClient() {
   return PresenceClient(lender_.GetBorrowable());
 }
 
-std::unique_ptr<ScanSession> PresenceService::StartScan(
+absl::StatusOr<ScanSessionId> PresenceService::StartScan(
     ScanRequest scan_request, ScanCallback callback) {
   return service_controller_->StartScan(scan_request, callback);
 }
+void PresenceService::StopScan(ScanSessionId id) {
+  service_controller_->StopScan(id);
+}
 
 absl::StatusOr<BroadcastSessionId> PresenceService::StartBroadcast(
     BroadcastRequest broadcast_request, BroadcastCallback callback) {

presence/presence_service.h

@@ -37,8 +37,10 @@ class PresenceService {
 
   PresenceClient CreatePresenceClient();
 
-  std::unique_ptr<ScanSession> StartScan(ScanRequest scan_request,
-                                         ScanCallback callback);
+  absl::StatusOr<ScanSessionId> StartScan(ScanRequest scan_request,
+                                          ScanCallback callback);
+  void StopScan(ScanSessionId session_id);
+
   absl::StatusOr<BroadcastSessionId> StartBroadcast(
       BroadcastRequest broadcast_request, BroadcastCallback callback);
 

presence/presence_service_test.cc

@@ -14,6 +14,8 @@
 
 #include "presence/presence_service.h"
 
+#include "gmock/gmock.h"
+#include "protobuf-matchers/protocol-buffer-matchers.h"
 #include "gtest/gtest.h"
 #include "internal/platform/medium_environment.h"
 #include "presence/presence_client.h"
@@ -42,22 +44,19 @@ TEST_F(PresenceServiceTest, StartThenStopScan) {
   PresenceService presence_service;
   PresenceClient client = presence_service.CreatePresenceClient();
 
-  auto scan_session = client.StartScan(
+  absl::StatusOr<ScanSessionId> scan_session = client.StartScan(
       {}, {
               .start_scan_cb = [&](Status status) { scan_result = status; },
           });
-  auto scan_session_with_default_params =
+  absl::StatusOr<ScanSessionId> scan_session_with_default_params =
       client.StartScan(ScanRequest(), ScanCallback());
 
-  EXPECT_NE(scan_session, nullptr);
-  EXPECT_NE(scan_session_with_default_params, nullptr);
+  ASSERT_OK(scan_session);
+  ASSERT_OK(scan_session_with_default_params);
+  EXPECT_NE(*scan_session, *scan_session_with_default_params);
 
-  Status stop_scan_session_status = scan_session->StopScan();
-  Status scan_session_with_default_params_status =
-      scan_session_with_default_params->StopScan();
-
-  EXPECT_TRUE(stop_scan_session_status.Ok());
-  EXPECT_TRUE(scan_session_with_default_params_status.Ok());
+  client.StopScan(*scan_session);
+  client.StopScan(*scan_session_with_default_params);
   env_.Stop();
 }