shill: Add Cellular devices to DeviceInfo.
This allows DeviceInfo to handle link messages for Cellular devices, so that we
can bring their interfaces up and down.
BUG=chromium-os:18438,18855
TEST=unit tests, tested on device
Change-Id: I7d642c7302472472c56042f731f746c8b997a149
Reviewed-on: http://gerrit.chromium.org/gerrit/5604
Tested-by: Darin Petkov <petkov@chromium.org>
Reviewed-by: Paul Stewart <pstew@chromium.org>
diff --git a/cellular.cc b/cellular.cc
index 2c56b9d..f1d7a45 100644
--- a/cellular.cc
+++ b/cellular.cc
@@ -372,7 +372,7 @@
// TODO(petkov): Set old_url/usage_url.
}
-bool Cellular::TechnologyIs(const Device::Technology type) {
+bool Cellular::TechnologyIs(const Device::Technology type) const {
return type == Device::kCellular;
}
diff --git a/cellular.h b/cellular.h
index dc9b00b..d88a69c 100644
--- a/cellular.h
+++ b/cellular.h
@@ -103,7 +103,7 @@
// Inherited from Device.
virtual void Start();
virtual void Stop();
- virtual bool TechnologyIs(Technology type);
+ virtual bool TechnologyIs(Technology type) const;
private:
static const char kPhoneNumberCDMA[];
diff --git a/device.cc b/device.cc
index e212cbf..ef75118 100644
--- a/device.cc
+++ b/device.cc
@@ -115,7 +115,7 @@
adaptor_->UpdateEnabled();
}
-bool Device::TechnologyIs(const Technology type) {
+bool Device::TechnologyIs(const Technology type) const {
return false;
}
diff --git a/device.h b/device.h
index be2ba2a..2bb068d 100644
--- a/device.h
+++ b/device.h
@@ -53,7 +53,7 @@
virtual void Stop();
// Base method always returns false.
- virtual bool TechnologyIs(const Technology type);
+ virtual bool TechnologyIs(const Technology type) const;
virtual void LinkEvent(unsigned flags, unsigned change);
virtual void Scan();
diff --git a/device_info.cc b/device_info.cc
index 5a368ef..908a2f9 100644
--- a/device_info.cc
+++ b/device_info.cc
@@ -34,6 +34,7 @@
#include "shill/service.h"
#include "shill/wifi.h"
+using base::hash_map;
using std::string;
namespace shill {
@@ -77,6 +78,26 @@
link_listener_.reset(NULL);
}
+void DeviceInfo::RegisterDevice(const DeviceRefPtr &device) {
+ VLOG(2) << __func__ << "(" << device->link_name() << ", "
+ << device->interface_index() << ")";
+ CHECK(!ContainsKey(devices_, device->interface_index()));
+ devices_[device->interface_index()] = device;
+ if (device->TechnologyIs(Device::kCellular) ||
+ device->TechnologyIs(Device::kEthernet) ||
+ device->TechnologyIs(Device::kWifi)) {
+ manager_->RegisterDevice(device);
+ }
+}
+
+DeviceRefPtr DeviceInfo::GetDevice(int interface_index) {
+ hash_map<int, DeviceRefPtr>::iterator iter = devices_.find(interface_index);
+ if (iter == devices_.end()) {
+ return NULL;
+ }
+ return iter->second;
+}
+
Device::Technology DeviceInfo::GetDeviceTechnology(const char *interface_name) {
char contents[1024];
int length;
@@ -124,19 +145,16 @@
void DeviceInfo::AddLinkMsgHandler(struct nlmsghdr *hdr) {
struct ifinfomsg *msg = reinterpret_cast<struct ifinfomsg *>(NLMSG_DATA(hdr));
- base::hash_map<int, DeviceRefPtr>::iterator ndev =
- devices_.find(msg->ifi_index);
int dev_index = msg->ifi_index;
struct rtattr *rta;
int rta_bytes;
char *link_name = NULL;
- DeviceRefPtr device;
Device::Technology technology = Device::kUnknown;
- bool is_stub = false;
VLOG(2) << __func__;
- if (ndev == devices_.end()) {
+ DeviceRefPtr device = GetDevice(dev_index);
+ if (!device.get()) {
rta_bytes = IFLA_PAYLOAD(hdr);
for (rta = IFLA_RTA(msg); RTA_OK(rta, rta_bytes);
rta = RTA_NEXT(rta, rta_bytes)) {
@@ -175,15 +193,9 @@
default:
device = new DeviceStub(control_interface_, dispatcher_, manager_,
link_name, dev_index, technology);
- is_stub = true;
+ break;
}
-
- devices_[dev_index] = device;
-
- if (!is_stub)
- manager_->RegisterDevice(device);
- } else {
- device = ndev->second;
+ RegisterDevice(device);
}
device->LinkEvent(msg->ifi_flags, msg->ifi_change);
@@ -195,8 +207,7 @@
}
void DeviceInfo::RemoveDevice(int interface_index) {
- base::hash_map<int, DeviceRefPtr>::iterator ndev =
- devices_.find(interface_index);
+ hash_map<int, DeviceRefPtr>::iterator ndev = devices_.find(interface_index);
if (ndev != devices_.end()) {
VLOG(2) << "Removing device index: " << interface_index;
manager_->DeregisterDevice(ndev->second);
diff --git a/device_info.h b/device_info.h
index c7c0537..2c29dd7 100644
--- a/device_info.h
+++ b/device_info.h
@@ -34,7 +34,11 @@
void Start();
void Stop();
- static Device::Technology GetDeviceTechnology(const char *interface_name);
+ // Adds |device| to this DeviceInfo instance so that we can handle its link
+ // messages, and registers it with the manager.
+ void RegisterDevice(const DeviceRefPtr &device);
+
+ DeviceRefPtr GetDevice(int interface_index);
private:
friend class DeviceInfoTest;
@@ -43,6 +47,8 @@
static const char *kInterfaceDriver;
static const char *kModemDrivers[];
+ static Device::Technology GetDeviceTechnology(const char *interface_name);
+
void AddLinkMsgHandler(struct nlmsghdr *hdr);
void DelLinkMsgHandler(struct nlmsghdr *hdr);
void LinkMsgHandler(struct nlmsghdr *hdr);
diff --git a/device_stub.h b/device_stub.h
index e53a8d1..9e45fa0 100644
--- a/device_stub.h
+++ b/device_stub.h
@@ -37,10 +37,11 @@
technology_(technology) {}
void Start() {}
void Stop() {}
- bool TechnologyIs(const Technology type) { return type == technology_; }
+ bool TechnologyIs(const Technology type) const { return type == technology_; }
private:
Technology technology_;
+
DISALLOW_COPY_AND_ASSIGN(DeviceStub);
};
diff --git a/ethernet.cc b/ethernet.cc
index ae3c332..4cdedc9 100644
--- a/ethernet.cc
+++ b/ethernet.cc
@@ -62,7 +62,7 @@
RTNLHandler::GetInstance()->SetInterfaceFlags(interface_index_, 0, IFF_UP);
}
-bool Ethernet::TechnologyIs(const Device::Technology type) {
+bool Ethernet::TechnologyIs(const Device::Technology type) const {
return type == Device::kEthernet;
}
diff --git a/ethernet.h b/ethernet.h
index 95feec0..9967e1c 100644
--- a/ethernet.h
+++ b/ethernet.h
@@ -24,7 +24,7 @@
void Start();
void Stop();
- bool TechnologyIs(Device::Technology type);
+ bool TechnologyIs(Device::Technology type) const;
void LinkEvent(unsigned int flags, unsigned int change);
private:
diff --git a/manager.h b/manager.h
index b784cdf..154e51c 100644
--- a/manager.h
+++ b/manager.h
@@ -64,9 +64,12 @@
ServiceRefPtr FindService(const std::string& name);
std::vector<std::string> EnumerateAvailableServices();
+ DeviceInfo *device_info() { return &device_info_; }
PropertyStore *store() { return &store_; }
private:
+ friend class ManagerAdaptorInterface;
+
std::string CalculateState();
std::vector<std::string> AvailableTechnologies();
std::vector<std::string> ConnectedTechnologies();
@@ -97,8 +100,6 @@
// Properties to be get/set via PropertyStore calls.
Properties props_;
PropertyStore store_;
-
- friend class ManagerAdaptorInterface;
};
} // namespace shill
diff --git a/manager_unittest.cc b/manager_unittest.cc
index a659579..9cc7d5f 100644
--- a/manager_unittest.cc
+++ b/manager_unittest.cc
@@ -8,10 +8,7 @@
#include <glib.h>
-#include <base/callback_old.h>
#include <base/logging.h>
-#include <base/memory/ref_counted.h>
-#include <base/message_loop.h>
#include <base/stl_util-inl.h>
#include <chromeos/dbus/service_constants.h>
#include <gtest/gtest.h>
@@ -23,7 +20,6 @@
#include "shill/mock_profile.h"
#include "shill/mock_service.h"
#include "shill/property_store_unittest.h"
-#include "shill/shill_event.h"
using std::map;
using std::set;
@@ -39,17 +35,21 @@
class ManagerTest : public PropertyStoreTest {
public:
ManagerTest()
- : factory_(this),
- mock_device_(new NiceMock<MockDevice>(&control_interface_,
+ : mock_device_(new NiceMock<MockDevice>(&control_interface_,
&dispatcher_,
&manager_,
"null0",
- -1)),
+ 0)),
mock_device2_(new NiceMock<MockDevice>(&control_interface_,
&dispatcher_,
&manager_,
- "null1",
- -1)) {
+ "null2",
+ 2)),
+ mock_device3_(new NiceMock<MockDevice>(&control_interface_,
+ &dispatcher_,
+ &manager_,
+ "null3",
+ 3)) {
}
virtual ~ManagerTest() {}
@@ -60,9 +60,9 @@
}
protected:
- ScopedRunnableMethodFactory<ManagerTest> factory_;
scoped_refptr<MockDevice> mock_device_;
scoped_refptr<MockDevice> mock_device2_;
+ scoped_refptr<MockDevice> mock_device3_;
};
TEST_F(ManagerTest, Contains) {
@@ -75,12 +75,16 @@
.WillByDefault(Return(true));
ON_CALL(*mock_device2_.get(), TechnologyIs(Device::kWifi))
.WillByDefault(Return(true));
+ ON_CALL(*mock_device3_.get(), TechnologyIs(Device::kCellular))
+ .WillByDefault(Return(true));
- manager_.RegisterDevice(mock_device_.get());
- manager_.RegisterDevice(mock_device2_.get());
+ manager_.RegisterDevice(mock_device_);
+ manager_.RegisterDevice(mock_device2_);
+ manager_.RegisterDevice(mock_device3_);
EXPECT_TRUE(IsDeviceRegistered(mock_device_, Device::kEthernet));
EXPECT_TRUE(IsDeviceRegistered(mock_device2_, Device::kWifi));
+ EXPECT_TRUE(IsDeviceRegistered(mock_device3_, Device::kCellular));
}
TEST_F(ManagerTest, DeviceDeregistration) {
diff --git a/mock_device.h b/mock_device.h
index 4d054f1..2147bbc 100644
--- a/mock_device.h
+++ b/mock_device.h
@@ -26,9 +26,9 @@
int interface_index);
virtual ~MockDevice();
- MOCK_METHOD0(Start, void(void));
- MOCK_METHOD0(Stop, void(void));
- MOCK_METHOD1(TechnologyIs, bool(Technology));
+ MOCK_METHOD0(Start, void());
+ MOCK_METHOD0(Stop, void());
+ MOCK_CONST_METHOD1(TechnologyIs, bool(const Technology technology));
private:
DISALLOW_COPY_AND_ASSIGN(MockDevice);
diff --git a/modem.cc b/modem.cc
index 8e15156..37d52fd 100644
--- a/modem.cc
+++ b/modem.cc
@@ -110,7 +110,7 @@
type,
owner_,
path_);
- manager_->RegisterDevice(device_);
+ manager_->device_info()->RegisterDevice(device_);
string unlock_required;
if (DBusProperties::GetString(
diff --git a/modem_unittest.cc b/modem_unittest.cc
index 963365c..99263f3 100644
--- a/modem_unittest.cc
+++ b/modem_unittest.cc
@@ -2,6 +2,8 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
+#include <vector>
+
#include <gtest/gtest.h>
#include <mm/mm-modem.h>
#include <net/if.h>
@@ -19,6 +21,7 @@
#include "shill/shill_event.h"
using std::string;
+using std::vector;
using testing::_;
using testing::DoAll;
using testing::Return;
@@ -147,7 +150,11 @@
ASSERT_TRUE(modem_.device_.get());
EXPECT_EQ(kLinkName, modem_.device_->link_name());
EXPECT_EQ(kTestInterfaceIndex, modem_.device_->interface_index());
- // TODO(petkov): Confirm the device is registered by the manager.
+
+ vector<DeviceRefPtr> devices;
+ manager_.FilterByTechnology(Device::kCellular, &devices);
+ EXPECT_EQ(1, devices.size());
+ EXPECT_TRUE(devices[0].get() == modem_.device_.get());
}
} // namespace shill
diff --git a/wifi.cc b/wifi.cc
index 2cbaed1..6db73ab 100644
--- a/wifi.cc
+++ b/wifi.cc
@@ -132,7 +132,7 @@
// XXX anything else to do?
}
-bool WiFi::TechnologyIs(const Device::Technology type) {
+bool WiFi::TechnologyIs(const Device::Technology type) const {
return type == Device::kWifi;
}
diff --git a/wifi.h b/wifi.h
index 2808b79..86969da 100644
--- a/wifi.h
+++ b/wifi.h
@@ -32,7 +32,7 @@
virtual ~WiFi();
virtual void Start();
virtual void Stop();
- virtual bool TechnologyIs(const Technology type);
+ virtual bool TechnologyIs(const Technology type) const;
// called by SupplicantInterfaceProxy, in response to events from
// wpa_supplicant.