shill: cellular: Update properties to remove Cellular::Operator usage.
BUG=chromium:428391
TEST=FEATURE="test" emerge-${BOARD} shill
Change-Id: I4fa52c96e7332818f3d5cf5d8c154b986d22278f
Reviewed-on: https://chromium-review.googlesource.com/226354
Reviewed-by: Thieu Le <thieule@chromium.org>
Tested-by: Roshan Pius <rpius@chromium.org>
Commit-Queue: Roshan Pius <rpius@chromium.org>
diff --git a/cellular.cc b/cellular.cc
index cdc630e..e24aa32 100644
--- a/cellular.cc
+++ b/cellular.cc
@@ -1218,8 +1218,7 @@
// These properties have setters that should be used to change their values.
// Events are generated whenever the values change.
- store->RegisterConstStringmap(kHomeProviderProperty,
- &home_provider_.ToDict());
+ store->RegisterConstStringmap(kHomeProviderProperty, &home_provider_);
store->RegisterConstString(kCarrierProperty, &carrier_);
store->RegisterConstBool(kSupportNetworkScanProperty, &scanning_supported_);
store->RegisterConstString(kEsnProperty, &esn_);
@@ -1253,15 +1252,12 @@
&Cellular::SetAllowRoaming);
}
-void Cellular::set_home_provider(const Cellular::Operator &home_provider) {
- if (home_provider_.Equals(home_provider)) {
+void Cellular::set_home_provider(const Stringmap &home_provider) {
+ if (home_provider_ == home_provider)
return;
- }
- home_provider_.CopyFrom(home_provider);
-
- adaptor()->EmitStringmapChanged(kHomeProviderProperty,
- home_provider_.ToDict());
+ home_provider_ = home_provider;
+ adaptor()->EmitStringmapChanged(kHomeProviderProperty, home_provider_);
}
void Cellular::set_carrier(const string &carrier) {
@@ -1488,27 +1484,25 @@
void Cellular::UpdateHomeProvider(const MobileOperatorInfo *operator_info) {
SLOG(Cellular, 3) << __func__;
- // TODO(pprabhu) Change |set_home_provider| to take Stringmap argument and
- // update this.
- Operator oper;
+
+ Stringmap home_provider;
if (!operator_info->sid().empty()) {
- oper.SetCode(operator_info->sid());
+ home_provider[kOperatorCodeKey] = operator_info->sid();
}
if (!operator_info->nid().empty()) {
- oper.SetCode(operator_info->nid());
+ home_provider[kOperatorCodeKey] = operator_info->nid();
}
if (!operator_info->mccmnc().empty()) {
- oper.SetCode(operator_info->mccmnc());
+ home_provider[kOperatorCodeKey] = operator_info->mccmnc();
}
if (!operator_info->operator_name().empty()) {
- oper.SetName(operator_info->operator_name());
+ home_provider[kOperatorNameKey] = operator_info->operator_name();
}
if (!operator_info->country().empty()) {
- oper.SetCountry(operator_info->country());
+ home_provider[kOperatorCountryKey] = operator_info->country();
}
- set_home_provider(oper);
+ set_home_provider(home_provider);
- // Update the APN list.
const ScopedVector<MobileOperatorInfo::MobileAPN> &apn_list =
operator_info->apn_list();
Stringmaps apn_list_dict;
@@ -1550,25 +1544,23 @@
return;
}
- // TODO(pprabhu) Update |CellularService::SetServingOperator| to take
- // Stringmap argument and update this.
- Operator oper;
+ Stringmap serving_operator;
if (!operator_info->sid().empty()) {
- oper.SetCode(operator_info->sid());
+ serving_operator[kOperatorCodeKey] = operator_info->sid();
}
if (!operator_info->nid().empty()) {
- oper.SetCode(operator_info->nid());
+ serving_operator[kOperatorCodeKey] = operator_info->nid();
}
if (!operator_info->mccmnc().empty()) {
- oper.SetCode(operator_info->mccmnc());
+ serving_operator[kOperatorCodeKey] = operator_info->mccmnc();
}
if (!operator_info->operator_name().empty()) {
- oper.SetName(operator_info->operator_name());
+ serving_operator[kOperatorNameKey] = operator_info->operator_name();
}
if (!operator_info->country().empty()) {
- oper.SetCountry(operator_info->country());
+ serving_operator[kOperatorCountryKey] = operator_info->country();
}
- service()->SetServingOperator(oper);
+ service()->set_serving_operator(serving_operator);
// Set friendly name of service.
string service_name;
diff --git a/cellular.h b/cellular.h
index 960ea15..0abad4d 100644
--- a/cellular.h
+++ b/cellular.h
@@ -255,7 +255,7 @@
const std::string &dbus_owner() const { return dbus_owner_; }
const std::string &dbus_service() const { return dbus_service_; }
const std::string &dbus_path() const { return dbus_path_; }
- const Operator &home_provider() const { return home_provider_; }
+ const Stringmap &home_provider() const { return home_provider_; }
const std::string &carrier() const { return carrier_; }
bool scanning_supported() const { return scanning_supported_; }
const std::string &esn() const { return esn_; }
@@ -282,7 +282,7 @@
uint16_t prl_version() const { return prl_version_; }
// setters
- void set_home_provider(const Operator &home_provider);
+ void set_home_provider(const Stringmap &home_provider);
void set_carrier(const std::string &carrier);
void set_scanning_supported(bool scanning_supported);
void set_esn(const std::string &esn);
@@ -525,7 +525,7 @@
const std::string dbus_owner_; // :x.y
const std::string dbus_service_; // org.*.ModemManager*
const std::string dbus_path_; // ModemManager.Modem
- Operator home_provider_;
+ Stringmap home_provider_;
bool scanning_supported_;
std::string carrier_;
diff --git a/cellular_capability_classic_unittest.cc b/cellular_capability_classic_unittest.cc
index 345caa9..306f5f9 100644
--- a/cellular_capability_classic_unittest.cc
+++ b/cellular_capability_classic_unittest.cc
@@ -94,14 +94,13 @@
service->SetStorageIdentifier(kStorageIdentifier);
service->SetFriendlyName(kFriendlyServiceName);
- Cellular::Operator oper;
- oper.SetCode(kOperatorCode);
- oper.SetName(kOperatorName);
- oper.SetCountry(kOperatorCountry);
+ Stringmap serving_operator;
+ serving_operator[kOperatorCodeKey] = kOperatorCode;
+ serving_operator[kOperatorNameKey] = kOperatorName;
+ serving_operator[kOperatorCountryKey] = kOperatorCountry;
- service->SetServingOperator(oper);
-
- cellular_->set_home_provider(oper);
+ service->set_serving_operator(serving_operator);
+ cellular_->set_home_provider(serving_operator);
cellular_->service_ = service;
}
diff --git a/cellular_capability_gsm_unittest.cc b/cellular_capability_gsm_unittest.cc
index c931297..88da97a 100644
--- a/cellular_capability_gsm_unittest.cc
+++ b/cellular_capability_gsm_unittest.cc
@@ -248,14 +248,13 @@
service->SetStorageIdentifier(kStorageIdentifier);
service->SetFriendlyName(kFriendlyServiceName);
- Cellular::Operator oper;
- oper.SetCode(kOperatorCode);
- oper.SetName(kOperatorName);
- oper.SetCountry(kOperatorCountry);
+ Stringmap serving_operator;
+ serving_operator[kOperatorCodeKey] = kOperatorCode;
+ serving_operator[kOperatorNameKey] = kOperatorName;
+ serving_operator[kOperatorCountryKey] = kOperatorCountry;
- service->SetServingOperator(oper);
-
- cellular_->set_home_provider(oper);
+ service->set_serving_operator(serving_operator);
+ cellular_->set_home_provider(serving_operator);
cellular_->service_ = service;
}
diff --git a/cellular_capability_universal_unittest.cc b/cellular_capability_universal_unittest.cc
index e25a1eb..ab8d335 100644
--- a/cellular_capability_universal_unittest.cc
+++ b/cellular_capability_universal_unittest.cc
@@ -133,14 +133,12 @@
service->SetStorageIdentifier(kStorageIdentifier);
service->SetFriendlyName(kFriendlyServiceName);
- Cellular::Operator oper;
- oper.SetCode(kOperatorCode);
- oper.SetName(kOperatorName);
- oper.SetCountry(kOperatorCountry);
-
- service->SetServingOperator(oper);
-
- cellular_->set_home_provider(oper);
+ Stringmap serving_operator;
+ serving_operator[kOperatorCodeKey] = kOperatorCode;
+ serving_operator[kOperatorNameKey] = kOperatorName;
+ serving_operator[kOperatorCountryKey] = kOperatorCountry;
+ service->set_serving_operator(serving_operator);
+ cellular_->set_home_provider(serving_operator);
cellular_->service_ = service;
}
@@ -961,7 +959,9 @@
cellular_->set_modem_state(Cellular::kModemStateConnected);
SetRegistrationDroppedUpdateTimeout(0);
- string home_provider = cellular_->home_provider().GetName();
+ const Stringmap &home_provider_map = cellular_->home_provider();
+ ASSERT_NE(home_provider_map.end(), home_provider_map.find(kOperatorNameKey));
+ string home_provider = home_provider_map.find(kOperatorNameKey)->second;
string ota_name = cellular_->service_->friendly_name();
// Home --> Roaming should be effective immediately.
@@ -1114,7 +1114,9 @@
cellular_->set_modem_state(Cellular::kModemStateRegistered);
SetRegistrationDroppedUpdateTimeout(0);
- string home_provider = cellular_->home_provider().GetName();
+ const Stringmap &home_provider_map = cellular_->home_provider();
+ ASSERT_NE(home_provider_map.end(), home_provider_map.find(kOperatorNameKey));
+ string home_provider = home_provider_map.find(kOperatorNameKey)->second;
string ota_name = cellular_->service_->friendly_name();
// Home --> Searching should be effective immediately.
diff --git a/cellular_service.cc b/cellular_service.cc
index a08bdef..223010c 100644
--- a/cellular_service.cc
+++ b/cellular_service.cc
@@ -77,8 +77,7 @@
nullptr);
store->RegisterConstStringmap(kPaymentPortalProperty, &olp_);
store->RegisterConstString(kRoamingStateProperty, &roaming_state_);
- store->RegisterConstStringmap(kServingOperatorProperty,
- &serving_operator_.ToDict());
+ store->RegisterConstStringmap(kServingOperatorProperty, &serving_operator_);
store->RegisterConstString(kUsageURLProperty, &usage_url_);
store->RegisterString(kCellularPPPUsernameProperty, &ppp_username_);
store->RegisterWriteOnlyString(kCellularPPPPasswordProperty, &ppp_password_);
@@ -441,16 +440,12 @@
adaptor()->EmitStringChanged(kRoamingStateProperty, state);
}
-const Cellular::Operator &CellularService::serving_operator() const {
- return serving_operator_;
-}
-
-void CellularService::SetServingOperator(const Cellular::Operator &oper) {
- if (serving_operator_.Equals(oper)) {
+void CellularService::set_serving_operator(const Stringmap &serving_operator) {
+ if (serving_operator_ == serving_operator)
return;
- }
- serving_operator_.CopyFrom(oper);
- adaptor()->EmitStringmapChanged(kServingOperatorProperty, oper.ToDict());
+
+ serving_operator_ = serving_operator;
+ adaptor()->EmitStringmapChanged(kServingOperatorProperty, serving_operator_);
}
} // namespace shill
diff --git a/cellular_service.h b/cellular_service.h
index 77cbeed..0ac2a20 100644
--- a/cellular_service.h
+++ b/cellular_service.h
@@ -71,8 +71,8 @@
void SetUsageURL(const std::string &url);
const std::string &usage_url() const { return usage_url_; }
- void SetServingOperator(const Cellular::Operator &oper);
- const Cellular::Operator &serving_operator() const;
+ void set_serving_operator(const Stringmap &serving_operator);
+ const Stringmap &serving_operator() const { return serving_operator_; }
// Sets network technology to |technology| and broadcasts the property change.
void SetNetworkTechnology(const std::string &technology);
@@ -201,7 +201,7 @@
// Properties
ActivationType activation_type_;
std::string activation_state_;
- Cellular::Operator serving_operator_;
+ Stringmap serving_operator_;
std::string network_technology_;
std::string roaming_state_;
Stringmap olp_;
diff --git a/cellular_service_unittest.cc b/cellular_service_unittest.cc
index a17f9d3..4439262 100644
--- a/cellular_service_unittest.cc
+++ b/cellular_service_unittest.cc
@@ -163,18 +163,24 @@
}
TEST_F(CellularServiceTest, SetServingOperator) {
- EXPECT_CALL(*adaptor_,
- EmitStringmapChanged(kServingOperatorProperty, _));
static const char kCode[] = "123456";
static const char kName[] = "Some Cellular Operator";
- Cellular::Operator oper;
- service_->SetServingOperator(oper);
- oper.SetCode(kCode);
- oper.SetName(kName);
- service_->SetServingOperator(oper);
- EXPECT_EQ(kCode, service_->serving_operator().GetCode());
- EXPECT_EQ(kName, service_->serving_operator().GetName());
- service_->SetServingOperator(oper);
+ Stringmap test_operator;
+ service_->set_serving_operator(test_operator);
+ test_operator[kOperatorCodeKey] = kCode;
+ test_operator[kOperatorNameKey] = kName;
+ EXPECT_CALL(*adaptor_,
+ EmitStringmapChanged(kServingOperatorProperty, _));
+ service_->set_serving_operator(test_operator);
+ const Stringmap &serving_operator = service_->serving_operator();
+ ASSERT_NE(serving_operator.end(), serving_operator.find(kOperatorCodeKey));
+ ASSERT_NE(serving_operator.end(), serving_operator.find(kOperatorNameKey));
+ EXPECT_EQ(kCode, serving_operator.find(kOperatorCodeKey)->second);
+ EXPECT_EQ(kName, serving_operator.find(kOperatorNameKey)->second);
+ Mock::VerifyAndClearExpectations(adaptor_);
+ EXPECT_CALL(*adaptor_,
+ EmitStringmapChanged(kServingOperatorProperty, _)).Times(0);
+ service_->set_serving_operator(serving_operator);
}
TEST_F(CellularServiceTest, SetOLP) {
diff --git a/cellular_unittest.cc b/cellular_unittest.cc
index e8f9e39..fc98681 100644
--- a/cellular_unittest.cc
+++ b/cellular_unittest.cc
@@ -419,6 +419,36 @@
return mm1_proxy_.get(); // Before the capability snags it.
}
+ void VerifyOperatorMap(const Stringmap &operator_map,
+ const string &code,
+ const string &name,
+ const string &country) {
+ Stringmap::const_iterator it;
+ Stringmap::const_iterator endit = operator_map.end();
+
+ it = operator_map.find(kOperatorCodeKey);
+ if (code == "") {
+ EXPECT_EQ(endit, it);
+ } else {
+ ASSERT_NE(endit, it);
+ EXPECT_EQ(code, it->second);
+ }
+ it = operator_map.find(kOperatorNameKey);
+ if (name == "") {
+ EXPECT_EQ(endit, it);
+ } else {
+ ASSERT_NE(endit, it);
+ EXPECT_EQ(name, it->second);
+ }
+ it = operator_map.find(kOperatorCountryKey);
+ if (country == "") {
+ EXPECT_EQ(endit, it);
+ } else {
+ ASSERT_NE(endit, it);
+ EXPECT_EQ(country, it->second);
+ }
+ }
+
MOCK_METHOD1(TestCallback, void(const Error &error));
protected:
@@ -907,6 +937,9 @@
SetMockMobileOperatorInfoObjects();
CHECK(mock_home_provider_info_);
CHECK(mock_serving_operator_info_);
+ Stringmap home_provider;
+ Stringmap serving_operator;
+
// (1) Neither home provider nor serving operator known.
EXPECT_CALL(*mock_home_provider_info_, IsMobileNetworkOperatorKnown())
@@ -916,12 +949,10 @@
device_->CreateService();
- EXPECT_EQ("", device_->home_provider().GetCode());
- EXPECT_EQ("", device_->home_provider().GetName());
- EXPECT_EQ("", device_->home_provider().GetCountry());
- EXPECT_EQ("", device_->service_->serving_operator().GetCode());
- EXPECT_EQ("", device_->service_->serving_operator().GetName());
- EXPECT_EQ("", device_->service_->serving_operator().GetCountry());
+ home_provider = device_->home_provider();
+ VerifyOperatorMap(home_provider, "", "", "");
+ serving_operator = device_->service_->serving_operator();
+ VerifyOperatorMap(serving_operator, "", "", "");
Mock::VerifyAndClearExpectations(mock_home_provider_info_);
Mock::VerifyAndClearExpectations(mock_serving_operator_info_);
device_->DestroyService();
@@ -942,15 +973,16 @@
device_->CreateService();
- EXPECT_EQ(kServingOperatorCode, device_->home_provider().GetCode());
- EXPECT_EQ(kServingOperatorName, device_->home_provider().GetName());
- EXPECT_EQ(kServingOperatorCountry, device_->home_provider().GetCountry());
- EXPECT_EQ(kServingOperatorCode,
- device_->service_->serving_operator().GetCode());
- EXPECT_EQ(kServingOperatorName,
- device_->service_->serving_operator().GetName());
- EXPECT_EQ(kServingOperatorCountry,
- device_->service_->serving_operator().GetCountry());
+ home_provider = device_->home_provider();
+ VerifyOperatorMap(home_provider,
+ kServingOperatorCode,
+ kServingOperatorName,
+ kServingOperatorCountry);
+ serving_operator = device_->service_->serving_operator();
+ VerifyOperatorMap(serving_operator,
+ kServingOperatorCode,
+ kServingOperatorName,
+ kServingOperatorCountry);
Mock::VerifyAndClearExpectations(mock_home_provider_info_);
Mock::VerifyAndClearExpectations(mock_serving_operator_info_);
device_->DestroyService();
@@ -971,15 +1003,16 @@
device_->CreateService();
- EXPECT_EQ(kHomeProviderCode, device_->home_provider().GetCode());
- EXPECT_EQ(kHomeProviderName, device_->home_provider().GetName());
- EXPECT_EQ(kHomeProviderCountry, device_->home_provider().GetCountry());
- EXPECT_EQ(kHomeProviderCode,
- device_->service_->serving_operator().GetCode());
- EXPECT_EQ(kHomeProviderName,
- device_->service_->serving_operator().GetName());
- EXPECT_EQ(kHomeProviderCountry,
- device_->service_->serving_operator().GetCountry());
+ home_provider = device_->home_provider();
+ VerifyOperatorMap(home_provider,
+ kHomeProviderCode,
+ kHomeProviderName,
+ kHomeProviderCountry);
+ serving_operator = device_->service_->serving_operator();
+ VerifyOperatorMap(serving_operator,
+ kHomeProviderCode,
+ kHomeProviderName,
+ kHomeProviderCountry);
Mock::VerifyAndClearExpectations(mock_home_provider_info_);
Mock::VerifyAndClearExpectations(mock_serving_operator_info_);
device_->DestroyService();
@@ -1006,15 +1039,16 @@
device_->CreateService();
- EXPECT_EQ(kHomeProviderCode, device_->home_provider().GetCode());
- EXPECT_EQ(kHomeProviderName, device_->home_provider().GetName());
- EXPECT_EQ(kHomeProviderCountry, device_->home_provider().GetCountry());
- EXPECT_EQ(kServingOperatorCode,
- device_->service_->serving_operator().GetCode());
- EXPECT_EQ(kServingOperatorName,
- device_->service_->serving_operator().GetName());
- EXPECT_EQ(kServingOperatorCountry,
- device_->service_->serving_operator().GetCountry());
+ home_provider = device_->home_provider();
+ VerifyOperatorMap(home_provider,
+ kHomeProviderCode,
+ kHomeProviderName,
+ kHomeProviderCountry);
+ serving_operator = device_->service_->serving_operator();
+ VerifyOperatorMap(serving_operator,
+ kServingOperatorCode,
+ kServingOperatorName,
+ kServingOperatorCountry);
}
static bool IllegalChar(char a) {