shill: Remove Cellular's obsolete |type_| data member.
Also, some cleanup and renaming.
BUG=chromium-os:18735
TEST=unit tests
Change-Id: Ia201113a9514c884a69209a2fd06a01068e8bd14
Reviewed-on: https://gerrit.chromium.org/gerrit/11901
Commit-Ready: Darin Petkov <petkov@chromium.org>
Reviewed-by: Darin Petkov <petkov@chromium.org>
Tested-by: Darin Petkov <petkov@chromium.org>
diff --git a/cellular.cc b/cellular.cc
index 569ee7a..c92355b 100644
--- a/cellular.cc
+++ b/cellular.cc
@@ -97,7 +97,6 @@
address,
interface_index),
proxy_factory_(ProxyFactory::GetInstance()),
- type_(type),
state_(kStateDisabled),
modem_state_(kModemStateUnknown),
dbus_owner_(owner),
@@ -125,23 +124,13 @@
store->RegisterConstString(flimflam::kMinProperty, &min_);
store->RegisterConstString(flimflam::kModelIDProperty, &model_id_);
- InitCapability(); // For now, only a single capability is supported.
+ InitCapability(type); // For now, only a single capability is supported.
- VLOG(2) << "Cellular device " << this->link_name() << " initialized: "
- << GetTypeString();
+ VLOG(2) << "Cellular device " << this->link_name() << " initialized.";
}
Cellular::~Cellular() {}
-string Cellular::GetTypeString() const {
- switch (type_) {
- case kTypeGSM: return "CellularTypeGSM";
- case kTypeCDMA: return "CellularTypeCDMA";
- default: NOTREACHED();
- }
- return StringPrintf("CellularTypeUnknown-%d", type_);
-}
-
// static
string Cellular::GetStateString(State state) {
switch (state) {
@@ -163,7 +152,7 @@
void Cellular::Start() {
LOG(INFO) << __func__ << ": " << GetStateString(state_);
Device::Start();
- capability_->OnStart();
+ capability_->OnDeviceStarted();
InitProxies();
EnableModem();
capability_->Register();
@@ -175,7 +164,7 @@
}
void Cellular::Stop() {
- capability_->OnStop();
+ capability_->OnDeviceStopped();
proxy_.reset();
simple_proxy_.reset();
manager()->DeregisterService(service_);
@@ -184,9 +173,11 @@
Device::Stop();
}
-void Cellular::InitCapability() {
- VLOG(2) << __func__;
- switch (type_) {
+void Cellular::InitCapability(Type type) {
+ // TODO(petkov): Consider moving capability construction into a factory that's
+ // external to the Cellular class.
+ VLOG(2) << __func__ << "(" << type << ")";
+ switch (type) {
case kTypeGSM:
capability_.reset(new CellularCapabilityGSM(this));
break;
@@ -343,7 +334,6 @@
service_->roaming_state() == flimflam::kRoamingStateRoaming) {
Error::PopulateAndLog(error, Error::kNotOnHomeNetwork,
"Roaming disallowed; connection request ignored.");
- CHECK(error);
return;
}
diff --git a/cellular.h b/cellular.h
index 978904c..154f64c 100644
--- a/cellular.h
+++ b/cellular.h
@@ -181,7 +181,6 @@
FRIEND_TEST(CellularTest, Connect);
FRIEND_TEST(CellularTest, GetModemInfo);
FRIEND_TEST(CellularTest, GetModemStatus);
- FRIEND_TEST(CellularTest, GetTypeString);
FRIEND_TEST(CellularTest, InitProxies);
FRIEND_TEST(CellularTest, StartConnected);
FRIEND_TEST(CellularTest, StartCDMARegister);
@@ -196,11 +195,9 @@
// to the network-connected state and bring the network interface up.
void EstablishLink();
- void InitCapability();
+ void InitCapability(Type type);
void InitProxies();
- std::string GetTypeString() const;
-
void EnableModem();
void GetModemStatus();
@@ -219,7 +216,6 @@
// Store cached copies of singletons for speed/ease of testing.
ProxyFactory *proxy_factory_;
- Type type_;
State state_;
ModemState modem_state_;
diff --git a/cellular_capability.h b/cellular_capability.h
index 3638c27..712e1bf 100644
--- a/cellular_capability.h
+++ b/cellular_capability.h
@@ -31,8 +31,11 @@
EventDispatcher *dispatcher() const;
// Invoked on starting and stopping the cellular device.
- virtual void OnStart() = 0;
- virtual void OnStop() = 0;
+ virtual void OnDeviceStarted() = 0;
+ virtual void OnDeviceStopped() = 0;
+
+ // Invoked by the parent Cellular device when a new service is created.
+ virtual void OnServiceCreated() = 0;
virtual void UpdateStatus(const DBusPropertiesMap &properties) = 0;
@@ -77,9 +80,6 @@
virtual void OnModemManagerPropertiesChanged(
const DBusPropertiesMap &properties) = 0;
- // Invoked by the parent Cellular device when a new service is created.
- virtual void OnServiceCreated() = 0;
-
private:
friend class CellularTest;
diff --git a/cellular_capability_cdma.cc b/cellular_capability_cdma.cc
index 27b7ea3..551caaa 100644
--- a/cellular_capability_cdma.cc
+++ b/cellular_capability_cdma.cc
@@ -25,21 +25,30 @@
registration_state_evdo_(MM_MODEM_CDMA_REGISTRATION_STATE_UNKNOWN),
registration_state_1x_(MM_MODEM_CDMA_REGISTRATION_STATE_UNKNOWN),
prl_version_(0) {
+ VLOG(2) << "Cellular capability constructed: CDMA";
PropertyStore *store = cellular->mutable_store();
store->RegisterConstUint16(flimflam::kPRLVersionProperty, &prl_version_);
}
-void CellularCapabilityCDMA::OnStart() {
+void CellularCapabilityCDMA::OnDeviceStarted() {
VLOG(2) << __func__;
proxy_.reset(proxy_factory()->CreateModemCDMAProxy(
this, cellular()->dbus_path(), cellular()->dbus_owner()));
}
-void CellularCapabilityCDMA::OnStop() {
+void CellularCapabilityCDMA::OnDeviceStopped() {
VLOG(2) << __func__;
proxy_.reset();
}
+void CellularCapabilityCDMA::OnServiceCreated() {
+ VLOG(2) << __func__;
+ cellular()->service()->set_payment_url(payment_url_);
+ cellular()->service()->set_usage_url(usage_url_);
+ UpdateServingOperator();
+ HandleNewActivationState(MM_MODEM_CDMA_ACTIVATION_ERROR_NO_ERROR);
+}
+
void CellularCapabilityCDMA::UpdateStatus(const DBusPropertiesMap &properties) {
string carrier;
if (DBusProperties::GetString(properties, "carrier", &carrier)) {
@@ -51,9 +60,9 @@
DBusProperties::GetUint32(
properties, "activation_state", &activation_state_);
DBusProperties::GetUint16(properties, "prl_version", &prl_version_);
- // TODO(petkov): For now, get the payment and usage URLs from ModemManager
- // to match flimflam. In the future, provide a plugin API to get these
- // directly from the modem driver.
+ // TODO(petkov): For now, get the payment and usage URLs from ModemManager to
+ // match flimflam. In the future, get these from an alternative source (e.g.,
+ // database, carrier-specific properties, etc.).
DBusProperties::GetString(properties, "payment_url", &payment_url_);
DBusProperties::GetString(properties, "usage_url", &usage_url_);
}
@@ -144,7 +153,7 @@
if (cellular()->meid().empty()) {
// TODO(petkov): Switch to asynchronous calls (crosbug.com/17583).
cellular()->set_meid(proxy_->MEID());
- VLOG(2) << "MEID: " << cellular()->imei();
+ VLOG(2) << "MEID: " << cellular()->meid();
}
}
@@ -206,14 +215,6 @@
}
}
-void CellularCapabilityCDMA::OnServiceCreated() {
- VLOG(2) << __func__;
- cellular()->service()->set_payment_url(payment_url_);
- cellular()->service()->set_usage_url(usage_url_);
- UpdateServingOperator();
- HandleNewActivationState(MM_MODEM_CDMA_ACTIVATION_ERROR_NO_ERROR);
-}
-
void CellularCapabilityCDMA::OnCDMAActivationStateChanged(
uint32 activation_state,
uint32 activation_error,
diff --git a/cellular_capability_cdma.h b/cellular_capability_cdma.h
index b354c71..9945fb3 100644
--- a/cellular_capability_cdma.h
+++ b/cellular_capability_cdma.h
@@ -21,8 +21,9 @@
CellularCapabilityCDMA(Cellular *cellular);
// Inherited from CellularCapability.
- virtual void OnStart();
- virtual void OnStop();
+ virtual void OnDeviceStarted();
+ virtual void OnDeviceStopped();
+ virtual void OnServiceCreated();
virtual void UpdateStatus(const DBusPropertiesMap &properties);
virtual void SetupConnectProperties(DBusPropertiesMap *properties);
virtual void Activate(const std::string &carrier, Error *error);
@@ -33,7 +34,6 @@
virtual void GetProperties();
virtual void OnModemManagerPropertiesChanged(
const DBusPropertiesMap &properties);
- virtual void OnServiceCreated();
// Obtains the MEID.
virtual void GetIdentifiers();
diff --git a/cellular_capability_gsm.cc b/cellular_capability_gsm.cc
index 7296bf9..7e5b9ef 100644
--- a/cellular_capability_gsm.cc
+++ b/cellular_capability_gsm.cc
@@ -41,6 +41,7 @@
access_technology_(MM_MODEM_GSM_ACCESS_TECH_UNKNOWN),
scanning_(false),
scan_interval_(0) {
+ VLOG(2) << "Cellular capability constructed: GSM";
PropertyStore *store = cellular->mutable_store();
store->RegisterConstString(flimflam::kSelectedNetworkProperty,
&selected_network_);
@@ -71,7 +72,7 @@
this, get, set)));
}
-void CellularCapabilityGSM::OnStart() {
+void CellularCapabilityGSM::OnDeviceStarted() {
VLOG(2) << __func__;
card_proxy_.reset(
proxy_factory()->CreateModemGSMCardProxy(this,
@@ -83,12 +84,18 @@
cellular()->dbus_owner()));
}
-void CellularCapabilityGSM::OnStop() {
+void CellularCapabilityGSM::OnDeviceStopped() {
VLOG(2) << __func__;
card_proxy_.reset();
network_proxy_.reset();
}
+void CellularCapabilityGSM::OnServiceCreated() {
+ cellular()->service()->set_activation_state(
+ flimflam::kActivationStateActivated);
+ UpdateServingOperator();
+}
+
void CellularCapabilityGSM::UpdateStatus(const DBusPropertiesMap &properties) {
string imsi;
if (DBusProperties::GetString(properties, "imsi", &imsi)) {
@@ -432,12 +439,6 @@
properties, kPropertyUnlockRetries, &sim_lock_status_.retries_left);
}
-void CellularCapabilityGSM::OnServiceCreated() {
- cellular()->service()->set_activation_state(
- flimflam::kActivationStateActivated);
- UpdateServingOperator();
-}
-
void CellularCapabilityGSM::OnGSMNetworkModeChanged(uint32 /*mode*/) {
// TODO(petkov): Implement this.
NOTIMPLEMENTED();
diff --git a/cellular_capability_gsm.h b/cellular_capability_gsm.h
index 6e5796b..dbfe8ca 100644
--- a/cellular_capability_gsm.h
+++ b/cellular_capability_gsm.h
@@ -26,8 +26,9 @@
CellularCapabilityGSM(Cellular *cellular);
// Inherited from CellularCapability.
- virtual void OnStart();
- virtual void OnStop();
+ virtual void OnDeviceStarted();
+ virtual void OnDeviceStopped();
+ virtual void OnServiceCreated();
virtual void UpdateStatus(const DBusPropertiesMap &properties);
virtual void SetupConnectProperties(DBusPropertiesMap *properties);
virtual void GetSignalQuality();
@@ -48,7 +49,6 @@
virtual std::string GetRoamingStateString() const;
virtual void OnModemManagerPropertiesChanged(
const DBusPropertiesMap &properties);
- virtual void OnServiceCreated();
// Obtains the IMEI, IMSI, SPN and MSISDN.
virtual void GetIdentifiers();
diff --git a/cellular_unittest.cc b/cellular_unittest.cc
index 8cb4c13..ebf226f 100644
--- a/cellular_unittest.cc
+++ b/cellular_unittest.cc
@@ -201,8 +201,7 @@
void StopRTNLHandler();
void SetCellularType(Cellular::Type type) {
- device_->type_ = type;
- device_->InitCapability();
+ device_->InitCapability(type);
}
CellularCapabilityCDMA *GetCapabilityCDMA() {
@@ -246,12 +245,6 @@
const char CellularTest::kTestMobileProviderDBPath[] =
"provider_db_unittest.bfd";
-TEST_F(CellularTest, GetTypeString) {
- EXPECT_EQ("CellularTypeGSM", device_->GetTypeString());
- SetCellularType(Cellular::kTypeCDMA);
- EXPECT_EQ("CellularTypeCDMA", device_->GetTypeString());
-}
-
TEST_F(CellularTest, GetStateString) {
EXPECT_EQ("CellularStateDisabled",
device_->GetStateString(Cellular::kStateDisabled));