shill: Disconnect before activation correctly
Allow cellular capabilities to correctly disconnect before activation by
only responding to the final disconnect notification. Previously, shill
attempted to activate the modem upon receiving the first notification
after a disconnect operation, which is usually just a notification that
a disconnect is in progress. Seeing that the modem hadn't actually
disconnected, shill would report the activation as failed.
The meat of this change is that CellularCapabilityCDMA stores away the
activation callback when it finds it is connected during an activation
attempt. When the cellular device cleans up its disconnect process, it
calls down to the capability, giving us the opportunity to respond by
triggering our activation.
TEST=Unit tests, activations making use of this logic proceed correctly.
BUG=chromium-os:36301
Change-Id: I0169328403a805a06309a41b31e00b66657b9f5b
Reviewed-on: https://gerrit.chromium.org/gerrit/39028
Tested-by: Christopher Wiley <wiley@chromium.org>
Reviewed-by: Darin Petkov <petkov@chromium.org>
Reviewed-by: Thieu Le <thieule@chromium.org>
Commit-Ready: Christopher Wiley <wiley@chromium.org>
diff --git a/cellular_capability_cdma.cc b/cellular_capability_cdma.cc
index 2505fc9..b434c9d 100644
--- a/cellular_capability_cdma.cc
+++ b/cellular_capability_cdma.cc
@@ -140,19 +140,16 @@
activation_starting_ = true;
if (cellular()->state() == Cellular::kStateEnabled ||
cellular()->state() == Cellular::kStateRegistered) {
- ActivationResultCallback cb =
+ ActivationResultCallback activation_callback =
Bind(&CellularCapabilityCDMA::OnActivateReply,
weak_ptr_factory_.GetWeakPtr(),
callback);
- proxy_->Activate(carrier, error, cb, kTimeoutActivate);
+ proxy_->Activate(carrier, error, activation_callback, kTimeoutActivate);
} else if (cellular()->state() == Cellular::kStateConnected ||
cellular()->state() == Cellular::kStateLinked) {
- ResultCallback continue_activation_callback =
- Bind(&CellularCapabilityCDMA::OnDisconnectBeforeActivate,
- weak_ptr_factory_.GetWeakPtr(),
- carrier,
- callback);
- cellular()->DisconnectWithCallback(continue_activation_callback, error);
+ pending_activation_callback_ = callback;
+ pending_activation_carrier_ = carrier;
+ cellular()->Disconnect(error);
} else {
Error::PopulateAndLog(error, Error::kInvalidArguments,
"Unable to activate in " +
@@ -162,6 +159,7 @@
}
void CellularCapabilityCDMA::HandleNewActivationState(uint32 error) {
+ SLOG(Cellular, 2) << __func__ << "(" << error << ")";
if (!cellular()->service().get()) {
LOG(ERROR) << "In " << __func__ << "(): service is null.";
return;
@@ -171,6 +169,31 @@
cellular()->service()->set_error(GetActivationErrorString(error));
}
+void CellularCapabilityCDMA::DisconnectCleanup() {
+ CellularCapabilityClassic::DisconnectCleanup();
+ if (pending_activation_callback_.is_null()) {
+ return;
+ }
+ if (cellular()->state() == Cellular::kStateEnabled ||
+ cellular()->state() == Cellular::kStateRegistered) {
+ Error ignored_error;
+ Activate(pending_activation_carrier_,
+ &ignored_error,
+ pending_activation_callback_);
+ } else {
+ Error error;
+ Error::PopulateAndLog(
+ &error,
+ Error::kOperationFailed,
+ "Tried to disconnect before activating cellular service and failed");
+ HandleNewActivationState(MM_MODEM_CDMA_ACTIVATION_ERROR_UNKNOWN);
+ activation_starting_ = false;
+ pending_activation_callback_.Run(error);
+ }
+ pending_activation_callback_.Reset();
+ pending_activation_carrier_.clear();
+}
+
// static
string CellularCapabilityCDMA::GetActivationStateString(uint32 state) {
switch (state) {
@@ -314,20 +337,6 @@
callback.Run(error);
}
-void CellularCapabilityCDMA::OnDisconnectBeforeActivate(
- const string &carrier, const ResultCallback &callback, const Error &error) {
- SLOG(Cellular, 2) << __func__;
- if (error.IsSuccess()) {
- Error ignored_error;
- Activate(carrier, &ignored_error, callback);
- } else {
- LOG(WARNING) << "Tried to disconnect before activating cellular service"
- << " and failed.";
- HandleNewActivationState(MM_MODEM_CDMA_ACTIVATION_ERROR_UNKNOWN);
- activation_starting_ = false;
- }
-}
-
void CellularCapabilityCDMA::OnGetRegistrationStateReply(
uint32 state_1x, uint32 state_evdo, const Error &error) {
SLOG(Cellular, 2) << __func__;