shill: Propagate Cellular errors to the DBus caller.
BUG=chromium-os:19547
TEST=unit tests
Change-Id: Ieda54d89d977a48718302c6e1e1c7927e3cf2dba
Reviewed-on: http://gerrit.chromium.org/gerrit/6603
Reviewed-by: Darin Petkov <petkov@chromium.org>
Tested-by: Darin Petkov <petkov@chromium.org>
diff --git a/cellular.cc b/cellular.cc
index 44e25e0..20cf129 100644
--- a/cellular.cc
+++ b/cellular.cc
@@ -20,6 +20,7 @@
#include "shill/control_interface.h"
#include "shill/device.h"
#include "shill/device_info.h"
+#include "shill/error.h"
#include "shill/manager.h"
#include "shill/modem_simple_proxy_interface.h"
#include "shill/profile.h"
@@ -556,10 +557,13 @@
return type == Device::kCellular;
}
-void Cellular::Connect() {
+void Cellular::Connect(Error *error) {
VLOG(2) << __func__;
if (state_ == kStateConnected ||
state_ == kStateLinked) {
+ LOG(ERROR) << "Already connected; connection request ignored.";
+ CHECK(error);
+ error->Populate(Error::kAlreadyConnected);
return;
}
CHECK_EQ(kStateRegistered, state_);
@@ -567,6 +571,8 @@
if (!allow_roaming_ &&
service_->roaming_state() == flimflam::kRoamingStateRoaming) {
LOG(ERROR) << "Roaming disallowed; connection request ignored.";
+ CHECK(error);
+ error->Populate(Error::kNotOnHomeNetwork);
return;
}
diff --git a/cellular.h b/cellular.h
index 0e7db86..ee226e2 100644
--- a/cellular.h
+++ b/cellular.h
@@ -20,6 +20,7 @@
namespace shill {
+class Error;
class ModemSimpleProxyInterface;
class Cellular : public Device,
@@ -150,8 +151,9 @@
const std::string &path);
virtual ~Cellular();
- // Asynchronously connects the modem to the network.
- void Connect();
+ // Asynchronously connects the modem to the network. Populates |error| on
+ // failure, leaves it unchanged otherwise.
+ void Connect(Error *error);
// Asynchronously activates the modem.
void Activate(const std::string &carrier);
diff --git a/cellular_service.cc b/cellular_service.cc
index 487df55..471ad4c 100644
--- a/cellular_service.cc
+++ b/cellular_service.cc
@@ -45,8 +45,8 @@
CellularService::~CellularService() { }
-void CellularService::Connect() {
- cellular_->Connect();
+void CellularService::Connect(Error *error) {
+ cellular_->Connect(error);
}
void CellularService::Disconnect() { }
diff --git a/cellular_service.h b/cellular_service.h
index 7224774..895a146 100644
--- a/cellular_service.h
+++ b/cellular_service.h
@@ -29,7 +29,7 @@
virtual ~CellularService();
// Inherited from Service.
- virtual void Connect();
+ virtual void Connect(Error *error);
virtual void Disconnect();
virtual void ActivateCellularModem(const std::string &carrier);
diff --git a/cellular_unittest.cc b/cellular_unittest.cc
index f227723..ac38b3a 100644
--- a/cellular_unittest.cc
+++ b/cellular_unittest.cc
@@ -12,6 +12,7 @@
#include <mm/mm-modem.h>
#include "shill/cellular_service.h"
+#include "shill/error.h"
#include "shill/mock_device_info.h"
#include "shill/mock_dhcp_config.h"
#include "shill/mock_dhcp_provider.h"
@@ -502,14 +503,18 @@
} // namespace {}
TEST_F(CellularTest, Connect) {
+ Error error;
EXPECT_CALL(device_info_, GetFlags(device_->interface_index(), _))
.WillOnce(Return(true))
.WillOnce(Return(true));
device_->state_ = Cellular::kStateConnected;
- device_->Connect();
+ device_->Connect(&error);
+ EXPECT_EQ(Error::kAlreadyConnected, error.type());
+ error.Populate(Error::kSuccess);
device_->state_ = Cellular::kStateLinked;
- device_->Connect();
+ device_->Connect(&error);
+ EXPECT_EQ(Error::kAlreadyConnected, error.type());
device_->state_ = Cellular::kStateRegistered;
device_->service_ = new CellularService(
@@ -517,16 +522,20 @@
device_->allow_roaming_ = false;
device_->service_->set_roaming_state(flimflam::kRoamingStateRoaming);
- device_->Connect();
+ 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_->Connect();
+ 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_->Connect();
+ device_->Connect(&error);
+ EXPECT_TRUE(error.IsSuccess());
DBusPropertiesMap properties;
properties[Cellular::kConnectPropertyPhoneNumber].writer().append_string(
diff --git a/ethernet_service.cc b/ethernet_service.cc
index 4193cb0..9479535 100644
--- a/ethernet_service.cc
+++ b/ethernet_service.cc
@@ -44,7 +44,7 @@
EthernetService::~EthernetService() { }
-void EthernetService::Connect() { }
+void EthernetService::Connect(Error *error) { }
void EthernetService::Disconnect() { }
diff --git a/ethernet_service.h b/ethernet_service.h
index adcc0eb..d0dbf0b 100644
--- a/ethernet_service.h
+++ b/ethernet_service.h
@@ -24,8 +24,10 @@
Manager *manager,
const EthernetRefPtr &device);
~EthernetService();
- void Connect();
- void Disconnect();
+
+ // Inherited from Service.
+ virtual void Connect(Error *error);
+ virtual void Disconnect();
// ethernet_<MAC>
virtual std::string GetStorageIdentifier(const std::string &mac);
diff --git a/mock_service.h b/mock_service.h
index a5af919..e82987c 100644
--- a/mock_service.h
+++ b/mock_service.h
@@ -25,7 +25,7 @@
Manager *manager);
virtual ~MockService();
- MOCK_METHOD0(Connect, void());
+ MOCK_METHOD1(Connect, void(Error *error));
MOCK_METHOD0(Disconnect, void());
MOCK_METHOD0(CalculateState, std::string());
MOCK_METHOD0(GetDeviceRpcId, std::string());
diff --git a/service.h b/service.h
index 9620933..a44d856 100644
--- a/service.h
+++ b/service.h
@@ -87,7 +87,7 @@
Manager *manager);
virtual ~Service();
- virtual void Connect() = 0;
+ virtual void Connect(Error *error) = 0;
virtual void Disconnect() = 0;
// The default implementation asserts.
diff --git a/service_dbus_adaptor.cc b/service_dbus_adaptor.cc
index 1b3f644..7aa801b 100644
--- a/service_dbus_adaptor.cc
+++ b/service_dbus_adaptor.cc
@@ -68,7 +68,9 @@
}
void ServiceDBusAdaptor::Connect(::DBus::Error &error) {
- service_->Connect();
+ Error e;
+ service_->Connect(&e);
+ e.ToDBusError(&error);
}
void ServiceDBusAdaptor::Disconnect(::DBus::Error &error) {
diff --git a/wifi_service.cc b/wifi_service.cc
index 6a2bdee..14105ac 100644
--- a/wifi_service.cc
+++ b/wifi_service.cc
@@ -57,7 +57,7 @@
LOG(INFO) << __func__;
}
-void WiFiService::Connect() {
+void WiFiService::Connect(Error *error) {
LOG(INFO) << __func__;
// NB(quiche) defer handling, since dbus-c++ does not permit us to
diff --git a/wifi_service.h b/wifi_service.h
index ec1a9c7..fe301dc 100644
--- a/wifi_service.h
+++ b/wifi_service.h
@@ -29,8 +29,10 @@
const std::string &mode,
const std::string &key_management);
~WiFiService();
- void Connect();
- void Disconnect();
+
+ // Inherited from Service.
+ virtual void Connect(Error *error);
+ virtual void Disconnect();
// wifi_<MAC>_<BSSID>_<mode_string>_<security_string>
std::string GetStorageIdentifier(const std::string &mac);