Add StopBroadcast

Use explicit StopBroadcast API instead of a callback in BoroadcastSession.

PiperOrigin-RevId: 491759852

presence/BUILD

@@ -29,7 +29,8 @@ cc_library(
         ":types",
         "//internal/platform:types",
         "//presence/implementation:internal",  # build_cleaner: keep
-        "@com_google_absl//absl/log:die_if_null",
+        "@com_google_absl//absl/status",
+        "@com_google_absl//absl/status:statusor",
     ],
 )
 

presence/data_types.h

@@ -76,17 +76,8 @@ struct ScanCallback {
   std::function<void(PresenceDevice)> on_lost_cb = [](PresenceDevice) {};
 };
 
-/**
- * Holds the callback of stop broadcast for client to invoke later.
- */
-struct BroadcastSession {
-  // Nearby library would provide the implementation of this callback in
-  // runtime. Assiging with a default value NotImplemented to surface potential
-  // issue where library failed to provide the implementation.
-  absl::AnyInvocable<Status(void)> stop_broadcast_callback = []() {
-    return Status{Status::Value::kNotImplemented};
-  };
-};
+// Unique Broadcast Session Identifier.
+using BroadcastSessionId = 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

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

presence/implementation/service_controller.h

@@ -35,8 +35,9 @@ class ServiceController {
   virtual ~ServiceController() = default;
   virtual std::unique_ptr<ScanSession> StartScan(ScanRequest scan_request,
                                                  ScanCallback callback) = 0;
-  virtual std::unique_ptr<BroadcastSession> StartBroadcast(
+  virtual absl::StatusOr<BroadcastSessionId> StartBroadcast(
       BroadcastRequest broadcast_request, BroadcastCallback callback) = 0;
+  virtual void StopBroadcast(BroadcastSessionId session_id) = 0;
 };
 
 }  // namespace presence

presence/implementation/service_controller_impl.cc

@@ -18,8 +18,11 @@
 #include <string>
 #include <utility>
 
+#include "absl/random/random.h"
+#include "absl/status/status.h"
 #include "absl/status/statusor.h"
 #include "internal/platform/implementation/ble_v2.h"
+#include "presence/data_types.h"
 #include "presence/implementation/advertisement_factory.h"
 #include "presence/implementation/base_broadcast_request.h"
 #include "presence/implementation/mediums/advertisement_data.h"
@@ -48,7 +51,7 @@ std::unique_ptr<ScanSession> ServiceControllerImpl::StartScan(
   return scan_manager_.StartScan(scan_request, callback);
 }
 
-std::unique_ptr<BroadcastSession> ServiceControllerImpl::StartBroadcast(
+absl::StatusOr<BroadcastSessionId> ServiceControllerImpl::StartBroadcast(
     BroadcastRequest broadcast_request, BroadcastCallback callback) {
   absl::StatusOr<BaseBroadcastRequest> request =
       BaseBroadcastRequest::Create(broadcast_request);
@@ -56,7 +59,7 @@ std::unique_ptr<BroadcastSession> ServiceControllerImpl::StartBroadcast(
     NEARBY_LOGS(WARNING) << "Invalid broadcast request, reason: "
                          << request.status();
     callback.start_broadcast_cb(Status{Status::Value::kError});
-    return nullptr;
+    return request.status();
   }
   absl::StatusOr<AdvertisementData> advertisement =
       AdvertisementFactory(&credential_manager_).CreateAdvertisement(*request);
@@ -64,7 +67,7 @@ std::unique_ptr<BroadcastSession> ServiceControllerImpl::StartBroadcast(
     NEARBY_LOGS(WARNING) << "Can't create advertisement, reason: "
                          << advertisement.status();
     callback.start_broadcast_cb(Status{Status::Value::kError});
-    return nullptr;
+    return advertisement.status();
   }
   std::unique_ptr<AdvertisingSession> session =
       mediums_.GetBle().StartAdvertising(
@@ -75,15 +78,29 @@ std::unique_ptr<BroadcastSession> ServiceControllerImpl::StartBroadcast(
                                         ConvertBleStatus(status));
                                   }});
   if (!session) {
-    NEARBY_LOGS(WARNING) << "Failed to start broadcasting";
     callback.start_broadcast_cb(Status{Status::Value::kError});
-    return nullptr;
+    return absl::UnavailableError("Failed to start broadcasting");
   }
 
