shill: Handle modem state changes reported by modem-manager.
Also, adjust correctly to initial state reported by
modem-manager. This involved some adjustments to the
StartModem path, e.g., to make the MM Enable operation
conditional on whether the current modem state is not
already Enabled. Other streamlining done for the StartModem
path includes:
- eliminating the GetModemStatus call in the GSM
case. All the needed information (such as IMSI)
is obtained in later calls.
- making the Register call conditional on the modem
not already being registered.
I added a couple of unit tests to check responses to
state changes, but many more tests could be created.
BUG=chromium-os:19962
TEST=Testing includes running mm-enable and
mm-disable while the modem is in the disabled, enabled,
registered, and connected states, and making sure that
shill updates its own state appropriately, and that the
UI properly reflects the new state. Also ran mm-connect
and mm-disconnect to see that the UI (and shill) respond
correctly to connect and disconnect events. Similarly,
started up shill while modem started out in these various
states.
Tested only using MM classic API. Code is in place to make
this work for the MM1 API, but I haven't tried testing while
the new modem manager is running.
Change-Id: I4f276bee62b64e8f1260a39f18c3c2a5de8d2798
Reviewed-on: https://gerrit.chromium.org/gerrit/20517
Commit-Ready: Eric Shienbrood <ers@chromium.org>
Reviewed-by: Eric Shienbrood <ers@chromium.org>
Tested-by: Eric Shienbrood <ers@chromium.org>
diff --git a/cellular.cc b/cellular.cc
index 4819130..1cac8f7 100644
--- a/cellular.cc
+++ b/cellular.cc
@@ -162,12 +162,6 @@
if (state_ != kStateDisabled) {
return;
}
- if (modem_state_ == kModemStateEnabled) {
- // Modem already enabled. Make sure shill
- // state matches ModemManager state.
- SetState(kStateEnabled);
- return;
- }
capability_->StartModem(error,
Bind(&Cellular::OnModemStarted, this, callback));
}
@@ -184,6 +178,31 @@
Bind(&Cellular::OnModemStopped, this, callback));
}
+bool Cellular::IsUnderlyingDeviceEnabled() const {
+ return IsEnabledModemState(modem_state_);
+}
+
+// static
+bool Cellular::IsEnabledModemState(ModemState state) {
+ switch (state) {
+ case kModemStateUnknown:
+ case kModemStateDisabled:
+ case kModemStateInitializing:
+ case kModemStateLocked:
+ case kModemStateDisabling:
+ case kModemStateEnabling:
+ return false;
+ case kModemStateEnabled:
+ case kModemStateSearching:
+ case kModemStateRegistered:
+ case kModemStateDisconnecting:
+ case kModemStateConnecting:
+ case kModemStateConnected:
+ return true;
+ }
+ return false;
+}
+
void Cellular::OnModemStarted(const EnabledStateChangedCallback &callback,
const Error &error) {
SLOG(Cellular, 2) << __func__ << ": " << GetStateString(state_);
@@ -338,10 +357,10 @@
DBusPropertiesMap properties;
capability_->SetupConnectProperties(&properties);
- service_->SetState(Service::kStateAssociating);
// TODO(njw): Should a weak pointer be used here instead?
// Would require something like a WeakPtrFactory on the class.
ResultCallback cb = Bind(&Cellular::OnConnectReply, this);
+ OnConnecting();
capability_->Connect(properties, error, cb);
}
@@ -355,8 +374,17 @@
OnConnectFailed(error);
}
+void Cellular::OnConnecting() {
+ if (service_)
+ service_->SetState(Service::kStateAssociating);
+}
+
void Cellular::OnConnected() {
SLOG(Cellular, 2) << __func__;
+ if (state_ == kStateConnected || state_ == kStateLinked) {
+ VLOG(2) << "Already connected";
+ return;
+ }
SetState(kStateConnected);
if (!capability_->AllowRoaming() &&
service_->roaming_state() == flimflam::kRoamingStateRoaming) {
@@ -367,7 +395,7 @@
}
void Cellular::OnConnectFailed(const Error &error) {
- service()->SetFailure(Service::kFailureUnknown);
+ service_->SetFailure(Service::kFailureUnknown);
}
void Cellular::Disconnect(Error *error) {
@@ -397,6 +425,8 @@
if (state_ == kStateConnected || state_ == kStateLinked) {
SetState(kStateRegistered);
SetServiceFailureSilent(Service::kFailureUnknown);
+ DestroyIPConfig();
+ rtnl_handler()->SetInterfaceFlags(interface_index(), 0, IFF_UP);
} else {
LOG(WARNING) << "Disconnect occurred while in state "
<< GetStateString(state_);
@@ -425,7 +455,6 @@
if ((flags & IFF_UP) != 0 && state_ == kStateConnected) {
LOG(INFO) << link_name() << " is up.";
SetState(kStateLinked);
- // TODO(petkov): For GSM, remember the APN.
if (AcquireIPConfig()) {
SelectService(service_);
SetServiceState(Service::kStateConfiguring);
@@ -456,4 +485,41 @@
return capability_.get() ? capability_->CreateFriendlyServiceName() : "";
}
+void Cellular::OnModemStateChanged(ModemState old_state,
+ ModemState new_state,
+ uint32 /*reason*/) {
+ if (old_state == new_state) {
+ return;
+ }
+ set_modem_state(new_state);
+ switch (new_state) {
+ case kModemStateDisabled:
+ SetEnabled(false);
+ break;
+ case kModemStateEnabled:
+ case kModemStateSearching:
+ // Note: we only handle changes to Enabled from the Registered
+ // state here. Changes from Disabled to Enabled are handled in
+ // the DBusPropertiesChanged handler.
+ if (old_state == kModemStateRegistered) {
+ capability_->SetUnregistered(new_state == kModemStateSearching);
+ HandleNewRegistrationState();
+ }
+ case kModemStateRegistered:
+ if (old_state == kModemStateConnected ||
+ old_state == kModemStateConnecting ||
+ old_state == kModemStateDisconnecting)
+ OnDisconnected();
+ break;
+ case kModemStateConnecting:
+ OnConnecting();
+ break;
+ case kModemStateConnected:
+ OnConnected();
+ break;
+ default:
+ break;
+ }
+}
+
} // namespace shill