shill: Delay cellular device creation if DeviceInfo is not available yet.
DeviceInfo notifies ModemInfo/ModemManager/Modem whenever a new cellular device
link is added so that cellular devices can be constructed and registered. This
fixes the current behavior where the device is not created at all if its device
info is not available yet.
BUG=chromium-os:24855
TEST=tested on device
Change-Id: I929bd70a6692a9ec6c66f51e01e44fca4433eb87
Reviewed-on: https://gerrit.chromium.org/gerrit/13866
Tested-by: Darin Petkov <petkov@chromium.org>
Reviewed-by: Paul Stewart <pstew@chromium.org>
Reviewed-by: Eric Shienbrood <ers@chromium.org>
Commit-Ready: Darin Petkov <petkov@chromium.org>
diff --git a/device_info.cc b/device_info.cc
index 193afb8..79588d2 100644
--- a/device_info.cc
+++ b/device_info.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium OS Authors. All rights reserved.
+// Copyright (c) 2012 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.
@@ -205,7 +205,8 @@
case Technology::kCellular:
// Cellular devices are managed by ModemInfo.
VLOG(2) << "Cellular link " << link_name << " at index " << dev_index
- << " ignored.";
+ << " -- notifying ModemInfo.";
+ manager_->modem_info()->OnDeviceInfoAvailable(link_name);
return;
case Technology::kEthernet:
device = new Ethernet(control_interface_, dispatcher_, manager_,
diff --git a/manager.h b/manager.h
index dd62b83..6f77c63 100644
--- a/manager.h
+++ b/manager.h
@@ -100,6 +100,7 @@
void PopAnyProfile(Error *error);
virtual DeviceInfo *device_info() { return &device_info_; }
+ ModemInfo *modem_info() { return &modem_info_; }
PropertyStore *mutable_store() { return &store_; }
virtual const PropertyStore &store() const { return store_; }
diff --git a/modem.cc b/modem.cc
index 9490656..f4f46cb 100644
--- a/modem.cc
+++ b/modem.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium OS Authors. All rights reserved.
+// Copyright (c) 2012 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.
@@ -36,7 +36,8 @@
control_interface_(control_interface),
dispatcher_(dispatcher),
manager_(manager),
- provider_db_(provider_db) {
+ provider_db_(provider_db),
+ pending_device_info_(false) {
LOG(INFO) << "Modem created: " << owner << " at " << path;
}
@@ -57,15 +58,31 @@
dbus_properties_proxy_.reset(
proxy_factory_->CreateDBusPropertiesProxy(this, path_, owner_));
+ CreateDevice();
+}
+void Modem::OnDeviceInfoAvailable(const string &link_name) {
+ VLOG(2) << __func__;
+ if (pending_device_info_ && link_name_ == link_name) {
+ pending_device_info_ = false;
+ CreateDevice();
+ }
+}
+
+void Modem::CreateDevice() {
+ VLOG(2) << __func__;
// TODO(petkov): Switch to asynchronous calls (crosbug.com/17583).
DBusPropertiesMap properties =
dbus_properties_proxy_->GetAll(MM_MODEM_INTERFACE);
- CreateCellularDevice(properties);
+ CreateDeviceFromProperties(properties);
}
-void Modem::CreateCellularDevice(const DBusPropertiesMap &properties) {
+void Modem::CreateDeviceFromProperties(const DBusPropertiesMap &properties) {
VLOG(2) << __func__;
+ if (device_.get()) {
+ return;
+ }
+
uint32 ip_method = kuint32max;
if (!DBusProperties::GetUint32(properties, kPropertyIPMethod, &ip_method) ||
ip_method != MM_MODEM_IP_METHOD_DHCP) {
@@ -73,13 +90,14 @@
return;
}
- string link_name;
- if (!DBusProperties::GetString(properties, kPropertyLinkName, &link_name)) {
+ if (!DBusProperties::GetString(properties, kPropertyLinkName, &link_name_)) {
LOG(ERROR) << "Unable to create cellular device without a link name.";
return;
}
+ // TODO(petkov): Get the interface index from DeviceInfo, similar to the MAC
+ // address below.
int interface_index =
- RTNLHandler::GetInstance()->GetInterfaceIndex(link_name);
+ RTNLHandler::GetInstance()->GetInterfaceIndex(link_name_);
if (interface_index < 0) {
LOG(ERROR) << "Unable to create cellular device -- no interface index.";
return;
@@ -88,9 +106,8 @@
ByteString address_bytes;
if (!manager_->device_info()->GetMACAddress(interface_index,
&address_bytes)) {
- // TODO(petkov): ensure that DeviceInfo has heard about this device before
- // we go ahead and try to add it.
- LOG(ERROR) << "Unable to create cellular device without a hardware addr.";
+ LOG(WARNING) << "No hardware address, device creation pending device info.";
+ pending_device_info_ = true;
return;
}
@@ -109,12 +126,12 @@
return;
}
- LOG(INFO) << "Creating a cellular device on link " << link_name
+ LOG(INFO) << "Creating a cellular device on link " << link_name_
<< " interface index " << interface_index << ".";
device_ = new Cellular(control_interface_,
dispatcher_,
manager_,
- link_name,
+ link_name_,
address_bytes.HexEncode(),
interface_index,
type,
diff --git a/modem.h b/modem.h
index 725b41b..654b649 100644
--- a/modem.h
+++ b/modem.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium OS Authors. All rights reserved.
+// Copyright (c) 2012 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.
@@ -44,12 +44,14 @@
// Cellular device.
void Init();
+ void OnDeviceInfoAvailable(const std::string &link_name);
+
private:
friend class ModemManagerTest;
friend class ModemTest;
FRIEND_TEST(ModemManagerTest, Connect);
FRIEND_TEST(ModemManagerTest, AddRemoveModem);
- FRIEND_TEST(ModemTest, CreateCellularDevice);
+ FRIEND_TEST(ModemTest, CreateDeviceFromProperties);
FRIEND_TEST(ModemTest, Init);
static const char kPropertyLinkName[];
@@ -62,7 +64,8 @@
// Creates and registers a Cellular device in |device_| based on
// ModemManager.Modem's |properties|. The device may not be created if the
// properties are invalid.
- void CreateCellularDevice(const DBusPropertiesMap &properties);
+ void CreateDeviceFromProperties(const DBusPropertiesMap &properties);
+ void CreateDevice();
// Signal callbacks inherited from DBusPropertiesProxyDelegate.
virtual void OnDBusPropertiesChanged(
@@ -90,6 +93,8 @@
EventDispatcher *dispatcher_;
Manager *manager_;
mobile_provider_db *provider_db_;
+ std::string link_name_;
+ bool pending_device_info_;
DISALLOW_COPY_AND_ASSIGN(Modem);
};
diff --git a/modem_info.cc b/modem_info.cc
index 0a29c5b..6add9bd 100644
--- a/modem_info.cc
+++ b/modem_info.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium OS Authors. All rights reserved.
+// Copyright (c) 2012 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.
@@ -50,6 +50,13 @@
modem_managers_.reset();
}
+void ModemInfo::OnDeviceInfoAvailable(const string &link_name) {
+ for (ModemManagers::iterator it = modem_managers_.begin();
+ it != modem_managers_.end(); ++it) {
+ (*it)->OnDeviceInfoAvailable(link_name);
+ }
+}
+
void ModemInfo::RegisterModemManager(const string &service,
const string &path) {
ModemManager *manager = new ModemManager(service,
diff --git a/modem_info.h b/modem_info.h
index 2678e37..4ecc9f4 100644
--- a/modem_info.h
+++ b/modem_info.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium OS Authors. All rights reserved.
+// Copyright (c) 2012 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.
@@ -32,11 +32,15 @@
void Start();
void Stop();
+ void OnDeviceInfoAvailable(const std::string &link_name);
+
private:
friend class ModemInfoTest;
FRIEND_TEST(ModemInfoTest, RegisterModemManager);
FRIEND_TEST(ModemInfoTest, StartStop);
+ typedef ScopedVector<ModemManager> ModemManagers;
+
static const char kCromoService[];
static const char kCromoPath[];
static const char kMobileProviderDBPath[];
@@ -45,7 +49,7 @@
void RegisterModemManager(const std::string &service,
const std::string &path);
- ScopedVector<ModemManager> modem_managers_;
+ ModemManagers modem_managers_;
ControlInterface *control_interface_;
EventDispatcher *dispatcher_;
diff --git a/modem_manager.cc b/modem_manager.cc
index f335053..f76c410 100644
--- a/modem_manager.cc
+++ b/modem_manager.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium OS Authors. All rights reserved.
+// Copyright (c) 2012 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.
@@ -95,7 +95,7 @@
manager->Disconnect();
}
-void ModemManager::AddModem(const std::string &path) {
+void ModemManager::AddModem(const string &path) {
LOG(INFO) << "Add modem: " << path;
CHECK(!owner_.empty());
if (ContainsKey(modems_, path)) {
@@ -112,10 +112,16 @@
modem->Init();
}
-void ModemManager::RemoveModem(const std::string &path) {
+void ModemManager::RemoveModem(const string &path) {
LOG(INFO) << "Remove modem: " << path;
CHECK(!owner_.empty());
modems_.erase(path);
}
+void ModemManager::OnDeviceInfoAvailable(const string &link_name) {
+ for (Modems::iterator it = modems_.begin(); it != modems_.end(); ++it) {
+ it->second->OnDeviceInfoAvailable(link_name);
+ }
+}
+
} // namespace shill
diff --git a/modem_manager.h b/modem_manager.h
index be404f9..0f26910 100644
--- a/modem_manager.h
+++ b/modem_manager.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium OS Authors. All rights reserved.
+// Copyright (c) 2012 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.
@@ -51,6 +51,8 @@
// Removes a modem on |path|.
void RemoveModem(const std::string &path);
+ void OnDeviceInfoAvailable(const std::string &link_name);
+
private:
friend class ModemManagerTest;
FRIEND_TEST(ModemInfoTest, RegisterModemManager);
diff --git a/modem_unittest.cc b/modem_unittest.cc
index 00646ca..1668c4d 100644
--- a/modem_unittest.cc
+++ b/modem_unittest.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium OS Authors. All rights reserved.
+// Copyright (c) 2012 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.
@@ -130,21 +130,21 @@
dispatcher_.DispatchPendingEvents();
}
-TEST_F(ModemTest, CreateCellularDevice) {
+TEST_F(ModemTest, CreateDeviceFromProperties) {
DBusPropertiesMap props;
- modem_.CreateCellularDevice(props);
+ modem_.CreateDeviceFromProperties(props);
EXPECT_FALSE(modem_.device_.get());
props[Modem::kPropertyIPMethod].writer().append_uint32(
MM_MODEM_IP_METHOD_PPP);
- modem_.CreateCellularDevice(props);
+ modem_.CreateDeviceFromProperties(props);
EXPECT_FALSE(modem_.device_.get());
props.erase(Modem::kPropertyIPMethod);
props[Modem::kPropertyIPMethod].writer().append_uint32(
MM_MODEM_IP_METHOD_DHCP);
- modem_.CreateCellularDevice(props);
+ modem_.CreateDeviceFromProperties(props);
EXPECT_FALSE(modem_.device_.get());
static const char kLinkName[] = "usb0";
@@ -168,7 +168,7 @@
.WillRepeatedly(Return(modem_.device_));
EXPECT_CALL(manager_, device_info()).WillRepeatedly(Return(&info_));
- modem_.CreateCellularDevice(props);
+ modem_.CreateDeviceFromProperties(props);
EXPECT_FALSE(modem_.device_.get());
props[Modem::kPropertyType].writer().append_uint32(MM_MODEM_TYPE_GSM);
@@ -180,7 +180,7 @@
kLockType);
props[CellularCapabilityGSM::kPropertyUnlockRetries].writer().append_uint32(
kRetries);
- modem_.CreateCellularDevice(props);
+ modem_.CreateDeviceFromProperties(props);
ASSERT_TRUE(modem_.device_.get());
EXPECT_EQ(kLinkName, modem_.device_->link_name());
EXPECT_EQ(kTestInterfaceIndex, modem_.device_->interface_index());