shill: cellular: Persist the Allow.Roaming property of cellular devices
Persist the Allow.Roaming property of cellular devices
BUG=none
TEST=unit tests and manually testing on the machine. restart shill.
Change-Id: I25c25588ca20e9fb941b0024d558b229e5fcfa90
Reviewed-on: https://gerrit.chromium.org/gerrit/21540
Reviewed-by: Jason Glasgow <jglasgow@chromium.org>
Tested-by: Jason Glasgow <jglasgow@chromium.org>
Commit-Ready: Jason Glasgow <jglasgow@chromium.org>
diff --git a/cellular.cc b/cellular.cc
index 1cac8f7..db6df6e 100644
--- a/cellular.cc
+++ b/cellular.cc
@@ -33,6 +33,7 @@
#include "shill/proxy_factory.h"
#include "shill/rtnl_handler.h"
#include "shill/scope_logger.h"
+#include "shill/store_interface.h"
#include "shill/technology.h"
using base::Bind;
@@ -41,6 +42,9 @@
namespace shill {
+// static
+const char Cellular::kAllowRoaming[] = "AllowRoaming";
+
Cellular::Operator::Operator() {
SetName("");
SetCode("");
@@ -104,13 +108,17 @@
modem_state_(kModemStateUnknown),
dbus_owner_(owner),
dbus_path_(path),
- provider_db_(provider_db) {
+ provider_db_(provider_db),
+ allow_roaming_(false) {
PropertyStore *store = this->mutable_store();
store->RegisterConstString(flimflam::kDBusConnectionProperty, &dbus_owner_);
store->RegisterConstString(flimflam::kDBusObjectProperty, &dbus_path_);
HelpRegisterDerivedString(flimflam::kTechnologyFamilyProperty,
&Cellular::GetTechnologyFamily,
NULL);
+ HelpRegisterDerivedBool(flimflam::kCellularAllowRoamingProperty,
+ &Cellular::GetAllowRoaming,
+ &Cellular::SetAllowRoaming);
store->RegisterConstStringmap(flimflam::kHomeProviderProperty,
&home_provider_.ToDict());
// For now, only a single capability is supported.
@@ -123,6 +131,22 @@
Cellular::~Cellular() {
}
+bool Cellular::Load(StoreInterface *storage) {
+ const string id = GetStorageIdentifier();
+ if (!storage->ContainsGroup(id)) {
+ LOG(WARNING) << "Device is not available in the persistent store: " << id;
+ return false;
+ }
+ storage->GetBool(id, kAllowRoaming, &allow_roaming_);
+ return Device::Load(storage);
+}
+
+bool Cellular::Save(StoreInterface *storage) {
+ const string id = GetStorageIdentifier();
+ storage->SetBool(id, kAllowRoaming, allow_roaming_);
+ return Device::Save(storage);
+}
+
// static
string Cellular::GetStateString(State state) {
switch (state) {
@@ -146,6 +170,16 @@
state_ = state;
}
+void Cellular::HelpRegisterDerivedBool(
+ const string &name,
+ bool(Cellular::*get)(Error *error),
+ void(Cellular::*set)(const bool &value, Error *error)) {
+ mutable_store()->RegisterDerivedBool(
+ name,
+ BoolAccessor(
+ new CustomAccessor<Cellular, bool>(this, get, set)));
+}
+
void Cellular::HelpRegisterDerivedString(
const string &name,
string(Cellular::*get)(Error *),
@@ -522,4 +556,24 @@
}
}
+void Cellular::SetAllowRoaming(const bool &value, Error */*error*/) {
+ SLOG(Cellular, 2) << __func__
+ << "(" << allow_roaming_ << "->" << value << ")";
+ if (allow_roaming_ == value) {
+ return;
+ }
+ allow_roaming_ = value;
+ manager()->SaveActiveProfile();
+
+ // Use AllowRoaming() instead of allow_roaming_ in order to
+ // incorporate provider preferences when evaluating if a disconnect
+ // is required.
+ if (!capability_->AllowRoaming() &&
+ capability_->GetRoamingStateString() == flimflam::kRoamingStateRoaming) {
+ Error error;
+ Disconnect(&error);
+ }
+ adaptor()->EmitBoolChanged(flimflam::kCellularAllowRoamingProperty, value);
+}
+
} // namespace shill
diff --git a/cellular.h b/cellular.h
index ee9683e..5f4f424 100644
--- a/cellular.h
+++ b/cellular.h
@@ -107,6 +107,12 @@
mobile_provider_db *provider_db);
virtual ~Cellular();
+ // Load configuration for the device from |storage|.
+ virtual bool Load(StoreInterface *storage);
+
+ // Save configuration for the device to |storage|.
+ virtual bool Save(StoreInterface *storage);
+
// Asynchronously connects the modem to the network. Populates |error| on
// failure, leaves it unchanged otherwise.
void Connect(Error *error);
@@ -185,6 +191,9 @@
ModemState new_state,
uint32 reason);
+ // accessor to read the allow roaming property
+ bool allow_roaming_property() const { return allow_roaming_; }
+
private:
friend class CellularTest;
friend class CellularCapabilityTest;
@@ -215,6 +224,9 @@
FRIEND_TEST(CellularCapabilityTest, GetModemInfo);
FRIEND_TEST(CellularCapabilityTest, GetModemStatus);
+ // Names of properties in storage
+ static const char kAllowRoaming[];
+
void SetState(State state);
// Invoked when the modem is connected to the cellular network to transition
@@ -231,6 +243,10 @@
// Reads of the property will be handled by invoking |get|.
// Writes to the property will be handled by invoking |set|.
// Clearing the property will be handled by PropertyStore.
+ void HelpRegisterDerivedBool(
+ const std::string &name,
+ bool(Cellular::*get)(Error *error),
+ void(Cellular::*set)(const bool &value, Error *error));
void HelpRegisterDerivedString(
const std::string &name,
std::string(Cellular::*get)(Error *error),
@@ -239,6 +255,10 @@
void OnConnectReply(const Error &error);
void OnDisconnectReply(const Error &error);
+ // DBUS accessors to read/modify the allow roaming property
+ bool GetAllowRoaming(Error */*error*/) { return allow_roaming_; }
+ void SetAllowRoaming(const bool &value, Error *error);
+
State state_;
ModemState modem_state_;
@@ -254,6 +274,9 @@
// Properties
Operator home_provider_;
+ // User preference to allow or disallow roaming
+ bool allow_roaming_;
+
DISALLOW_COPY_AND_ASSIGN(Cellular);
};
diff --git a/cellular_capability.cc b/cellular_capability.cc
index 22eb749..f278793 100644
--- a/cellular_capability.cc
+++ b/cellular_capability.cc
@@ -30,44 +30,11 @@
CellularCapability::CellularCapability(Cellular *cellular,
ProxyFactory *proxy_factory)
: cellular_(cellular),
- proxy_factory_(proxy_factory),
- allow_roaming_(false) {
- HelpRegisterDerivedBool(flimflam::kCellularAllowRoamingProperty,
- &CellularCapability::GetAllowRoaming,
- &CellularCapability::SetAllowRoaming);
+ proxy_factory_(proxy_factory) {
}
CellularCapability::~CellularCapability() {}
-void CellularCapability::HelpRegisterDerivedBool(
- const string &name,
- bool(CellularCapability::*get)(Error *error),
- void(CellularCapability::*set)(const bool &value, Error *error)) {
- cellular()->mutable_store()->RegisterDerivedBool(
- name,
- BoolAccessor(
- new CustomAccessor<CellularCapability, bool>(this, get, set)));
-}
-
-void CellularCapability::SetAllowRoaming(const bool &value, Error */*error*/) {
- SLOG(Cellular, 2) << __func__
- << "(" << allow_roaming_ << "->" << value << ")";
- if (allow_roaming_ == value) {
- return;
- }
- allow_roaming_ = value;
- // Use AllowRoaming() instead of allow_roaming_ in order to
- // incorporate provider preferences when evaluating if a disconnect
- // is required.
- if (!AllowRoaming() &&
- GetRoamingStateString() == flimflam::kRoamingStateRoaming) {
- Error error;
- cellular()->Disconnect(&error);
- }
- cellular()->adaptor()->EmitBoolChanged(
- flimflam::kCellularAllowRoamingProperty, value);
-}
-
void CellularCapability::RunNextStep(CellularTaskList *tasks) {
CHECK(!tasks->empty());
SLOG(Cellular, 2) << __func__ << ": " << tasks->size() << " remaining tasks";
diff --git a/cellular_capability.h b/cellular_capability.h
index 69a8f1e..e7eca19 100644
--- a/cellular_capability.h
+++ b/cellular_capability.h
@@ -198,7 +198,9 @@
CellularTaskList *tasks, const Error &error);
// accessor for subclasses to read the allow roaming property
- bool allow_roaming_property() const { return allow_roaming_; }
+ bool allow_roaming_property() const {
+ return cellular_->allow_roaming_property();
+ }
private:
friend class CellularCapabilityGSMTest;
@@ -209,23 +211,11 @@
FRIEND_TEST(CellularTest, Connect);
FRIEND_TEST(CellularTest, TearDown);
- void HelpRegisterDerivedBool(
- const std::string &name,
- bool(CellularCapability::*get)(Error *error),
- void(CellularCapability::*set)(const bool &value, Error *error));
-
- // DBUS accessors to read/modify the allow roaming property
- bool GetAllowRoaming(Error */*error*/) { return allow_roaming_; }
- void SetAllowRoaming(const bool &value, Error *error);
-
Cellular *cellular_;
// Store cached copies of singletons for speed/ease of testing.
ProxyFactory *proxy_factory_;
- // User preference to allow or disallow roaming
- bool allow_roaming_;
-
DISALLOW_COPY_AND_ASSIGN(CellularCapability);
};
diff --git a/cellular_capability_classic_unittest.cc b/cellular_capability_classic_unittest.cc
index 2b1f1e9..ec83452 100644
--- a/cellular_capability_classic_unittest.cc
+++ b/cellular_capability_classic_unittest.cc
@@ -299,9 +299,9 @@
}
TEST_F(CellularCapabilityTest, AllowRoaming) {
- EXPECT_FALSE(capability_->GetAllowRoaming(NULL));
- capability_->SetAllowRoaming(false, NULL);
- EXPECT_FALSE(capability_->GetAllowRoaming(NULL));
+ EXPECT_FALSE(cellular_->GetAllowRoaming(NULL));
+ cellular_->SetAllowRoaming(false, NULL);
+ EXPECT_FALSE(cellular_->GetAllowRoaming(NULL));
{
InSequence seq;
@@ -314,16 +314,16 @@
cellular_->state_ = Cellular::kStateConnected;
dynamic_cast<CellularCapabilityGSM *>(capability_)->registration_state_ =
MM_MODEM_GSM_NETWORK_REG_STATUS_ROAMING;
- capability_->SetAllowRoaming(true, NULL);
- EXPECT_TRUE(capability_->GetAllowRoaming(NULL));
+ cellular_->SetAllowRoaming(true, NULL);
+ EXPECT_TRUE(cellular_->GetAllowRoaming(NULL));
EXPECT_EQ(Cellular::kStateConnected, cellular_->state_);
EXPECT_CALL(*proxy_, Disconnect(_, _, CellularCapability::kTimeoutDefault))
.WillOnce(Invoke(this, &CellularCapabilityTest::InvokeDisconnect));
SetProxy();
cellular_->state_ = Cellular::kStateConnected;
- capability_->SetAllowRoaming(false, NULL);
- EXPECT_FALSE(capability_->GetAllowRoaming(NULL));
+ cellular_->SetAllowRoaming(false, NULL);
+ EXPECT_FALSE(cellular_->GetAllowRoaming(NULL));
EXPECT_EQ(Cellular::kStateRegistered, cellular_->state_);
}
diff --git a/cellular_unittest.cc b/cellular_unittest.cc
index 336f175..c2738fa 100644
--- a/cellular_unittest.cc
+++ b/cellular_unittest.cc
@@ -542,7 +542,7 @@
device_->service_ = new CellularService(
&control_interface_, &dispatcher_, &metrics_, &manager_, device_);
- device_->capability_->allow_roaming_ = false;
+ device_->allow_roaming_ = false;
device_->service_->roaming_state_ = flimflam::kRoamingStateRoaming;
device_->Connect(&error);
EXPECT_EQ(Error::kNotOnHomeNetwork, error.type());
@@ -561,7 +561,7 @@
dispatcher_.DispatchPendingEvents();
EXPECT_EQ(Cellular::kStateConnected, device_->state_);
- device_->capability_->allow_roaming_ = true;
+ device_->allow_roaming_ = true;
device_->service_->roaming_state_ = flimflam::kRoamingStateRoaming;
device_->state_ = Cellular::kStateRegistered;
device_->Connect(&error);