shill: Use Operator ID when setting home provider.
CellularCapabilityUniversal now takes Operator ID into account when
setting the home provider. This may remedy the issue of not being able
to show a proper service name when home_provider data is unknown due to
a lack of imsi.
BUG=chromium-os:36475,chrome-os-partner:17310
TEST=1. Build and run unit tests.
2. Make sure that cellular connection works as it used to.
3. (Optional) Use pseudomodem to have the SIM report no operator
id and verify that the MCCMNC from the database is used. Also
verify that shill uses the operator id from the SIM if it is
present.
Change-Id: Ic02fb6bc506e6fce160b67540d5a319ee5906e9a
Reviewed-on: https://gerrit.chromium.org/gerrit/42426
Reviewed-by: Ben Chan <benchan@chromium.org>
Commit-Queue: Arman Uguray <armansito@chromium.org>
Tested-by: Arman Uguray <armansito@chromium.org>
diff --git a/cellular.h b/cellular.h
index 9ced902..c9180c3 100644
--- a/cellular.h
+++ b/cellular.h
@@ -238,6 +238,7 @@
FRIEND_TEST(CellularCapabilityUniversalTest, CreateFriendlyServiceName);
FRIEND_TEST(CellularCapabilityUniversalTest, Connect);
FRIEND_TEST(CellularCapabilityUniversalTest, IsServiceActivationRequired);
+ FRIEND_TEST(CellularCapabilityUniversalTest, SetHomeProvider);
FRIEND_TEST(CellularCapabilityUniversalTest, StopModemConnected);
FRIEND_TEST(CellularCapabilityUniversalTest, UpdateOLP);
FRIEND_TEST(CellularCapabilityUniversalTest, UpdateOperatorInfoViaOperatorId);
diff --git a/cellular_capability_universal.cc b/cellular_capability_universal.cc
index 45ffcf9..bcf1330 100644
--- a/cellular_capability_universal.cc
+++ b/cellular_capability_universal.cc
@@ -567,16 +567,19 @@
void CellularCapabilityUniversal::SetHomeProvider() {
SLOG(Cellular, 2) << __func__ << "(IMSI: " << imsi_
<< " SPN: " << spn_ << ")";
- // TODO(petkov): The test for NULL provider_db should be done by
- // mobile_provider_lookup_best_match.
- if (imsi_.empty() || !cellular()->provider_db()) {
+
+ if (!cellular()->provider_db())
return;
- }
- mobile_provider *provider =
- mobile_provider_lookup_best_match(
- cellular()->provider_db(), spn_.c_str(), imsi_.c_str());
+
+ // MCCMNC can be determined either from IMSI or Operator Code. Use whichever
+ // one is available. If both were reported by the SIM, use IMSI.
+ const string &network_id = imsi_.empty() ? operator_id_ : imsi_;
+ mobile_provider *provider = mobile_provider_lookup_best_match(
+ cellular()->provider_db(),
+ spn_.c_str(),
+ network_id.c_str());
if (!provider) {
- SLOG(Cellular, 2) << "GSM provider not found.";
+ SLOG(Cellular, 2) << "3GPP provider not found.";
return;
}
@@ -585,7 +588,11 @@
home_provider_ = provider;
provider_requires_roaming_ = home_provider_->requires_roaming;
Cellular::Operator oper;
- if (provider->networks && provider->networks[0]) {
+ // If Operator ID is available, use that as network code, otherwise
+ // use what was returned from the database.
+ if (!operator_id_.empty()) {
+ oper.SetCode(operator_id_);
+ } else if (provider->networks && provider->networks[0]) {
oper.SetCode(provider->networks[0]);
}
if (provider->country) {
@@ -1479,24 +1486,26 @@
const vector<string> &/* invalidated_properties */) {
SLOG(Cellular, 2) << __func__;
string value;
- bool must_update_home_provider = false;
if (DBusProperties::GetString(props, MM_SIM_PROPERTY_SIMIDENTIFIER, &value))
OnSimIdentifierChanged(value);
if (DBusProperties::GetString(props, MM_SIM_PROPERTY_OPERATORIDENTIFIER,
&value))
OnOperatorIdChanged(value);
- if (DBusProperties::GetString(props, MM_SIM_PROPERTY_OPERATORNAME, &value)) {
- spn_ = value;
- must_update_home_provider = true;
- }
- if (DBusProperties::GetString(props, MM_SIM_PROPERTY_IMSI, &value)) {
- imsi_ = value;
- must_update_home_provider = true;
- }
- // TODO(jglasgow): May eventually want to get SPDI, etc
+ if (DBusProperties::GetString(props, MM_SIM_PROPERTY_OPERATORNAME, &value))
+ OnSpnChanged(value);
+ if (DBusProperties::GetString(props, MM_SIM_PROPERTY_IMSI, &value))
+ OnImsiChanged(value);
+ SetHomeProvider();
+}
- if (must_update_home_provider)
- SetHomeProvider();
+// TODO(armansito): The following methods should probably log their argument
+// values. Need to learn if any of them need to be scrubbed.
+void CellularCapabilityUniversal::OnImsiChanged(const std::string &imsi) {
+ imsi_ = imsi;
+}
+
+void CellularCapabilityUniversal::OnSpnChanged(const std::string &spn) {
+ spn_ = spn;
}
void CellularCapabilityUniversal::OnSimIdentifierChanged(const string &id) {
diff --git a/cellular_capability_universal.h b/cellular_capability_universal.h
index 84ef2de..747c111 100644
--- a/cellular_capability_universal.h
+++ b/cellular_capability_universal.h
@@ -171,12 +171,10 @@
void Stop_DisableCompleted(const ResultCallback &callback,
const Error &error);
- // Sets the upper level information about the home cellular provider from the
- // modem's IMSI and SPN.
- void SetHomeProvider();
-
void UpdateScanningProperty();
+ // Methods used in acquiring information related to the carrier;
+
// Updates the online payment portal information, if any, for the cellular
// provider.
void UpdateOLP();
@@ -185,17 +183,22 @@
// obtained network id.
void UpdateOperatorInfo();
+ // Sets the upper level information about the home cellular provider from the
+ // modem's IMSI and SPN.
+ void SetHomeProvider();
+
// Updates the serving operator on the active service.
void UpdateServingOperator();
+ // Initializes the |apn_list_| property based on the current |home_provider_|.
+ void InitAPNList();
+
// Updates |bearer_path_| to match the currently active bearer.
void UpdateBearerPath();
// Updates the storage identifier used for the current cellular service.
void UpdateStorageIdentifier();
- // Initializes the |apn_list_| property based on the current |home_provider_|.
- void InitAPNList();
Stringmap ParseScanResult(const ScanResult &result);
@@ -261,6 +264,7 @@
const DBusPropertiesMap &props,
const std::vector<std::string> &invalidated_properties);
void OnImsiChanged(const std::string &imsi);
+ void OnSpnChanged(const std::string &spn);
void OnSimIdentifierChanged(const std::string &id);
void OnOperatorIdChanged(const std::string &operator_id);
void OnOperatorNameChanged(const std::string &operator_name);
diff --git a/cellular_capability_universal_unittest.cc b/cellular_capability_universal_unittest.cc
index 2eaf186..f7bf893 100644
--- a/cellular_capability_universal_unittest.cc
+++ b/cellular_capability_universal_unittest.cc
@@ -663,7 +663,6 @@
// Updating the SIM
DBusPropertiesMap new_properties;
const char kCountry[] = "us";
- const char kCode[] = "310160";
const char kNewImsi[] = "310240123456789";
const char kSimIdentifier[] = "9999888";
const char kOperatorIdentifier[] = "310240";
@@ -682,7 +681,7 @@
EXPECT_EQ("", capability_->spn_);
EXPECT_EQ("T-Mobile", cellular_->home_provider().GetName());
EXPECT_EQ(kCountry, cellular_->home_provider().GetCountry());
- EXPECT_EQ(kCode, cellular_->home_provider().GetCode());
+ EXPECT_EQ(kOperatorIdentifier, cellular_->home_provider().GetCode());
EXPECT_EQ(4, capability_->apn_list_.size());
new_properties[MM_SIM_PROPERTY_OPERATORNAME].writer().
@@ -998,18 +997,43 @@
static const char kTestCarrier[] = "The Cellular Carrier";
static const char kCountry[] = "us";
static const char kCode[] = "310160";
- capability_->imsi_ = "310240123456789";
EXPECT_FALSE(capability_->home_provider_);
EXPECT_FALSE(capability_->provider_requires_roaming_);
- capability_->SetHomeProvider(); // No mobile provider DB available.
+ // No mobile provider DB available.
+ capability_->SetHomeProvider();
EXPECT_TRUE(cellular_->home_provider().GetName().empty());
EXPECT_TRUE(cellular_->home_provider().GetCountry().empty());
EXPECT_TRUE(cellular_->home_provider().GetCode().empty());
EXPECT_FALSE(capability_->provider_requires_roaming_);
InitProviderDB();
+
+ // IMSI and Operator Code not available.
+ capability_->SetHomeProvider();
+ EXPECT_TRUE(cellular_->home_provider().GetName().empty());
+ EXPECT_TRUE(cellular_->home_provider().GetCountry().empty());
+ EXPECT_TRUE(cellular_->home_provider().GetCode().empty());
+ EXPECT_FALSE(capability_->provider_requires_roaming_);
+
+ // Operator Code available.
+ capability_->operator_id_ = "310240";
+ capability_->SetHomeProvider();
+ EXPECT_EQ("T-Mobile", cellular_->home_provider().GetName());
+ EXPECT_EQ(kCountry, cellular_->home_provider().GetCountry());
+ EXPECT_EQ("310240", cellular_->home_provider().GetCode());
+ EXPECT_EQ(4, capability_->apn_list_.size());
+ ASSERT_TRUE(capability_->home_provider_);
+ EXPECT_FALSE(capability_->provider_requires_roaming_);
+
+ cellular_->home_provider_.SetName("");
+ cellular_->home_provider_.SetCountry("");
+ cellular_->home_provider_.SetCode("");
+
+ // IMSI available
+ capability_->imsi_ = "310240123456789";
+ capability_->operator_id_.clear();
capability_->SetHomeProvider();
EXPECT_EQ("T-Mobile", cellular_->home_provider().GetName());
EXPECT_EQ(kCountry, cellular_->home_provider().GetCountry());