shill: wimax: Listen to WiMaxManager.Device.StatusChanged signal.
Use the status signal to update the connection state of the WiMax
service appropriately. Also, make sure that WiMaxService is
disassociated from the carrier device if a connection can't be
initiated or if it's dropped.
BUG=chrome-os-partner:10010
TEST=unit tests
CQ-DEPEND=I29dcfe4915e6f2559d022c60353aa12358ef5966
CQ-DEPEND=I223eaf61894f74905c591e38590e5e0620d07be0
Change-Id: I5fe48f0cc84c066eb6a63dc5d347ac5f265b86b1
Reviewed-on: https://gerrit.chromium.org/gerrit/23879
Tested-by: Darin Petkov <petkov@chromium.org>
Reviewed-by: Paul Stewart <pstew@chromium.org>
Commit-Ready: Ben Chan <benchan@chromium.org>
diff --git a/mock_wimax_device_proxy.h b/mock_wimax_device_proxy.h
index 2b1412a..906a984 100644
--- a/mock_wimax_device_proxy.h
+++ b/mock_wimax_device_proxy.h
@@ -36,6 +36,8 @@
int timeout));
MOCK_METHOD1(set_networks_changed_callback,
void(const NetworksChangedCallback &callback));
+ MOCK_METHOD1(set_status_changed_callback,
+ void(const StatusChangedCallback &callback));
MOCK_METHOD1(Index, uint8(Error *error));
MOCK_METHOD1(Name, std::string(Error *error));
diff --git a/mock_wimax_service.h b/mock_wimax_service.h
index 42c5ff3..52d1453 100644
--- a/mock_wimax_service.h
+++ b/mock_wimax_service.h
@@ -23,6 +23,7 @@
MOCK_METHOD1(Start, bool(WiMaxNetworkProxyInterface *proxy));
MOCK_METHOD0(Stop, void());
MOCK_CONST_METHOD0(IsStarted, bool());
+ MOCK_METHOD1(SetState, void(ConnectState state));
private:
DISALLOW_COPY_AND_ASSIGN(MockWiMaxService);
diff --git a/wimax.cc b/wimax.cc
index 03eb3ee..98db5de 100644
--- a/wimax.cc
+++ b/wimax.cc
@@ -51,6 +51,8 @@
proxy_.reset(proxy_factory_->CreateWiMaxDeviceProxy(path_));
proxy_->set_networks_changed_callback(
Bind(&WiMax::OnNetworksChanged, Unretained(this)));
+ proxy_->set_status_changed_callback(
+ Bind(&WiMax::OnStatusChanged, Unretained(this)));
proxy_->Enable(
error, Bind(&WiMax::OnEnableComplete, this, callback), kTimeoutDefault);
}
@@ -155,19 +157,17 @@
void WiMax::OnConnectComplete(const Error &error) {
SLOG(WiMax, 2) << __func__;
- if (!pending_service_) {
- LOG(ERROR) << "Unexpected OnConnectComplete callback.";
+ if (error.IsSuccess()) {
+ // Nothing to do -- the connection process is resumed on the StatusChanged
+ // signal.
return;
}
- if (error.IsSuccess() && AcquireIPConfig()) {
- LOG(INFO) << "Connected to " << pending_service_->friendly_name();
- SelectService(pending_service_);
- SetServiceState(Service::kStateConfiguring);
- } else {
- LOG(ERROR) << "Unable to connect to " << pending_service_->friendly_name();
+ if (pending_service_) {
+ LOG(ERROR) << "Unable to initiate connection to "
+ << pending_service_->GetStorageIdentifier();
pending_service_->SetState(Service::kStateFailure);
+ pending_service_ = NULL;
}
- pending_service_ = NULL;
}
void WiMax::OnDisconnectComplete(const Error &/*error*/) {
@@ -202,6 +202,43 @@
manager()->wimax_provider()->OnNetworksChanged();
}
+void WiMax::OnStatusChanged(wimax_manager::DeviceStatus status) {
+ SLOG(WiMax, 2) << __func__ << "(" << status << ")";
+ switch (status) {
+ case wimax_manager::kDeviceStatusConnected:
+ if (!pending_service_) {
+ LOG(WARNING) << "Unexpected status change; ignored.";
+ return;
+ }
+ if (AcquireIPConfig()) {
+ LOG(INFO) << "Connected to "
+ << pending_service_->GetStorageIdentifier();
+ SelectService(pending_service_);
+ SetServiceState(Service::kStateConfiguring);
+ } else {
+ LOG(ERROR) << "Unable to connect to "
+ << pending_service_->GetStorageIdentifier();
+ pending_service_->SetState(Service::kStateFailure);
+ }
+ pending_service_ = NULL;
+ break;
+ case wimax_manager::kDeviceStatusScanning:
+ case wimax_manager::kDeviceStatusConnecting:
+ // Nothing to do.
+ break;
+ default:
+ if (pending_service_) {
+ pending_service_->SetState(Service::kStateFailure);
+ pending_service_ = NULL;
+ }
+ if (selected_service()) {
+ selected_service()->SetState(Service::kStateFailure);
+ DropConnection();
+ }
+ break;
+ }
+}
+
void WiMax::DropConnection() {
DestroyIPConfig();
SelectService(NULL);
diff --git a/wimax.h b/wimax.h
index 1000d7e..7aab6b1 100644
--- a/wimax.h
+++ b/wimax.h
@@ -7,6 +7,7 @@
#include <set>
+#include <chromeos/dbus/service_constants.h>
#include <gtest/gtest_prod.h> // for FRIEND_TEST
#include "shill/device.h"
@@ -48,6 +49,7 @@
private:
friend class WiMaxTest;
FRIEND_TEST(WiMaxProviderTest, OnNetworksChanged);
+ FRIEND_TEST(WiMaxTest, OnConnectComplete);
FRIEND_TEST(WiMaxTest, OnNetworksChanged);
FRIEND_TEST(WiMaxTest, OnServiceStopped);
FRIEND_TEST(WiMaxTest, StartStop);
@@ -63,6 +65,7 @@
const Error &error);
void OnNetworksChanged(const RpcIdentifiers &networks);
+ void OnStatusChanged(wimax_manager::DeviceStatus status);
void DropConnection();
diff --git a/wimax_device_proxy.cc b/wimax_device_proxy.cc
index 72c24c5..72c779e 100644
--- a/wimax_device_proxy.cc
+++ b/wimax_device_proxy.cc
@@ -16,6 +16,7 @@
using base::Unretained;
using std::string;
using std::vector;
+using wimax_manager::DeviceStatus;
namespace shill {
@@ -81,6 +82,11 @@
proxy_.set_networks_changed_callback(callback);
}
+void WiMaxDeviceProxy::set_status_changed_callback(
+ const StatusChangedCallback &callback) {
+ proxy_.set_status_changed_callback(callback);
+}
+
uint8 WiMaxDeviceProxy::Index(Error *error) {
SLOG(DBus, 2) << __func__;
try {
@@ -139,6 +145,11 @@
networks_changed_callback_ = callback;
}
+void WiMaxDeviceProxy::Proxy::set_status_changed_callback(
+ const StatusChangedCallback &callback) {
+ status_changed_callback_ = callback;
+}
+
void WiMaxDeviceProxy::Proxy::NetworksChanged(
const vector<DBus::Path> &networks) {
SLOG(DBus, 2) << __func__ << "(" << networks.size() << ")";
@@ -150,6 +161,14 @@
networks_changed_callback_.Run(rpc_networks);
}
+void WiMaxDeviceProxy::Proxy::StatusChanged(const int32 &status) {
+ SLOG(DBus, 2) << __func__ << "(" << status << ")";
+ if (status_changed_callback_.is_null()) {
+ return;
+ }
+ status_changed_callback_.Run(static_cast<DeviceStatus>(status));
+}
+
void WiMaxDeviceProxy::Proxy::EnableCallback(const DBus::Error &error,
void *data) {
SLOG(DBus, 2) << __func__;
diff --git a/wimax_device_proxy.h b/wimax_device_proxy.h
index d61736d..04ce1eb 100644
--- a/wimax_device_proxy.h
+++ b/wimax_device_proxy.h
@@ -39,6 +39,8 @@
int timeout);
virtual void set_networks_changed_callback(
const NetworksChangedCallback &callback);
+ virtual void set_status_changed_callback(
+ const StatusChangedCallback &callback);
virtual uint8 Index(Error *error);
virtual std::string Name(Error *error);
@@ -50,10 +52,12 @@
virtual ~Proxy();
void set_networks_changed_callback(const NetworksChangedCallback &callback);
+ void set_status_changed_callback(const StatusChangedCallback &callback);
private:
// Signal callbacks inherited from WiMaxManager::Device_proxy.
virtual void NetworksChanged(const std::vector<DBus::Path> &networks);
+ virtual void StatusChanged(const int32 &status);
// Method callbacks inherited from WiMaxManager::Device_proxy.
virtual void EnableCallback(const DBus::Error &error, void *data);
@@ -65,6 +69,7 @@
static void HandleCallback(const DBus::Error &error, void *data);
NetworksChangedCallback networks_changed_callback_;
+ StatusChangedCallback status_changed_callback_;
DISALLOW_COPY_AND_ASSIGN(Proxy);
};
diff --git a/wimax_device_proxy_interface.h b/wimax_device_proxy_interface.h
index 7e97a8f..9a32253 100644
--- a/wimax_device_proxy_interface.h
+++ b/wimax_device_proxy_interface.h
@@ -8,6 +8,7 @@
#include <string>
#include <base/basictypes.h>
+#include <chromeos/dbus/service_constants.h>
#include "shill/callbacks.h"
@@ -21,6 +22,8 @@
class WiMaxDeviceProxyInterface {
public:
typedef base::Callback<void(const RpcIdentifiers &)> NetworksChangedCallback;
+ typedef base::Callback<void(
+ wimax_manager::DeviceStatus)> StatusChangedCallback;
virtual ~WiMaxDeviceProxyInterface() {}
@@ -44,6 +47,8 @@
virtual void set_networks_changed_callback(
const NetworksChangedCallback &callback) = 0;
+ virtual void set_status_changed_callback(
+ const StatusChangedCallback &callback) = 0;
// Properties.
virtual uint8 Index(Error *error) = 0;
diff --git a/wimax_service.cc b/wimax_service.cc
index 3bb7096..06463f3 100644
--- a/wimax_service.cc
+++ b/wimax_service.cc
@@ -135,14 +135,19 @@
error, Error::kAlreadyConnected, "Already connected.");
return;
}
- device_ = manager()->wimax_provider()->SelectCarrier(this);
- if (!device_) {
+ WiMaxRefPtr carrier = manager()->wimax_provider()->SelectCarrier(this);
+ if (!carrier) {
Error::PopulateAndLog(
error, Error::kNoCarrier, "No suitable WiMAX device available.");
return;
}
Service::Connect(error);
- device_->ConnectTo(this, error);
+ carrier->ConnectTo(this, error);
+ if (error->IsSuccess()) {
+ // Associate with the carrier device if the connection process has been
+ // initiated successfully.
+ device_ = carrier;
+ }
}
void WiMaxService::Disconnect(Error *error) {
@@ -212,6 +217,14 @@
return manager()->wimax_provider()->OnServiceUnloaded(this);
}
+void WiMaxService::SetState(ConnectState state) {
+ Service::SetState(state);
+ if (!IsConnecting() && !IsConnected()) {
+ // Disassociate from any carrier device if it's not connected anymore.
+ device_ = NULL;
+ }
+}
+
// static
WiMaxNetworkId WiMaxService::ConvertIdentifierToNetworkId(uint32 identifier) {
return base::StringPrintf("%08x", identifier);
diff --git a/wimax_service.h b/wimax_service.h
index f50510a..44c5ef8 100644
--- a/wimax_service.h
+++ b/wimax_service.h
@@ -72,12 +72,14 @@
virtual void set_eap(const EapCredentials &eap);
virtual bool Save(StoreInterface *storage);
virtual bool Unload();
+ virtual void SetState(ConnectState state);
private:
friend class WiMaxServiceTest;
FRIEND_TEST(WiMaxServiceTest, GetDeviceRpcId);
FRIEND_TEST(WiMaxServiceTest, OnSignalStrengthChanged);
FRIEND_TEST(WiMaxServiceTest, SetEAP);
+ FRIEND_TEST(WiMaxServiceTest, SetState);
FRIEND_TEST(WiMaxServiceTest, StartStop);
virtual std::string GetDeviceRpcId(Error *error);
diff --git a/wimax_service_unittest.cc b/wimax_service_unittest.cc
index 0cebd8f..38aef0d 100644
--- a/wimax_service_unittest.cc
+++ b/wimax_service_unittest.cc
@@ -240,4 +240,20 @@
EXPECT_TRUE(service_->eap().identity.empty());
}
+TEST_F(WiMaxServiceTest, SetState) {
+ service_->device_ = device_;
+ ServiceRefPtr base_service = service_;
+ EXPECT_EQ(Service::kStateIdle, service_->state());
+
+ EXPECT_CALL(manager_, UpdateService(_));
+ base_service->SetState(Service::kStateAssociating);
+ EXPECT_EQ(Service::kStateAssociating, service_->state());
+ EXPECT_TRUE(service_->device_);
+
+ EXPECT_CALL(manager_, UpdateService(_));
+ base_service->SetState(Service::kStateFailure);
+ EXPECT_EQ(Service::kStateFailure, service_->state());
+ EXPECT_FALSE(service_->device_);
+}
+
} // namespace shill
diff --git a/wimax_unittest.cc b/wimax_unittest.cc
index 5d972cd..2b21501 100644
--- a/wimax_unittest.cc
+++ b/wimax_unittest.cc
@@ -20,6 +20,7 @@
using std::string;
using testing::_;
+using testing::NiceMock;
using testing::Return;
namespace shill {
@@ -94,6 +95,7 @@
EXPECT_FALSE(device_->proxy_.get());
EXPECT_CALL(*proxy_, Enable(_, _, _));
EXPECT_CALL(*proxy_, set_networks_changed_callback(_));
+ EXPECT_CALL(*proxy_, set_status_changed_callback(_));
EXPECT_CALL(*proxy_, Disable(_, _, _));
device_->Start(NULL, EnabledStateChangedCallback());
ASSERT_TRUE(device_->proxy_.get());
@@ -107,8 +109,12 @@
}
TEST_F(WiMaxTest, OnServiceStopped) {
- scoped_refptr<MockWiMaxService> service0(
- new MockWiMaxService(&control_, NULL, &metrics_, &manager_));
+ scoped_refptr<NiceMock<MockWiMaxService> > service0(
+ new NiceMock<MockWiMaxService>(
+ &control_,
+ reinterpret_cast<EventDispatcher *>(NULL),
+ &metrics_,
+ &manager_));
scoped_refptr<MockWiMaxService> service1(
new MockWiMaxService(&control_, NULL, &metrics_, &manager_));
device_->SelectService(service0);
@@ -142,4 +148,15 @@
EXPECT_TRUE(ContainsKey(device_->networks_, "zoo"));
}
+TEST_F(WiMaxTest, OnConnectComplete) {
+ scoped_refptr<MockWiMaxService> service(
+ new MockWiMaxService(&control_, NULL, &metrics_, &manager_));
+ device_->pending_service_ = service;
+ EXPECT_CALL(*service, SetState(_)).Times(0);
+ EXPECT_TRUE(device_->pending_service_);
+ EXPECT_CALL(*service, SetState(Service::kStateFailure));
+ device_->OnConnectComplete(Error(Error::kOperationFailed));
+ EXPECT_FALSE(device_->pending_service_);
+}
+
} // namespace shill