shill: cellular: call SetPowerState after disabling modem.
Since ModemManager no longer powers down the modem when the modem is
disabled, CellularCapabilityUniversal now calls the new SetPowerState
method on org.freedesktop.ModemManager1.Modem to put the modem into a
low power state after disabling it.
In the event that the call to SetPowerState fails, the error is ignored
and the overall disable operation is treated as success.
BUG=chromium-os:38864
TEST=1. Build and run unit tests.
2. Enable and disable cellular through the UI. On a modem that
supports the explicit switch to a low-power state,
ModemManager's PowerState property should be set to low after a
successful disable.
Change-Id: I631d8df118ed6e300ec0b197ec87e744bd2502bd
Reviewed-on: https://gerrit.chromium.org/gerrit/43241
Reviewed-by: Thieu Le <thieule@chromium.org>
Commit-Queue: Arman Uguray <armansito@chromium.org>
Tested-by: Arman Uguray <armansito@chromium.org>
diff --git a/cellular_capability_universal.cc b/cellular_capability_universal.cc
index 8a36418..574c8e5 100644
--- a/cellular_capability_universal.cc
+++ b/cellular_capability_universal.cc
@@ -67,6 +67,7 @@
const char CellularCapabilityUniversal::kOperatorAccessTechnologyProperty[] =
"access-technology";
const char CellularCapabilityUniversal::kE362ModelId[] = "E362 WWAN";
+const int CellularCapabilityUniversal::kSetPowerStateTimeoutSeconds = 20;
unsigned int CellularCapabilityUniversal::friendly_service_name_id_ = 0;
@@ -338,10 +339,48 @@
SLOG(Cellular, 2) << __func__;
if (error.IsSuccess()) {
- metrics()->NotifyDeviceDisableFinished(cellular()->interface_index());
- ReleaseProxies();
+ // The modem has been successfully disabled, but we still need to power it
+ // down.
+ Stop_PowerDown(callback);
+ } else {
+ // An error occurred; terminate the disable sequence.
+ callback.Run(error);
}
- callback.Run(error);
+}
+
+void CellularCapabilityUniversal::Stop_PowerDown(
+ const ResultCallback &callback) {
+ SLOG(Cellular, 2) << __func__;
+ Error error;
+ modem_proxy_->SetPowerState(
+ MM_MODEM_POWER_STATE_LOW,
+ &error,
+ Bind(&CellularCapabilityUniversal::Stop_PowerDownCompleted,
+ weak_ptr_factory_.GetWeakPtr(), callback),
+ kSetPowerStateTimeoutSeconds);
+
+ if (error.IsFailure())
+ // This really shouldn't happen, but if it does, report success,
+ // because a stop initiated power down is only called if the
+ // modem was successfully disabled, but the failure of this
+ // operation should still be propagated up as a successful disable.
+ Stop_PowerDownCompleted(callback, error);
+}
+
+void CellularCapabilityUniversal::Stop_PowerDownCompleted(
+ const ResultCallback &callback,
+ const Error &error) {
+ SLOG(Cellular, 2) << __func__;
+
+ if (error.IsFailure())
+ SLOG(Cellular, 2) << "Ignoring error returned by SetPowerState: " << error;
+
+ // Since the disable succeeded, if power down fails, we currently fail
+ // silently, i.e. we need to report the disable operation as having
+ // succeeded.
+ metrics()->NotifyDeviceDisableFinished(cellular()->interface_index());
+ ReleaseProxies();
+ callback.Run(Error());
}
void CellularCapabilityUniversal::Connect(const DBusPropertiesMap &properties,
diff --git a/cellular_capability_universal.h b/cellular_capability_universal.h
index 19d7667..bf4ee3a 100644
--- a/cellular_capability_universal.h
+++ b/cellular_capability_universal.h
@@ -131,6 +131,8 @@
static const char kGenericServiceNamePrefix[];
static const unsigned int kDefaultScanningOrSearchingTimeoutMilliseconds;
+ static const int kSetPowerStateTimeoutSeconds;
+
// Root path. The SIM path is reported by ModemManager to be the root path
// when no SIM is present.
@@ -186,6 +188,9 @@
void Stop_Disable(const ResultCallback &callback);
void Stop_DisableCompleted(const ResultCallback &callback,
const Error &error);
+ void Stop_PowerDown(const ResultCallback &callback);
+ void Stop_PowerDownCompleted(const ResultCallback &callback,
+ const Error &error);
// Updates the name property that is exposed by the service to Chrome.
void UpdateServiceName();
diff --git a/cellular_capability_universal_unittest.cc b/cellular_capability_universal_unittest.cc
index 508f0b5..af66ce9 100644
--- a/cellular_capability_universal_unittest.cc
+++ b/cellular_capability_universal_unittest.cc
@@ -421,8 +421,21 @@
.WillOnce(SaveArg<2>(&disable_callback));
dispatcher_.DispatchPendingEvents();
- EXPECT_CALL(*this, TestCallback(IsSuccess()));
+ ResultCallback set_power_state_callback;
+ EXPECT_CALL(*modem_proxy,
+ SetPowerState(
+ MM_MODEM_POWER_STATE_LOW, _, _,
+ CellularCapabilityUniversal::kSetPowerStateTimeoutSeconds))
+ .WillOnce(SaveArg<2>(&set_power_state_callback));
disable_callback.Run(Error(Error::kSuccess));
+
+ EXPECT_CALL(*this, TestCallback(IsSuccess()));
+ set_power_state_callback.Run(Error(Error::kSuccess));
+
+ // TestCallback should get called with success even if the power state
+ // callback gets called with an error
+ EXPECT_CALL(*this, TestCallback(IsSuccess()));
+ set_power_state_callback.Run(Error(Error::kOperationFailed));
}
TEST_F(CellularCapabilityUniversalMainTest, StopModemConnected) {
@@ -451,8 +464,17 @@
.WillOnce(SaveArg<2>(&disable_callback));
disconnect_callback.Run(Error(Error::kSuccess));
- EXPECT_CALL(*this, TestCallback(IsSuccess()));
+ ResultCallback set_power_state_callback;
+ EXPECT_CALL(*modem_proxy,
+ SetPowerState(
+ MM_MODEM_POWER_STATE_LOW, _, _,
+ CellularCapabilityUniversal::kSetPowerStateTimeoutSeconds))
+ .WillOnce(SaveArg<2>(&set_power_state_callback));
+
disable_callback.Run(Error(Error::kSuccess));
+
+ EXPECT_CALL(*this, TestCallback(IsSuccess()));
+ set_power_state_callback.Run(Error(Error::kSuccess));
}
TEST_F(CellularCapabilityUniversalMainTest, DisconnectModemNoBearer) {
diff --git a/mm1_modem_proxy.cc b/mm1_modem_proxy.cc
index 7649487..58736d5 100644
--- a/mm1_modem_proxy.cc
+++ b/mm1_modem_proxy.cc
@@ -161,6 +161,21 @@
}
}
+void ModemProxy::SetPowerState(const uint32_t &power_state,
+ Error *error,
+ const ResultCallback &callback,
+ int timeout) {
+ scoped_ptr<ResultCallback> cb(new ResultCallback(callback));
+ try {
+ SLOG(DBus, 2) << __func__;
+ proxy_.SetPowerState(power_state, cb.get(), timeout);
+ cb.release();
+ } catch (const DBus::Error &e) {
+ if (error)
+ CellularError::FromDBusError(e, error);
+ }
+}
+
// Inherited properties from ModemProxyInterface.
const ::DBus::Path ModemProxy::Sim() {
SLOG(DBus, 2) << __func__;
@@ -378,6 +393,15 @@
return std::vector<uint32_t>(); // Make the compiler happy.
}
}
+uint32_t ModemProxy::PowerState() {
+ SLOG(DBus, 2) << __func__;
+ try {
+ return proxy_.PowerState();
+ } catch (const DBus::Error &e) {
+ LOG(FATAL) << "DBus exception: " << e.name() << ": " << e.what();
+ return 0; // Make the compiler happy.
+ }
+}
ModemProxy::Proxy::Proxy(DBus::Connection *connection,
const std::string &path,
@@ -489,5 +513,14 @@
callback->Run(error);
}
+void ModemProxy::Proxy::SetPowerStateCallback(const ::DBus::Error &dberror,
+ void *data) {
+ SLOG(DBus, 2) << __func__;
+ scoped_ptr<ResultCallback> callback(reinterpret_cast<ResultCallback *>(data));
+ Error error;
+ CellularError::FromDBusError(dberror, &error);
+ callback->Run(error);
+}
+
} // namespace mm1
} // namespace shill
diff --git a/mm1_modem_proxy.h b/mm1_modem_proxy.h
index c95ae9a..d6c934f 100644
--- a/mm1_modem_proxy.h
+++ b/mm1_modem_proxy.h
@@ -60,6 +60,10 @@
Error *error,
const StringCallback &callback,
int timeout);
+ virtual void SetPowerState(const uint32_t &power_state,
+ Error *error,
+ const ResultCallback &callback,
+ int timeout);
virtual void set_state_changed_callback(
const ModemStateChangedSignalCallback &callback);
@@ -89,6 +93,7 @@
virtual uint32_t PreferredMode();
virtual const std::vector< uint32_t > SupportedBands();
virtual const std::vector< uint32_t > Bands();
+ virtual uint32_t PowerState();
private:
class Proxy : public org::freedesktop::ModemManager1::Modem_proxy,
@@ -126,6 +131,8 @@
virtual void CommandCallback(const std::string &response,
const ::DBus::Error &dberror,
void *data);
+ virtual void SetPowerStateCallback(const ::DBus::Error &dberror,
+ void *data);
ModemStateChangedSignalCallback state_changed_callback_;
diff --git a/mm1_modem_proxy_interface.h b/mm1_modem_proxy_interface.h
index 5d4b71c..b21a3f1 100644
--- a/mm1_modem_proxy_interface.h
+++ b/mm1_modem_proxy_interface.h
@@ -63,6 +63,10 @@
Error *error,
const StringCallback &callback,
int timeout) = 0;
+ virtual void SetPowerState(const uint32_t &power_state,
+ Error *error,
+ const ResultCallback &callback,
+ int timeout) = 0;
virtual void set_state_changed_callback(
@@ -93,6 +97,7 @@
virtual uint32_t PreferredMode() = 0;
virtual const std::vector< uint32_t > SupportedBands() = 0;
virtual const std::vector< uint32_t > Bands() = 0;
+ virtual uint32_t PowerState() = 0;
};
} // namespace mm1
diff --git a/mock_mm1_modem_proxy.h b/mock_mm1_modem_proxy.h
index 47bff43..e49c0ba 100644
--- a/mock_mm1_modem_proxy.h
+++ b/mock_mm1_modem_proxy.h
@@ -57,6 +57,10 @@
Error *error,
const StringCallback &callback,
int timeout));
+ MOCK_METHOD4(SetPowerState, void(const uint32_t &power_state,
+ Error *error,
+ const ResultCallback &callback,
+ int timeout));
MOCK_METHOD1(set_state_changed_callback, void(
const ModemStateChangedSignalCallback &callback));
@@ -87,6 +91,7 @@
MOCK_METHOD0(PreferredMode, uint32_t());
MOCK_METHOD0(SupportedBands, const std::vector< uint32_t >());
MOCK_METHOD0(Bands, const std::vector< uint32_t >());
+ MOCK_METHOD0(PowerState, uint32_t());
private:
DISALLOW_COPY_AND_ASSIGN(MockModemProxy);