-  return std::make_unique<BroadcastSession>(BroadcastSession{
-      .stop_broadcast_callback = [session = std::move(session)]() {
-        return ConvertBleStatus(session->stop_advertising());
-      }});
+  BroadcastSessionId id = GenerateBroadcastSessionId();
+  sessions_.insert({id, Session{.advertising_session = std::move(session)}});
+
+  return id;
+}
+
+void ServiceControllerImpl::StopBroadcast(BroadcastSessionId id) {
+  auto it = sessions_.find(id);
+  if (it != sessions_.end()) {
+    it->second.advertising_session->stop_advertising();
+    sessions_.erase(it);
+  } else {
+    NEARBY_LOGS(WARNING) << absl::StrFormat(
+        "BroadcastSessionId(0x%x) not found", id);
+  }
+}
+
+BroadcastSessionId ServiceControllerImpl::GenerateBroadcastSessionId() {
+  return absl::Uniform<BroadcastSessionId>(bit_gen_);
 }
 
 }  // namespace presence

presence/implementation/service_controller_impl.h

@@ -17,6 +17,7 @@
 
 #include <memory>
 
+#include "absl/random/random.h"
 #include "presence/broadcast_request.h"
 #include "presence/data_types.h"
 #include "presence/implementation/credential_manager_impl.h"
