shill: Decouple Cellular's IsRegistered from the network technology string.
Also, broadcast changes in the cellular service network technology and roaming
state properties.
BUG=chromium-os:23327
TEST=unit tests, tested on device
Change-Id: Ia4c297586dbc9b8a32d297c126c4d791310b5abd
Reviewed-on: https://gerrit.chromium.org/gerrit/12028
Tested-by: Darin Petkov <petkov@chromium.org>
Reviewed-by: Eric Shienbrood <ers@chromium.org>
diff --git a/Makefile b/Makefile
index 25d9689..60d8c9a 100644
--- a/Makefile
+++ b/Makefile
@@ -152,6 +152,7 @@
byte_string_unittest.o \
cellular_capability_cdma_unittest.o \
cellular_capability_gsm_unittest.o \
+ cellular_service_unittest.o \
cellular_unittest.o \
crypto_des_cbc_unittest.o \
crypto_provider_unittest.o \
diff --git a/cellular.cc b/cellular.cc
index 6912ff2..d8cf370 100644
--- a/cellular.cc
+++ b/cellular.cc
@@ -273,8 +273,7 @@
void Cellular::HandleNewRegistrationStateTask() {
VLOG(2) << __func__;
- const string network_tech = capability_->GetNetworkTechnologyString();
- if (network_tech.empty()) {
+ if (!capability_->IsRegistered()) {
if (state_ == kStateLinked) {
manager()->DeregisterService(service_);
}
@@ -298,8 +297,8 @@
SetState(kStateConnected);
EstablishLink();
}
- service_->set_network_tech(network_tech);
- service_->set_roaming_state(capability_->GetRoamingStateString());
+ service_->SetNetworkTechnology(capability_->GetNetworkTechnologyString());
+ service_->SetRoamingState(capability_->GetRoamingStateString());
}
void Cellular::HandleNewSignalQuality(uint32 strength) {
diff --git a/cellular_capability.h b/cellular_capability.h
index 712e1bf..31f954d 100644
--- a/cellular_capability.h
+++ b/cellular_capability.h
@@ -48,6 +48,7 @@
// Network registration.
virtual void Register();
virtual void RegisterOnNetwork(const std::string &network_id, Error *error);
+ virtual bool IsRegistered() = 0;
// Retrieves identifiers associated with the modem and the capability.
virtual void GetIdentifiers() = 0;
diff --git a/cellular_capability_cdma.cc b/cellular_capability_cdma.cc
index 551caaa..3b2e703 100644
--- a/cellular_capability_cdma.cc
+++ b/cellular_capability_cdma.cc
@@ -162,6 +162,11 @@
// No properties.
}
+bool CellularCapabilityCDMA::IsRegistered() {
+ return registration_state_evdo_ != MM_MODEM_CDMA_REGISTRATION_STATE_UNKNOWN ||
+ registration_state_1x_ != MM_MODEM_CDMA_REGISTRATION_STATE_UNKNOWN;
+}
+
string CellularCapabilityCDMA::GetNetworkTechnologyString() const {
if (registration_state_evdo_ != MM_MODEM_CDMA_REGISTRATION_STATE_UNKNOWN) {
return flimflam::kNetworkTechnologyEvdo;
diff --git a/cellular_capability_cdma.h b/cellular_capability_cdma.h
index 9945fb3..01c5fb4 100644
--- a/cellular_capability_cdma.h
+++ b/cellular_capability_cdma.h
@@ -27,6 +27,7 @@
virtual void UpdateStatus(const DBusPropertiesMap &properties);
virtual void SetupConnectProperties(DBusPropertiesMap *properties);
virtual void Activate(const std::string &carrier, Error *error);
+ virtual bool IsRegistered();
virtual std::string GetNetworkTechnologyString() const;
virtual std::string GetRoamingStateString() const;
virtual void GetSignalQuality();
diff --git a/cellular_capability_cdma_unittest.cc b/cellular_capability_cdma_unittest.cc
index 0f33fb9..6b9a091 100644
--- a/cellular_capability_cdma_unittest.cc
+++ b/cellular_capability_cdma_unittest.cc
@@ -177,6 +177,30 @@
EXPECT_EQ(kMEID, cellular_->meid());
}
+TEST_F(CellularCapabilityCDMATest, IsRegisteredEVDO) {
+ EXPECT_FALSE(capability_->IsRegistered());
+ SetRegistrationStateEVDO(MM_MODEM_CDMA_REGISTRATION_STATE_UNKNOWN);
+ EXPECT_FALSE(capability_->IsRegistered());
+ SetRegistrationStateEVDO(MM_MODEM_CDMA_REGISTRATION_STATE_REGISTERED);
+ EXPECT_TRUE(capability_->IsRegistered());
+ SetRegistrationStateEVDO(MM_MODEM_CDMA_REGISTRATION_STATE_HOME);
+ EXPECT_TRUE(capability_->IsRegistered());
+ SetRegistrationStateEVDO(MM_MODEM_CDMA_REGISTRATION_STATE_ROAMING);
+ EXPECT_TRUE(capability_->IsRegistered());
+}
+
+TEST_F(CellularCapabilityCDMATest, IsRegistered1x) {
+ EXPECT_FALSE(capability_->IsRegistered());
+ SetRegistrationState1x(MM_MODEM_CDMA_REGISTRATION_STATE_UNKNOWN);
+ EXPECT_FALSE(capability_->IsRegistered());
+ SetRegistrationState1x(MM_MODEM_CDMA_REGISTRATION_STATE_REGISTERED);
+ EXPECT_TRUE(capability_->IsRegistered());
+ SetRegistrationState1x(MM_MODEM_CDMA_REGISTRATION_STATE_HOME);
+ EXPECT_TRUE(capability_->IsRegistered());
+ SetRegistrationState1x(MM_MODEM_CDMA_REGISTRATION_STATE_ROAMING);
+ EXPECT_TRUE(capability_->IsRegistered());
+}
+
TEST_F(CellularCapabilityCDMATest, GetNetworkTechnologyString) {
EXPECT_EQ("", capability_->GetNetworkTechnologyString());
SetRegistrationStateEVDO(MM_MODEM_CDMA_REGISTRATION_STATE_HOME);
diff --git a/cellular_capability_gsm.cc b/cellular_capability_gsm.cc
index c33cf14..ac8c1a9 100644
--- a/cellular_capability_gsm.cc
+++ b/cellular_capability_gsm.cc
@@ -135,9 +135,13 @@
}
}
if (cellular()->mdn().empty()) {
- // TODO(petkov): Switch to asynchronous calls (crosbug.com/17583).
- cellular()->set_mdn(card_proxy_->GetMSISDN());
- VLOG(2) << "MSISDN/MDN: " << cellular()->mdn();
+ try {
+ // TODO(petkov): Switch to asynchronous calls (crosbug.com/17583).
+ cellular()->set_mdn(card_proxy_->GetMSISDN());
+ VLOG(2) << "MSISDN/MDN: " << cellular()->mdn();
+ } catch (const DBus::Error e) {
+ LOG(WARNING) << "Unable to obtain MSISDN/MDN: " << e.what();
+ }
}
SetHomeProvider();
}
@@ -300,6 +304,11 @@
network_id));
}
+bool CellularCapabilityGSM::IsRegistered() {
+ return (registration_state_ == MM_MODEM_GSM_NETWORK_REG_STATUS_HOME ||
+ registration_state_ == MM_MODEM_GSM_NETWORK_REG_STATUS_ROAMING);
+}
+
void CellularCapabilityGSM::RegisterOnNetworkTask(const string &network_id) {
LOG(INFO) << __func__ << "(" << network_id << ")";
// TODO(petkov): Switch to asynchronous calls (crosbug.com/17583).
@@ -463,32 +472,29 @@
void CellularCapabilityGSM::SetAccessTechnology(uint32 access_technology) {
access_technology_ = access_technology;
if (cellular()->service().get()) {
- cellular()->service()->set_network_tech(GetNetworkTechnologyString());
+ cellular()->service()->SetNetworkTechnology(GetNetworkTechnologyString());
}
}
string CellularCapabilityGSM::GetNetworkTechnologyString() const {
- if (registration_state_ == MM_MODEM_GSM_NETWORK_REG_STATUS_HOME ||
- registration_state_ == MM_MODEM_GSM_NETWORK_REG_STATUS_ROAMING) {
- switch (access_technology_) {
- case MM_MODEM_GSM_ACCESS_TECH_GSM:
- case MM_MODEM_GSM_ACCESS_TECH_GSM_COMPACT:
- return flimflam::kNetworkTechnologyGsm;
- case MM_MODEM_GSM_ACCESS_TECH_GPRS:
- return flimflam::kNetworkTechnologyGprs;
- case MM_MODEM_GSM_ACCESS_TECH_EDGE:
- return flimflam::kNetworkTechnologyEdge;
- case MM_MODEM_GSM_ACCESS_TECH_UMTS:
- return flimflam::kNetworkTechnologyUmts;
- case MM_MODEM_GSM_ACCESS_TECH_HSDPA:
- case MM_MODEM_GSM_ACCESS_TECH_HSUPA:
- case MM_MODEM_GSM_ACCESS_TECH_HSPA:
- return flimflam::kNetworkTechnologyHspa;
- case MM_MODEM_GSM_ACCESS_TECH_HSPA_PLUS:
- return flimflam::kNetworkTechnologyHspaPlus;
- default:
- NOTREACHED();
- }
+ switch (access_technology_) {
+ case MM_MODEM_GSM_ACCESS_TECH_GSM:
+ case MM_MODEM_GSM_ACCESS_TECH_GSM_COMPACT:
+ return flimflam::kNetworkTechnologyGsm;
+ case MM_MODEM_GSM_ACCESS_TECH_GPRS:
+ return flimflam::kNetworkTechnologyGprs;
+ case MM_MODEM_GSM_ACCESS_TECH_EDGE:
+ return flimflam::kNetworkTechnologyEdge;
+ case MM_MODEM_GSM_ACCESS_TECH_UMTS:
+ return flimflam::kNetworkTechnologyUmts;
+ case MM_MODEM_GSM_ACCESS_TECH_HSDPA:
+ case MM_MODEM_GSM_ACCESS_TECH_HSUPA:
+ case MM_MODEM_GSM_ACCESS_TECH_HSPA:
+ return flimflam::kNetworkTechnologyHspa;
+ case MM_MODEM_GSM_ACCESS_TECH_HSPA_PLUS:
+ return flimflam::kNetworkTechnologyHspaPlus;
+ default:
+ break;
}
return "";
}
diff --git a/cellular_capability_gsm.h b/cellular_capability_gsm.h
index 45e9fb4..80a72f4 100644
--- a/cellular_capability_gsm.h
+++ b/cellular_capability_gsm.h
@@ -39,6 +39,7 @@
virtual void GetProperties();
virtual void Register();
virtual void RegisterOnNetwork(const std::string &network_id, Error *error);
+ virtual bool IsRegistered();
virtual void RequirePIN(const std::string &pin, bool require, Error *error);
virtual void EnterPIN(const std::string &pin, Error *error);
virtual void UnblockPIN(const std::string &unblock_code,
diff --git a/cellular_capability_gsm_unittest.cc b/cellular_capability_gsm_unittest.cc
index ac7952a..a8476a0 100644
--- a/cellular_capability_gsm_unittest.cc
+++ b/cellular_capability_gsm_unittest.cc
@@ -158,6 +158,22 @@
EXPECT_EQ(kTestCarrier, capability_->selected_network_);
}
+TEST_F(CellularCapabilityGSMTest, IsRegistered) {
+ EXPECT_FALSE(capability_->IsRegistered());
+ SetRegistrationState(MM_MODEM_GSM_NETWORK_REG_STATUS_IDLE);
+ EXPECT_FALSE(capability_->IsRegistered());
+ SetRegistrationState(MM_MODEM_GSM_NETWORK_REG_STATUS_HOME);
+ EXPECT_TRUE(capability_->IsRegistered());
+ SetRegistrationState(MM_MODEM_GSM_NETWORK_REG_STATUS_SEARCHING);
+ EXPECT_FALSE(capability_->IsRegistered());
+ SetRegistrationState(MM_MODEM_GSM_NETWORK_REG_STATUS_DENIED);
+ EXPECT_FALSE(capability_->IsRegistered());
+ SetRegistrationState(MM_MODEM_GSM_NETWORK_REG_STATUS_UNKNOWN);
+ EXPECT_FALSE(capability_->IsRegistered());
+ SetRegistrationState(MM_MODEM_GSM_NETWORK_REG_STATUS_ROAMING);
+ EXPECT_TRUE(capability_->IsRegistered());
+}
+
TEST_F(CellularCapabilityGSMTest, RequirePIN) {
Error error;
EXPECT_CALL(*card_proxy_, EnablePIN(kPIN, true));
@@ -253,11 +269,11 @@
capability_->SetAccessTechnology(MM_MODEM_GSM_ACCESS_TECH_GSM);
EXPECT_EQ(MM_MODEM_GSM_ACCESS_TECH_GSM, capability_->access_technology_);
SetService();
- capability_->registration_state_ = MM_MODEM_GSM_NETWORK_REG_STATUS_HOME;
+ SetRegistrationState(MM_MODEM_GSM_NETWORK_REG_STATUS_HOME);
capability_->SetAccessTechnology(MM_MODEM_GSM_ACCESS_TECH_GPRS);
EXPECT_EQ(MM_MODEM_GSM_ACCESS_TECH_GPRS, capability_->access_technology_);
EXPECT_EQ(flimflam::kNetworkTechnologyGprs,
- cellular_->service()->network_tech());
+ cellular_->service()->network_technology());
}
TEST_F(CellularCapabilityGSMTest, UpdateOperatorInfo) {
@@ -334,8 +350,6 @@
TEST_F(CellularCapabilityGSMTest, GetNetworkTechnologyString) {
EXPECT_EQ("", capability_->GetNetworkTechnologyString());
SetAccessTechnology(MM_MODEM_GSM_ACCESS_TECH_GSM);
- EXPECT_EQ("", capability_->GetNetworkTechnologyString());
- SetRegistrationState(MM_MODEM_GSM_NETWORK_REG_STATUS_HOME);
EXPECT_EQ(flimflam::kNetworkTechnologyGsm,
capability_->GetNetworkTechnologyString());
SetAccessTechnology(MM_MODEM_GSM_ACCESS_TECH_GSM_COMPACT);
@@ -350,7 +364,6 @@
SetAccessTechnology(MM_MODEM_GSM_ACCESS_TECH_UMTS);
EXPECT_EQ(flimflam::kNetworkTechnologyUmts,
capability_->GetNetworkTechnologyString());
- SetRegistrationState(MM_MODEM_GSM_NETWORK_REG_STATUS_ROAMING);
SetAccessTechnology(MM_MODEM_GSM_ACCESS_TECH_HSDPA);
EXPECT_EQ(flimflam::kNetworkTechnologyHspa,
capability_->GetNetworkTechnologyString());
diff --git a/cellular_service.cc b/cellular_service.cc
index 2a5ff93..d27a360 100644
--- a/cellular_service.cc
+++ b/cellular_service.cc
@@ -10,6 +10,7 @@
#include <base/stringprintf.h>
#include <chromeos/dbus/service_constants.h>
+#include "shill/adaptor_interfaces.h"
#include "shill/cellular.h"
using std::string;
@@ -33,7 +34,7 @@
store->RegisterConstStringmap(flimflam::kCellularLastGoodApnProperty,
&last_good_apn_info_);
store->RegisterConstString(flimflam::kNetworkTechnologyProperty,
- &network_tech_);
+ &network_technology_);
store->RegisterConstString(flimflam::kPaymentURLProperty, &payment_url_);
store->RegisterConstString(flimflam::kRoamingStateProperty, &roaming_state_);
store->RegisterConstStringmap(flimflam::kServingOperatorProperty,
@@ -72,6 +73,23 @@
return cellular_->GetRpcIdentifier();
}
+void CellularService::SetNetworkTechnology(const string &technology) {
+ if (technology == network_technology_) {
+ return;
+ }
+ network_technology_ = technology;
+ adaptor()->EmitStringChanged(flimflam::kNetworkTechnologyProperty,
+ technology);
+}
+
+void CellularService::SetRoamingState(const string &state) {
+ if (state == roaming_state_) {
+ return;
+ }
+ roaming_state_ = state;
+ adaptor()->EmitStringChanged(flimflam::kRoamingStateProperty, state);
+}
+
const Cellular::Operator &CellularService::serving_operator() const {
return serving_operator_;
}
diff --git a/cellular_service.h b/cellular_service.h
index d304999..cd9694c 100644
--- a/cellular_service.h
+++ b/cellular_service.h
@@ -9,6 +9,7 @@
#include <string>
#include <base/basictypes.h>
+#include <gtest/gtest_prod.h> // for FRIEND_TEST
#include "shill/cellular.h"
#include "shill/refptr_types.h"
@@ -55,13 +56,18 @@
const Cellular::Operator &serving_operator() const;
void set_serving_operator(const Cellular::Operator &oper);
- const std::string &network_tech() const { return network_tech_; }
- void set_network_tech(const std::string &tech) { network_tech_ = tech; }
+ // Sets network technology to |technology| and broadcasts the property change.
+ void SetNetworkTechnology(const std::string &technology);
+ const std::string &network_technology() const { return network_technology_; }
+ // Sets roaming state to |state| and broadcasts the property change.
+ void SetRoamingState(const std::string &state);
const std::string &roaming_state() const { return roaming_state_; }
- void set_roaming_state(const std::string &state) { roaming_state_ = state; }
private:
+ friend class CellularServiceTest;
+ FRIEND_TEST(CellularTest, Connect);
+
static const char kServiceType[];
virtual std::string GetDeviceRpcId(Error *error);
@@ -69,7 +75,7 @@
// Properties
std::string activation_state_;
Cellular::Operator serving_operator_;
- std::string network_tech_;
+ std::string network_technology_;
std::string roaming_state_;
std::string payment_url_;
uint8 strength_;
diff --git a/cellular_service_unittest.cc b/cellular_service_unittest.cc
new file mode 100644
index 0000000..55d61d9
--- /dev/null
+++ b/cellular_service_unittest.cc
@@ -0,0 +1,56 @@
+// Copyright (c) 2011 The Chromium OS Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "shill/cellular_service.h"
+
+#include <chromeos/dbus/service_constants.h>
+#include <gtest/gtest.h>
+
+#include "shill/nice_mock_control.h"
+#include "shill/mock_adaptors.h"
+
+using testing::NiceMock;
+
+namespace shill {
+
+class CellularServiceTest : public testing::Test {
+ public:
+ CellularServiceTest()
+ : service_(new CellularService(&control_, NULL, NULL, NULL)),
+ adaptor_(NULL) {}
+
+ virtual ~CellularServiceTest() {
+ adaptor_ = NULL;
+ }
+
+ virtual void SetUp() {
+ adaptor_ =
+ dynamic_cast<NiceMock<ServiceMockAdaptor> *>(service_->adaptor());
+ }
+
+ protected:
+ NiceMockControl control_;
+ CellularServiceRefPtr service_;
+ NiceMock<ServiceMockAdaptor> *adaptor_; // Owned by |service_|.
+};
+
+TEST_F(CellularServiceTest, SetNetworkTechnology) {
+ EXPECT_CALL(*adaptor_, EmitStringChanged(flimflam::kNetworkTechnologyProperty,
+ flimflam::kNetworkTechnologyUmts));
+ EXPECT_TRUE(service_->network_technology().empty());
+ service_->SetNetworkTechnology(flimflam::kNetworkTechnologyUmts);
+ EXPECT_EQ(flimflam::kNetworkTechnologyUmts, service_->network_technology());
+ service_->SetNetworkTechnology(flimflam::kNetworkTechnologyUmts);
+}
+
+TEST_F(CellularServiceTest, SetRoamingState) {
+ EXPECT_CALL(*adaptor_, EmitStringChanged(flimflam::kRoamingStateProperty,
+ flimflam::kRoamingStateHome));
+ EXPECT_TRUE(service_->roaming_state().empty());
+ service_->SetRoamingState(flimflam::kRoamingStateHome);
+ EXPECT_EQ(flimflam::kRoamingStateHome, service_->roaming_state());
+ service_->SetRoamingState(flimflam::kRoamingStateHome);
+}
+
+} // namespace shill
diff --git a/cellular_unittest.cc b/cellular_unittest.cc
index ebf226f..608730a 100644
--- a/cellular_unittest.cc
+++ b/cellular_unittest.cc
@@ -277,7 +277,7 @@
EXPECT_EQ(Cellular::kStateRegistered, device_->state_);
ASSERT_TRUE(device_->service_.get());
EXPECT_EQ(flimflam::kNetworkTechnology1Xrtt,
- device_->service_->network_tech());
+ device_->service_->network_technology());
EXPECT_EQ(kStrength, device_->service_->strength());
EXPECT_EQ(flimflam::kRoamingStateHome, device_->service_->roaming_state());
}
@@ -315,7 +315,7 @@
EXPECT_EQ(Cellular::kStateRegistered, device_->state_);
ASSERT_TRUE(device_->service_.get());
EXPECT_EQ(flimflam::kNetworkTechnologyEdge,
- device_->service_->network_tech());
+ device_->service_->network_technology());
EXPECT_EQ(kStrength, device_->service_->strength());
EXPECT_EQ(flimflam::kRoamingStateRoaming, device_->service_->roaming_state());
EXPECT_EQ(kNetworkID, device_->service_->serving_operator().GetCode());
@@ -446,19 +446,19 @@
&control_interface_, &dispatcher_, &manager_, device_);
device_->allow_roaming_ = false;
- device_->service_->set_roaming_state(flimflam::kRoamingStateRoaming);
+ device_->service_->roaming_state_ = flimflam::kRoamingStateRoaming;
device_->Connect(&error);
ASSERT_TRUE(device_->task_factory_.empty());
EXPECT_EQ(Error::kNotOnHomeNetwork, error.type());
error.Populate(Error::kSuccess);
- device_->service_->set_roaming_state(flimflam::kRoamingStateHome);
+ device_->service_->roaming_state_ = flimflam::kRoamingStateHome;
device_->Connect(&error);
ASSERT_FALSE(device_->task_factory_.empty());
EXPECT_TRUE(error.IsSuccess());
device_->allow_roaming_ = true;
- device_->service_->set_roaming_state(flimflam::kRoamingStateRoaming);
+ device_->service_->roaming_state_ = flimflam::kRoamingStateRoaming;
device_->Connect(&error);
EXPECT_TRUE(error.IsSuccess());
diff --git a/service.h b/service.h
index b0dde90..49719b6 100644
--- a/service.h
+++ b/service.h
@@ -197,6 +197,8 @@
std::string(Service::*get)(Error *),
void(Service::*set)(const std::string&, Error *));
+ ServiceAdaptorInterface *adaptor() const { return adaptor_.get(); }
+
// Assigns |value| to |key| in |storage| if |value| is non-empty and |save| is
// true. Otherwise, removes |key| from |storage|. If |crypted| is true, the
// value is encrypted.