shill: cellular: Hande failures when querying modem state via proxy.
BUG=chromium:244485
TEST=Build and run unit tests.
Change-Id: I70bd091fe54a91e0bd614da0ffcdc4442be58fb1
Reviewed-on: https://chromium-review.googlesource.com/168552
Reviewed-by: Thieu Le <thieule@chromium.org>
Commit-Queue: Ben Chan <benchan@chromium.org>
Tested-by: Ben Chan <benchan@chromium.org>
diff --git a/cellular_capability_universal.cc b/cellular_capability_universal.cc
index e62d207..3c057d7 100644
--- a/cellular_capability_universal.cc
+++ b/cellular_capability_universal.cc
@@ -250,9 +250,16 @@
InitProxies();
// ModemManager must be in the disabled state to accept the Enable command.
+ // If the modem state is unknown, proceed with the Enable command as if the
+ // modem is disabled.
+ //
+ // TODO(benchan): Simply invoke the 'Enable' command regardless of the modem
+ // state and handle the DBus error returned by ModemManager accordingly
+ // (crbug.com/287667).
Cellular::ModemState state =
- static_cast<Cellular::ModemState>(modem_proxy_->State());
- if (state == Cellular::kModemStateDisabled) {
+ static_cast<Cellular::ModemState>(modem_proxy_->State(NULL));
+ if (state == Cellular::kModemStateUnknown ||
+ state == Cellular::kModemStateDisabled) {
EnableModem(error, callback);
} else if (!cellular()->IsUnderlyingDeviceEnabled()) {
SLOG(Cellular, 2) << "Enabling of modem deferred because state is "
diff --git a/cellular_capability_universal_unittest.cc b/cellular_capability_universal_unittest.cc
index b885830..3a062b7 100644
--- a/cellular_capability_universal_unittest.cc
+++ b/cellular_capability_universal_unittest.cc
@@ -364,7 +364,7 @@
// Let the modem report that it is initializing. StartModem() should defer
// enabling the modem until its state changes to disabled.
- EXPECT_CALL(*modem_proxy_, State())
+ EXPECT_CALL(*modem_proxy_, State(_))
.WillOnce(Return(Cellular::kModemStateInitializing));
Error error;
@@ -397,7 +397,7 @@
}
TEST_F(CellularCapabilityUniversalMainTest, StartModemFail) {
- EXPECT_CALL(*modem_proxy_, State())
+ EXPECT_CALL(*modem_proxy_, State(_))
.WillOnce(Return(Cellular::kModemStateDisabled));
EXPECT_CALL(*modem_proxy_,
Enable(true, _, _, CellularCapability::kTimeoutEnable))
@@ -412,8 +412,24 @@
EXPECT_TRUE(error.IsOngoing());
}
+TEST_F(CellularCapabilityUniversalMainTest, StartModemAndModemDisappears) {
+ EXPECT_CALL(*modem_proxy_, State(_))
+ .WillOnce(Return(Cellular::kModemStateUnknown));
+ EXPECT_CALL(*modem_proxy_,
+ Enable(true, _, _, CellularCapability::kTimeoutEnable))
+ .WillOnce(
+ Invoke(this, &CellularCapabilityUniversalTest::InvokeEnableFail));
+ EXPECT_CALL(*this, TestCallback(IsFailure()));
+ ResultCallback callback =
+ Bind(&CellularCapabilityUniversalTest::TestCallback, Unretained(this));
+
+ Error error;
+ capability_->StartModem(&error, callback);
+ EXPECT_TRUE(error.IsOngoing());
+}
+
TEST_F(CellularCapabilityUniversalMainTest, StartModemAlreadyEnabled) {
- EXPECT_CALL(*modem_proxy_, State())
+ EXPECT_CALL(*modem_proxy_, State(_))
.WillOnce(Return(Cellular::kModemStateEnabled));
capability_->cellular()->modem_state_ = Cellular::kModemStateConnected;
diff --git a/mm1_modem_proxy.cc b/mm1_modem_proxy.cc
index 9a1d293..be05d5f 100644
--- a/mm1_modem_proxy.cc
+++ b/mm1_modem_proxy.cc
@@ -4,6 +4,8 @@
#include "shill/mm1_modem_proxy.h"
+#include <ModemManager/ModemManager.h>
+
#include "shill/cellular_error.h"
#include "shill/logging.h"
@@ -342,13 +344,14 @@
}
}
-uint32_t ModemProxy::State() {
+uint32_t ModemProxy::State(Error *error) {
SLOG(DBus, 2) << __func__;
try {
return proxy_.State();
} catch (const DBus::Error &e) {
- LOG(FATAL) << "DBus exception: " << e.name() << ": " << e.what();
- return 0; // Make the compiler happy.
+ if (error)
+ CellularError::FromMM1DBusError(e, error);
+ return MM_MODEM_STATE_UNKNOWN;
}
}
diff --git a/mm1_modem_proxy.h b/mm1_modem_proxy.h
index ffab77f..f9be15f 100644
--- a/mm1_modem_proxy.h
+++ b/mm1_modem_proxy.h
@@ -88,7 +88,7 @@
virtual const std::string EquipmentIdentifier();
virtual uint32_t UnlockRequired();
virtual const std::map<uint32_t, uint32_t> UnlockRetries();
- virtual uint32_t State();
+ virtual uint32_t State(Error *error);
virtual uint32_t AccessTechnologies();
virtual const ::DBus::Struct<uint32_t, bool> SignalQuality();
virtual const std::vector<std::string> OwnNumbers();
diff --git a/mm1_modem_proxy_interface.h b/mm1_modem_proxy_interface.h
index 0b40d04..9613776 100644
--- a/mm1_modem_proxy_interface.h
+++ b/mm1_modem_proxy_interface.h
@@ -91,7 +91,7 @@
virtual const std::string EquipmentIdentifier() = 0;
virtual uint32_t UnlockRequired() = 0;
virtual const std::map<uint32_t, uint32_t> UnlockRetries() = 0;
- virtual uint32_t State() = 0;
+ virtual uint32_t State(Error *error) = 0;
virtual uint32_t AccessTechnologies() = 0;
virtual const ::DBus::Struct<uint32_t, bool> SignalQuality() = 0;
virtual const std::vector<std::string> OwnNumbers() = 0;
diff --git a/mock_mm1_modem_proxy.h b/mock_mm1_modem_proxy.h
index f7a231d..8ce48e1 100644
--- a/mock_mm1_modem_proxy.h
+++ b/mock_mm1_modem_proxy.h
@@ -85,7 +85,7 @@
MOCK_METHOD0(UnlockRequired, uint32_t());
typedef std::map<uint32_t, uint32_t> RetryData;
MOCK_METHOD0(UnlockRetries, const RetryData());
- MOCK_METHOD0(State, uint32_t());
+ MOCK_METHOD1(State, uint32_t(Error *error));
MOCK_METHOD0(AccessTechnologies, uint32_t());
typedef ::DBus::Struct<uint32_t, bool> SignalQualityData;
MOCK_METHOD0(SignalQuality, const SignalQualityData());