@@ -33,21 +34,31 @@ namespace presence {
 
 class ServiceControllerImpl : public ServiceController {
  public:
+  using AdvertisingSession =
+      location::nearby::api::ble_v2::BleMedium::AdvertisingSession;
   ServiceControllerImpl() = default;
   std::unique_ptr<ScanSession> StartScan(ScanRequest scan_request,
                                          ScanCallback callback) override;
-  std::unique_ptr<BroadcastSession> StartBroadcast(
+  absl::StatusOr<BroadcastSessionId> StartBroadcast(
       BroadcastRequest broadcast_request, BroadcastCallback callback) override;
+  void StopBroadcast(BroadcastSessionId) override;
 
   // Gives tests access to mediums.
   Mediums& GetMediums() { return mediums_; }
 
  private:
+  struct Session {
+    std::unique_ptr<AdvertisingSession> advertising_session;
+  };
+
+  BroadcastSessionId GenerateBroadcastSessionId();
   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.
+  absl::flat_hash_map<BroadcastSessionId, Session> sessions_;
+  absl::BitGen bit_gen_;
 };
 
 }  // namespace presence

presence/implementation/service_controller_impl_test.cc

@@ -32,6 +32,7 @@ namespace {
 using FeatureFlags = ::location::nearby::FeatureFlags::Flags;
 using internal::IdentityType;
 using ::location::nearby::MediumEnvironment;
+using ::testing::status::StatusIs;
 
 constexpr FeatureFlags kTestCases[] = {
     FeatureFlags{},
@@ -76,12 +77,12 @@ INSTANTIATE_TEST_SUITE_P(ParametrisedServiceControllerImplTest,
                          testing::ValuesIn(kTestCases));
 
 TEST_P(ServiceControllerImplTest, StartBroadcastPublicIdentity) {
-  std::unique_ptr<BroadcastSession> session =
+  absl::StatusOr<BroadcastSessionId> session =
       service_controller_.StartBroadcast(
           CreateBroadcastRequest(internal::IDENTITY_TYPE_PUBLIC),
           broadcast_callback_);
 
-  EXPECT_TRUE(session);
+  EXPECT_OK(session);
   EXPECT_TRUE(start_broadcast_status_.Get().ok());
   EXPECT_EQ(start_broadcast_status_.Get().GetResult(),
             Status{Status::Value::kSuccess});
@@ -92,14 +93,13 @@ TEST_P(ServiceControllerImplTest, StartBroadcastPublicIdentity) {
 }
 
 TEST_P(ServiceControllerImplTest, StartAndStopBroadcast) {
-  std::unique_ptr<BroadcastSession> session =
+  absl::StatusOr<BroadcastSessionId> session =
       service_controller_.StartBroadcast(
           CreateBroadcastRequest(internal::IDENTITY_TYPE_PUBLIC),
           broadcast_callback_);
 
-  ASSERT_TRUE(session);
-  EXPECT_EQ(session->stop_broadcast_callback(),
-            Status{Status::Value::kSuccess});
+  ASSERT_OK(session);
+  service_controller_.StopBroadcast(*session);
   MediumEnvironment::Instance().Sync();
   EXPECT_FALSE(MediumEnvironment::Instance()
                    .GetBleV2MediumStatus(
@@ -107,12 +107,27 @@ TEST_P(ServiceControllerImplTest, StartAndStopBroadcast) {
                    ->is_advertising);
 }
 
+TEST_P(ServiceControllerImplTest, StopBroadcastTwiceNoSideEffects) {
+  absl::StatusOr<BroadcastSessionId> session =
+      service_controller_.StartBroadcast(
+          CreateBroadcastRequest(internal::IDENTITY_TYPE_PUBLIC),
+          broadcast_callback_);
+  ASSERT_OK(session);
+  service_controller_.StopBroadcast(*session);
+
+  service_controller_.StopBroadcast(*session);
+}
+
+TEST_P(ServiceControllerImplTest, StopBroadcastInvalidSessionNoSideEffects) {
+  service_controller_.StopBroadcast(123456);
+}
+
 TEST_P(ServiceControllerImplTest, StartBroadcastInvalidRequestFails) {
-  std::unique_ptr<BroadcastSession> session =
+  absl::StatusOr<BroadcastSessionId> session =
       service_controller_.StartBroadcast(BroadcastRequest{},
                                          broadcast_callback_);
 
-  EXPECT_FALSE(session);
+  EXPECT_THAT(session, StatusIs(absl::StatusCode::kInvalidArgument));
   EXPECT_TRUE(start_broadcast_status_.Get().ok());
   EXPECT_EQ(start_broadcast_status_.Get().GetResult(),
             Status{Status::Value::kError});
@@ -120,12 +135,12 @@ TEST_P(ServiceControllerImplTest, StartBroadcastInvalidRequestFails) {
 
 TEST_P(ServiceControllerImplTest, StartBroadcastPrivateIdentityFails) {
   // TODO(b/256249404): Support private identity.
-  std::unique_ptr<BroadcastSession> session =
+  absl::StatusOr<BroadcastSessionId> session =
       service_controller_.StartBroadcast(
           CreateBroadcastRequest(internal::IDENTITY_TYPE_PRIVATE),
           broadcast_callback_);
 
-  EXPECT_FALSE(session);
+  EXPECT_THAT(session, StatusIs(absl::StatusCode::kUnavailable));
   EXPECT_TRUE(start_broadcast_status_.Get().ok());
   EXPECT_EQ(start_broadcast_status_.Get().GetResult(),
             Status{Status::Value::kError});

presence/presence_client.cc

@@ -15,9 +15,12 @@
 #include "presence/presence_client.h"
 
 #include <memory>
+#include <utility>
 #include <vector>
 
+#include "absl/status/status.h"
 #include "internal/platform/borrowable.h"
+#include "internal/platform/logging.h"
 #include "presence/presence_service.h"
 
 namespace nearby {
@@ -31,14 +34,24 @@ std::unique_ptr<ScanSession> PresenceClient::StartScan(ScanRequest scan_request,
   }
   return (*borrowed)->StartScan(scan_request, callback);
 }
-std::unique_ptr<BroadcastSession> PresenceClient::StartBroadcast(
+absl::StatusOr<BroadcastSessionId> PresenceClient::StartBroadcast(
     BroadcastRequest broadcast_request, BroadcastCallback callback) {
   ::location::nearby::Borrowed<PresenceService*> borrowed = service_.Borrow();
   if (!borrowed) {
-    return nullptr;
+    return absl::FailedPreconditionError(
+        "Can't start broadcast, presence service is gone");
   }
   return (*borrowed)->StartBroadcast(broadcast_request, callback);
 }
 
+void PresenceClient::StopBroadcast(BroadcastSessionId session_id) {
+  ::location::nearby::Borrowed<PresenceService*> borrowed = service_.Borrow();
+  if (borrowed) {
+    (*borrowed)->StopBroadcast(session_id);
+  } else {
+    NEARBY_LOGS(VERBOSE) << "Session already finished, id: " << session_id;
+  }
+}
+
 }  // namespace presence
 }  // namespace nearby

presence/presence_client.h

@@ -19,6 +19,7 @@
 #include <memory>
 #include <vector>
 
+#include "absl/status/statusor.h"
 #include "internal/platform/borrowable.h"
 #include "presence/broadcast_request.h"
 #include "presence/data_types.h"
@@ -59,21 +60,23 @@ class PresenceClient {
   std::unique_ptr<ScanSession> StartScan(ScanRequest scan_request,
                                          ScanCallback callback);
 
-  /**
-   * Starts a Nearby Presence broadcast and registers {@link BroadcastCallback}
-   * which will be invoked after broadcast is started.
-   *
-   * <p>The {@link BroadcastCallback} is kept at the Nearby Presence service.
-   * Returning unique_ptr of BroadcastSession including stop broadcast callback
-   * for the client to invoke later.
-   *
-   * <p>The {@link BroadcastRequest} contains the options like tx_power,
-   * the credential info like salt and private credential, the actions and
-   * extended properties.
-   */
-  std::unique_ptr<BroadcastSession> StartBroadcast(
+  // Starts a Nearby Presence broadcast and registers `BroadcastCallback`
+  // which will be invoked after broadcast is started.
+  // The session can be terminated with `StopBroadcast()`.
+  //
+  // `BroadcastCallback` is kept in the Nearby Presence service until
+  // `StopBroadcast()` is called.
+  //
+  // `BroadcastRequest` contains the options like tx_power,
+  // the credential info like salt and private credential, the actions and
+  // extended properties.
+  absl::StatusOr<BroadcastSessionId> StartBroadcast(
       BroadcastRequest broadcast_request, BroadcastCallback callback);
 
+  // Terminates a broadcast session. Does nothing if the session is already
+  // terminated.
+  void StopBroadcast(BroadcastSessionId session_id);
+
  private:
   BorrowablePresenceService service_;
 };

presence/presence_client_test.cc

@@ -14,6 +14,8 @@
 
 #include "presence/presence_client.h"
 
+#include "gmock/gmock.h"
+#include "protobuf-matchers/protocol-buffer-matchers.h"
 #include "gtest/gtest.h"
 #include "internal/platform/medium_environment.h"
 #include "presence/data_types.h"
@@ -24,6 +26,8 @@ namespace nearby {
 namespace presence {
 namespace {
 
+using ::testing::status::StatusIs;
+
 // Creates a PresenceClient and destroys PresenceService that was used to create
 // it.
 PresenceClient CreateDefunctPresenceClient() {
@@ -46,7 +50,7 @@ TEST_F(PresenceClientTest, StartBroadcastWithDefaultConstructor) {
 
   PresenceService presence_service;
   PresenceClient presence_client = presence_service.CreatePresenceClient();
-  presence_client.StartBroadcast({}, broadcast_callback);
+  auto unused = presence_client.StartBroadcast({}, broadcast_callback);
 
   EXPECT_FALSE(broadcast_result.Ok());
   env_.Stop();
@@ -59,8 +63,10 @@ TEST_F(PresenceClientTest, StartBroadcastFailsWhenPresenceServiceIsGone) {
       .start_broadcast_cb = [&](Status status) { broadcast_result = status; },
   };
 
-  CreateDefunctPresenceClient().StartBroadcast({}, broadcast_callback);
+  absl::StatusOr<BroadcastSessionId> session_id =
+      CreateDefunctPresenceClient().StartBroadcast({}, broadcast_callback);
 
+  EXPECT_THAT(session_id, StatusIs(absl::StatusCode::kFailedPrecondition));
   EXPECT_FALSE(broadcast_result.Ok());
   env_.Stop();
 }

presence/presence_service.cc

@@ -35,10 +35,14 @@ std::unique_ptr<ScanSession> PresenceService::StartScan(
   return service_controller_->StartScan(scan_request, callback);
 }
 
-std::unique_ptr<BroadcastSession> PresenceService::StartBroadcast(
+absl::StatusOr<BroadcastSessionId> PresenceService::StartBroadcast(
     BroadcastRequest broadcast_request, BroadcastCallback callback) {
   return service_controller_->StartBroadcast(broadcast_request, callback);
 }
 
+void PresenceService::StopBroadcast(BroadcastSessionId session) {
+  service_controller_->StopBroadcast(session);
+}
+
 }  // namespace presence
 }  // namespace nearby

presence/presence_service.h

@@ -39,9 +39,11 @@ class PresenceService {
 
   std::unique_ptr<ScanSession> StartScan(ScanRequest scan_request,
                                          ScanCallback callback);
-  std::unique_ptr<BroadcastSession> StartBroadcast(
+  absl::StatusOr<BroadcastSessionId> StartBroadcast(
       BroadcastRequest broadcast_request, BroadcastCallback callback);
 
+  void StopBroadcast(BroadcastSessionId session_id);
+
  private:
   std::unique_ptr<ServiceController> service_controller_;
   ::location::nearby::Lender<PresenceService *> lender_{this};