From 363d2a819e861bac73bc0a0134e9c6f0e515c1ac Mon Sep 17 00:00:00 2001 From: Qin Wang Date: Thu, 18 May 2023 12:45:01 -0700 Subject: [PATCH] Pure refactor set/get functions in FastPairDevice PiperOrigin-RevId: 533218929 --- fastpair/common/fast_pair_device.cc | 4 +-- fastpair/common/fast_pair_device.h | 24 +++++++------ fastpair/common/fast_pair_device_test.cc | 34 +++++++------------ fastpair/fast_pair_controller.cc | 2 +- .../handshake/fast_pair_handshake_impl.cc | 2 +- .../fast_pair_handshake_impl_test.cc | 2 +- .../handshake/fast_pair_handshake_lookup.cc | 4 +-- .../fast_pair_handshake_lookup_test.cc | 2 +- fastpair/message_stream/medium.cc | 11 +++--- fastpair/message_stream/medium_test.cc | 16 ++++----- .../message_stream/message_stream_test.cc | 2 +- .../ui/fast_pair/fast_pair_presenter_impl.cc | 2 +- .../fast_pair_presenter_impl_test.cc | 4 +-- 13 files changed, 51 insertions(+), 58 deletions(-) diff --git a/fastpair/common/fast_pair_device.cc b/fastpair/common/fast_pair_device.cc index 0b17bb63..b3f93680 100644 --- a/fastpair/common/fast_pair_device.cc +++ b/fastpair/common/fast_pair_device.cc @@ -26,8 +26,8 @@ namespace fastpair { std::ostream& operator<<(std::ostream& stream, const FastPairDevice& device) { stream << "[Device: model_id = " << device.GetModelId() << ", ble_address = " << device.GetBleAddress() - << ", public_address = " << device.public_address().value_or("null") - << ", display_name = " << device.display_name().value_or("null") + << ", public_address = " << device.GetPublicAddress().value_or("null") + << ", display_name = " << device.GetDisplayName().value_or("null") << ", " << device.GetAccountKey() << ", protocol = " << device.GetProtocol() << "]"; diff --git a/fastpair/common/fast_pair_device.h b/fastpair/common/fast_pair_device.h index 36dcd3e7..ea11347a 100644 --- a/fastpair/common/fast_pair_device.h +++ b/fastpair/common/fast_pair_device.h @@ -23,6 +23,7 @@ #include #include "absl/container/flat_hash_map.h" +#include "absl/strings/string_view.h" #include "fastpair/common/account_key.h" #include "fastpair/common/protocol.h" @@ -48,25 +49,23 @@ class FastPairDevice { FastPairDevice& operator=(FastPairDevice&&) = delete; ~FastPairDevice() = default; - const std::optional& public_address() const { + std::optional GetPublicAddress() const { return public_address_; } - void set_public_address(absl::string_view address) { + void SetPublicAddress(absl::string_view address) { public_address_ = std::string(address); } - const std::optional& display_name() const { - return display_name_; + std::optional GetDisplayName() const { return display_name_; } + + void SetDisplayName(absl::string_view display_name) { + display_name_ = std::string(display_name); } - void set_display_name(const std::optional& display_name) { - display_name_ = display_name; - } + std::optional GetVersion() { return version_; } - std::optional version() { return version_; } - - void set_version(std::optional version) { + void SetVersion(std::optional version) { version_ = version; } @@ -74,7 +73,10 @@ class FastPairDevice { void SetAccountKey(AccountKey account_key) { account_key_ = account_key; } - void SetModelId(absl::string_view model_id) { model_id_ = model_id; } + void SetModelId(absl::string_view model_id) { + model_id_ = std::string(model_id); + } + absl::string_view GetModelId() const { return model_id_; } void SetBleAddress(absl::string_view address) { diff --git a/fastpair/common/fast_pair_device_test.cc b/fastpair/common/fast_pair_device_test.cc index 62d5505f..9c54610c 100644 --- a/fastpair/common/fast_pair_device_test.cc +++ b/fastpair/common/fast_pair_device_test.cc @@ -46,44 +46,34 @@ TEST(FastPairDevice, GetAndSetName) { FastPairDevice device("model_id", "ble_address", Protocol::kFastPairInitialPairing); // Test that name returns null before any sets. - std::optional name = device.display_name(); - EXPECT_FALSE(name.has_value()); + EXPECT_FALSE(device.GetDisplayName().has_value()); // Test that name returns the set value. std::string test_name = "test_name"; - device.set_display_name(test_name); - name = device.display_name(); - EXPECT_TRUE(name.has_value()); - EXPECT_EQ(name.value(), test_name); + device.SetDisplayName(test_name); + EXPECT_EQ(device.GetDisplayName().value(), test_name); // Test that overriding works. std::string new_test_name = "new_test_name"; - device.set_display_name(new_test_name); - name = device.display_name(); - EXPECT_TRUE(name.has_value()); - EXPECT_EQ(name.value(), new_test_name); + device.SetDisplayName(new_test_name); + EXPECT_EQ(device.GetDisplayName().value(), new_test_name); } TEST(FastPairDevice, GetAndPublicAddress) { FastPairDevice device("model_id", "ble_address", Protocol::kFastPairInitialPairing); // Test that public address returns null before any sets. - std::optional public_address = device.public_address(); - EXPECT_FALSE(public_address.has_value()); + EXPECT_FALSE(device.GetPublicAddress().has_value()); // Test that name returns the set value. - std::string test_public_address = "test_public_address "; - device.set_public_address(test_public_address); - public_address = device.public_address(); - EXPECT_TRUE(public_address.has_value()); - EXPECT_EQ(public_address.value(), test_public_address); + std::string test_GetPublicAddress = "test_GetPublicAddress "; + device.SetPublicAddress(test_GetPublicAddress); + EXPECT_EQ(device.GetPublicAddress().value(), test_GetPublicAddress); // Test that overriding works. - std::string new_test_public_address = "new_test_public_address "; - device.set_public_address(new_test_public_address); - public_address = device.public_address(); - EXPECT_TRUE(public_address.has_value()); - EXPECT_EQ(public_address.value(), new_test_public_address); + std::string new_test_GetPublicAddress = "new_test_GetPublicAddress "; + device.SetPublicAddress(new_test_GetPublicAddress); + EXPECT_EQ(device.GetPublicAddress().value(), new_test_GetPublicAddress); } } // namespace diff --git a/fastpair/fast_pair_controller.cc b/fastpair/fast_pair_controller.cc index dac8e40a..6a9e297a 100644 --- a/fastpair/fast_pair_controller.cc +++ b/fastpair/fast_pair_controller.cc @@ -33,7 +33,7 @@ namespace fastpair { FastPairController::FastPairController(Mediums* mediums, const BluetoothDevice& device) : mediums_(mediums), device_(Protocol::kFastPairRetroactivePairing) { - device_.set_public_address(device.GetMacAddress()); + device_.SetPublicAddress(device.GetMacAddress()); } absl::Status FastPairController::OpenMessageStream() { diff --git a/fastpair/handshake/fast_pair_handshake_impl.cc b/fastpair/handshake/fast_pair_handshake_impl.cc index 734c7112..33f79739 100644 --- a/fastpair/handshake/fast_pair_handshake_impl.cc +++ b/fastpair/handshake/fast_pair_handshake_impl.cc @@ -126,7 +126,7 @@ void FastPairHandshakeImpl::OnParseDecryptedResponse( NEARBY_LOGS(INFO) << __func__ << ": Successfully decrypted and parsed response."; - device.set_public_address( + device.SetPublicAddress( device::CanonicalizeBluetoothAddress(response->address_bytes)); completed_successfully_ = true; std::move(on_complete_callback_)(device, absl::nullopt); diff --git a/fastpair/handshake/fast_pair_handshake_impl_test.cc b/fastpair/handshake/fast_pair_handshake_impl_test.cc index a6a50e12..79c842ac 100644 --- a/fastpair/handshake/fast_pair_handshake_impl_test.cc +++ b/fastpair/handshake/fast_pair_handshake_impl_test.cc @@ -225,7 +225,7 @@ TEST_F(FastPairHandshakeImplTest, Success) { device, mediums, [&](FastPairDevice& callback_device, std::optional failure) { EXPECT_EQ(&device, &callback_device); - EXPECT_EQ(device.public_address(), kPublicAddress); + EXPECT_EQ(device.GetPublicAddress(), kPublicAddress); EXPECT_FALSE(failure.has_value()); latch.CountDown(); }); diff --git a/fastpair/handshake/fast_pair_handshake_lookup.cc b/fastpair/handshake/fast_pair_handshake_lookup.cc index 93dc075e..4b748fe3 100644 --- a/fastpair/handshake/fast_pair_handshake_lookup.cc +++ b/fastpair/handshake/fast_pair_handshake_lookup.cc @@ -46,7 +46,7 @@ FastPairHandshake* FastPairHandshakeLookup::Get(FastPairDevice* device) { FastPairHandshake* FastPairHandshakeLookup::Get(absl::string_view address) { absl::MutexLock lock(&mutex_); for (const auto& pair : fast_pair_handshakes_) { - if (pair.first->public_address() == address || + if (pair.first->GetPublicAddress() == address || pair.first->GetBleAddress() == address) { return pair.second.get(); } @@ -62,7 +62,7 @@ bool FastPairHandshakeLookup::Erase(FastPairDevice* device) { bool FastPairHandshakeLookup::Erase(absl::string_view address) { absl::MutexLock lock(&mutex_); for (const auto& pair : fast_pair_handshakes_) { - if (pair.first->public_address() == address || + if (pair.first->GetPublicAddress() == address || pair.first->GetBleAddress() == address) { fast_pair_handshakes_.erase(pair.first); return true; diff --git a/fastpair/handshake/fast_pair_handshake_lookup_test.cc b/fastpair/handshake/fast_pair_handshake_lookup_test.cc index bab98bfc..29e9d3a5 100644 --- a/fastpair/handshake/fast_pair_handshake_lookup_test.cc +++ b/fastpair/handshake/fast_pair_handshake_lookup_test.cc @@ -45,7 +45,7 @@ class FastPairHandshakeLookupTest : public ::testing::Test { provider_address_ = adapter_.GetMacAddress(); device_ = new FastPairDevice(kValidModelId, provider_address_, Protocol::kFastPairInitialPairing); - device_->set_public_address(kPubliceAddress); + device_->SetPublicAddress(kPubliceAddress); } ~FastPairHandshakeLookupTest() override { delete device_; } diff --git a/fastpair/message_stream/medium.cc b/fastpair/message_stream/medium.cc index a2494ea2..4d9fdcc4 100644 --- a/fastpair/message_stream/medium.cc +++ b/fastpair/message_stream/medium.cc @@ -37,7 +37,7 @@ absl::Status Medium::OpenRfcomm() { if (!bt_classic_medium_.has_value()) { return absl::FailedPreconditionError("BT classic unsupported"); } - if (!device_.public_address().has_value()) { + if (!device_.GetPublicAddress().has_value()) { return absl::FailedPreconditionError( "Connect open RFCOMM without public BT address"); } @@ -45,10 +45,11 @@ absl::Status Medium::OpenRfcomm() { executor_.Execute("open-rfcomm", [this, classic_medium]() { if (cancellation_flag_.Cancelled()) return; BluetoothDevice device = - classic_medium->GetRemoteDevice(device_.public_address().value()); + classic_medium->GetRemoteDevice(device_.GetPublicAddress().value()); if (!device.IsValid()) { - observer_.OnConnectionResult(absl::UnavailableError(absl::StrFormat( - "Remote BT device %s not found", device_.public_address().value()))); + observer_.OnConnectionResult(absl::UnavailableError( + absl::StrFormat("Remote BT device %s not found", + device_.GetPublicAddress().value()))); return; } SetSocket(classic_medium->ConnectToService(device, kRfcommUuid, @@ -58,7 +59,7 @@ absl::Status Medium::OpenRfcomm() { ? absl::OkStatus() : absl::UnavailableError(absl::StrFormat( "Failed to open RFCOMM with %s", - device_.public_address().value())); + device_.GetPublicAddress().value())); observer_.OnConnectionResult(status); if (status.ok()) { RunLoop(std::move(socket)); diff --git a/fastpair/message_stream/medium_test.cc b/fastpair/message_stream/medium_test.cc index 1fb68b49..a23208f6 100644 --- a/fastpair/message_stream/medium_test.cc +++ b/fastpair/message_stream/medium_test.cc @@ -72,7 +72,7 @@ class MediumTest : public testing::Test { TEST_F(MediumTest, ConnectWithNonExistingDeviceFails) { FastPairDevice fp_device("model id", "ble address", Protocol::kFastPairRetroactivePairing); - fp_device.set_public_address("11:22:33:44:55:66"); + fp_device.SetPublicAddress("11:22:33:44:55:66"); Medium medium = Medium(fp_device, std::optional(&seeker_medium_), observer_); @@ -86,7 +86,7 @@ TEST_F(MediumTest, ConnectWithNonExistingDeviceFails) { TEST_F(MediumTest, Connect) { FastPairDevice fp_device("model id", "ble address", Protocol::kFastPairRetroactivePairing); - fp_device.set_public_address(provider_.GetMacAddress()); + fp_device.SetPublicAddress(provider_.GetMacAddress()); provider_.DiscoverProvider(seeker_medium_); provider_.EnableProviderRfcomm(); Medium medium = @@ -101,7 +101,7 @@ TEST_F(MediumTest, Connect) { TEST_F(MediumTest, ProviderDisconnectsCallsOnDisconnectCallback) { FastPairDevice fp_device("model id", "ble address", Protocol::kFastPairRetroactivePairing); - fp_device.set_public_address(provider_.GetMacAddress()); + fp_device.SetPublicAddress(provider_.GetMacAddress()); provider_.DiscoverProvider(seeker_medium_); provider_.EnableProviderRfcomm(); Medium medium = @@ -120,7 +120,7 @@ TEST_F(MediumTest, ProviderDisconnectsCallsOnDisconnectCallback) { TEST_F(MediumTest, DisconnectSendFails) { FastPairDevice fp_device("model id", "ble address", Protocol::kFastPairRetroactivePairing); - fp_device.set_public_address(provider_.GetMacAddress()); + fp_device.SetPublicAddress(provider_.GetMacAddress()); provider_.DiscoverProvider(seeker_medium_); provider_.EnableProviderRfcomm(); Medium medium = @@ -145,7 +145,7 @@ TEST_F(MediumTest, SendMessage) { std::string expected_result = absl::HexStringToBytes("030A0003ABCDEF"); FastPairDevice fp_device("model id", "ble address", Protocol::kFastPairRetroactivePairing); - fp_device.set_public_address(provider_.GetMacAddress()); + fp_device.SetPublicAddress(provider_.GetMacAddress()); provider_.DiscoverProvider(seeker_medium_); provider_.EnableProviderRfcomm(); Medium medium = @@ -170,7 +170,7 @@ TEST_F(MediumTest, ReceiveMessage) { std::string input = absl::HexStringToBytes("030A0003ABCDEF"); FastPairDevice fp_device("model id", "ble address", Protocol::kFastPairRetroactivePairing); - fp_device.set_public_address(provider_.GetMacAddress()); + fp_device.SetPublicAddress(provider_.GetMacAddress()); provider_.DiscoverProvider(seeker_medium_); provider_.EnableProviderRfcomm(); Medium medium = @@ -192,7 +192,7 @@ class MediumFuzzTest : public fuzztest::PerIterationFixtureAdapter { void HandlesAnyInput(absl::string_view input) { FastPairDevice fp_device("model id", "ble address", Protocol::kFastPairRetroactivePairing); - fp_device.set_public_address(provider_.GetMacAddress()); + fp_device.SetPublicAddress(provider_.GetMacAddress()); provider_.DiscoverProvider(seeker_medium_); provider_.EnableProviderRfcomm(); Medium medium = Medium( @@ -213,7 +213,7 @@ class MediumFuzzTest : public fuzztest::PerIterationFixtureAdapter { .payload = std::string(payload)}; FastPairDevice fp_device("model id", "ble address", Protocol::kFastPairRetroactivePairing); - fp_device.set_public_address(provider_.GetMacAddress()); + fp_device.SetPublicAddress(provider_.GetMacAddress()); provider_.DiscoverProvider(seeker_medium_); provider_.EnableProviderRfcomm(); Medium medium = Medium( diff --git a/fastpair/message_stream/message_stream_test.cc b/fastpair/message_stream/message_stream_test.cc index c5c656b5..f856bc5f 100644 --- a/fastpair/message_stream/message_stream_test.cc +++ b/fastpair/message_stream/message_stream_test.cc @@ -111,7 +111,7 @@ class MessageStreamTest : public testing::Test { void SetUp() override { MediumEnvironment::Instance().Start(); - fp_device_.set_public_address(provider_.GetMacAddress()); + fp_device_.SetPublicAddress(provider_.GetMacAddress()); provider_.DiscoverProvider(seeker_medium_); provider_.EnableProviderRfcomm(); } diff --git a/fastpair/ui/fast_pair/fast_pair_presenter_impl.cc b/fastpair/ui/fast_pair/fast_pair_presenter_impl.cc index e8990454..91dba042 100644 --- a/fastpair/ui/fast_pair/fast_pair_presenter_impl.cc +++ b/fastpair/ui/fast_pair/fast_pair_presenter_impl.cc @@ -64,7 +64,7 @@ void FastPairPresenterImpl::ShowDiscovery( void FastPairPresenterImpl::OnDiscoveryMetadataRetrieved( FastPairDevice& device, const DeviceMetadata& device_metadata, FastPairNotificationController& notification_controller) { - device.set_version(device_metadata.GetFastPairVersion()); + device.SetVersion(device_metadata.GetFastPairVersion()); notification_controller.ShowGuestDiscoveryNotification(device_metadata, std::move(callback_)); } diff --git a/fastpair/ui/fast_pair/fast_pair_presenter_impl_test.cc b/fastpair/ui/fast_pair/fast_pair_presenter_impl_test.cc index 84bc68e9..e110f772 100644 --- a/fastpair/ui/fast_pair/fast_pair_presenter_impl_test.cc +++ b/fastpair/ui/fast_pair/fast_pair_presenter_impl_test.cc @@ -58,7 +58,7 @@ TEST(FastPairPresenterImplTest, ShowDiscoveryForV1Version) { }); latch_1->Await(); EXPECT_EQ(notification_controller_observer.on_update_device_count(), 1); - EXPECT_EQ(device.version(), DeviceFastPairVersion::kV1); + EXPECT_EQ(device.GetVersion(), DeviceFastPairVersion::kV1); controller.OnDiscoveryClicked(DiscoveryAction::kPairToDevice); latch_2.Await(); EXPECT_EQ(discovery_action, DiscoveryAction::kPairToDevice); @@ -92,7 +92,7 @@ TEST(FastPairPresenterImplTest, ShowDiscoveryForHigherThanV1Version) { }); latch_1->Await(); EXPECT_EQ(notification_controller_observer.on_update_device_count(), 1); - EXPECT_EQ(device.version(), DeviceFastPairVersion::kHigherThanV1); + EXPECT_EQ(device.GetVersion(), DeviceFastPairVersion::kHigherThanV1); controller.OnDiscoveryClicked(DiscoveryAction::kDismissedByUser); latch_2.Await(); EXPECT_EQ(discovery_action, DiscoveryAction::kDismissedByUser);