[CrOS Cellular] Add multi-SIM support in OtaActivator

This CL updates OtaActivatorImpl so that it handles multiple
SIM slots correctly.

(cherry picked from commit fb1a7ab)

Fixed: 1182865
Change-Id: Ie8af216c2dda0e9011a850f86b2c526c92d378fb
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2708625
Commit-Queue: Azeem Arshad <azeemarshad@chromium.org>
Reviewed-by: Kyle Horimoto <khorimoto@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#858272}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2724013
Reviewed-by: Azeem Arshad <azeemarshad@chromium.org>
Commit-Queue: Kyle Horimoto <khorimoto@chromium.org>
Auto-Submit: Kyle Horimoto <khorimoto@chromium.org>
Cr-Commit-Position: refs/branch-heads/4430@{#10}
Cr-Branched-From: e5ce7dc-refs/heads/master@{#857950}

chromeos/services/cellular_setup/ota_activator_impl.cc

@@ -13,12 +13,14 @@
 #include "base/macros.h"
 #include "base/memory/ptr_util.h"
 #include "chromeos/dbus/shill/shill_device_client.h"
+#include "chromeos/network/cellular_utils.h"
 #include "chromeos/network/device_state.h"
 #include "chromeos/network/network_activation_handler.h"
 #include "chromeos/network/network_connection_handler.h"
 #include "chromeos/network/network_event_log.h"
 #include "chromeos/network/network_state.h"
 #include "chromeos/network/network_state_handler.h"
+#include "chromeos/network/network_util.h"
 #include "dbus/object_path.h"
 #include "third_party/cros_system_api/dbus/shill/dbus-constants.h"
 #include "url/gurl.h"
@@ -139,11 +141,15 @@ const DeviceState* OtaActivatorImpl::GetCellularDeviceState() const {
 }
 
 const NetworkState* OtaActivatorImpl::GetCellularNetworkState() const {
-  // Note: Chrome OS only supports up to one Cellular network at a time. Other
-  // configurations (e.g., a USB modem dongle in addition to an integrated SIM)
-  // are not supported.
-  return network_state_handler_->FirstNetworkByType(
-      NetworkTypePattern::Cellular());
+  NetworkStateHandler::NetworkStateList network_list;
+  network_state_handler_->GetVisibleNetworkListByType(
+      NetworkTypePattern::Cellular(), &network_list);
+  for (const NetworkState* network_state : network_list) {
+    if (network_state->iccid() == iccid_) {
+      return network_state;
+    }
+  }
+  return nullptr;
 }
 
 void OtaActivatorImpl::StartActivation() {
@@ -213,11 +219,31 @@ void OtaActivatorImpl::AttemptToDiscoverSim() {
   if (!cellular_device)
     return;
 
+  // Find status of first available pSIM slot.
+  // Note: We currently only support setting up devices with single physical
+  // SIM slots. This excludes other configurations such as multiple physical
+  // SIM slots and SIM cards in external dongles.
+  bool has_psim_slots = false;
+  for (const CellularSIMSlotInfo& sim_slot_info :
+       GetSimSlotInfosWithUpdatedEid(cellular_device)) {
+    if (sim_slot_info.eid.empty()) {
+      has_psim_slots = true;
+      iccid_ = sim_slot_info.iccid;
+      break;
+    }
+  }
+
+  if (!has_psim_slots) {
+    NET_LOG(ERROR) << "No PSim slots found";
+    FinishActivationAttempt(mojom::ActivationResult::kFailedToActivate);
+    return;
+  }
+
   // If no SIM card is present, it may be due to the fact that some devices do
   // not have hardware support for determining whether a SIM has been inserted.
   // Restart the modem to see if the SIM is detected when the modem powers back
   // on.
-  if (!cellular_device->sim_present()) {
+  if (iccid_.empty()) {
     NET_LOG(DEBUG) << "No SIM detected; restarting modem.";
     ShillDeviceClient::Get()->Reset(
         dbus::ObjectPath(cellular_device->path()),
@@ -227,19 +253,6 @@ void OtaActivatorImpl::AttemptToDiscoverSim() {
     return;
   }
 
-  // The device must have the properties required for the activation flow;
-  // namely, the operator name, IMEI, and MDN must be available. Return
-  // and wait to see if DevicePropertiesUpdated() is invoked with valid
-  // properties.
-  if (cellular_device->operator_name().empty() ||
-      cellular_device->imei().empty() || cellular_device->mdn().empty()) {
-    NET_LOG(DEBUG) << "Insufficient activation data: "
-                   << "Operator name: " << cellular_device->operator_name()
-                   << ", IMEI: " << cellular_device->imei() << ", "
-                   << "MDN: " << cellular_device->mdn();
-    return;
-  }
-
   ChangeStateAndAttemptNextStep(State::kWaitingForCellularConnection);
 }
 
@@ -253,6 +266,21 @@ void OtaActivatorImpl::AttemptConnectionToCellularNetwork() {
   if (!cellular_network)
     return;
 
+  // The network is disconnected; trigger a connection and wait for
+  // NetworkPropertiesUpdated() to be called when the network connects.
+  if (!cellular_network->IsConnectingOrConnected()) {
+    network_connection_handler_->ConnectToNetwork(
+        cellular_network->path(), base::DoNothing(),
+        base::BindOnce(&OnNetworkConnectionError),
+        false /* check_error_state */, ConnectCallbackMode::ON_STARTED);
+    return;
+  }
+
+  // The network is connecting; return early and wait for
+  // NetworkPropertiesUpdated() to be called if/when the network connects.
+  if (cellular_network->IsConnectingState())
+    return;
+
   // If the network is already activated, there is no need to complete the rest
   // of the flow.
   if (cellular_network->activation_state() ==
@@ -261,6 +289,18 @@ void OtaActivatorImpl::AttemptConnectionToCellularNetwork() {
     return;
   }
 
+  const DeviceState* cellular_device = GetCellularDeviceState();
+  // The device must have the properties required for the actication flow;
+  // namely, the operator name and IMEI. Return and wait to see if
+  // DevicePropertiesUpdated() is invoked with valid properties.
+  if (cellular_device->operator_name().empty() ||
+      cellular_device->imei().empty()) {
+    NET_LOG(DEBUG) << "Insufficient activation data: "
+                   << "Operator name: " << cellular_device->operator_name()
+                   << ", IMEI: " << cellular_device->imei();
+    return;
+  }
+
   // The network must have payment information; at minimum, a payment URL is
   // required in order to contact the carrier payment portal. Return and wait to
   // see if NetworkPropertiesUpdated() is invoked with valid properties.
@@ -271,21 +311,6 @@ void OtaActivatorImpl::AttemptConnectionToCellularNetwork() {
     return;
   }
 
-  // The network is disconnected; trigger a connection and wait for
-  // NetworkPropertiesUpdated() to be called when the network connects.
-  if (!cellular_network->IsConnectingOrConnected()) {
-    network_connection_handler_->ConnectToNetwork(
-        cellular_network->path(), base::DoNothing(),
-        base::BindOnce(&OnNetworkConnectionError),
-        false /* check_error_state */, ConnectCallbackMode::ON_STARTED);
-    return;
-  }
-
-  // The network is connecting; return early and wait for
-  // NetworkPropertiesUpdated() to be called if/when the network connects.
-  if (cellular_network->IsConnectingState())
-    return;
-
   ChangeStateAndAttemptNextStep(State::kWaitingForCellularPayment);
 }
 

chromeos/services/cellular_setup/ota_activator_impl.h

@@ -129,6 +129,7 @@ class OtaActivatorImpl : public OtaActivator,
 
   State state_ = State::kNotYetStarted;
   base::Optional<mojom::CarrierPortalStatus> last_carrier_portal_status_;
+  std::string iccid_;
   bool has_sent_metadata_ = false;
   bool has_called_complete_activation_ = false;
 

chromeos/services/cellular_setup/ota_activator_impl_unittest.cc

@@ -34,6 +34,7 @@ const char kTestCellularDeviceImei[] = "testDeviceImei";
 const char kTestCellularDeviceMdn[] = "testDeviceMdn";
 
 const char kTestCellularServicePath[] = "/service/cellular0";
+const char kTestCellularServiceIccid[] = "1234567890";
 const char kTestCellularServiceGuid[] = "testServiceGuid";
 const char kTestCellularServiceName[] = "testServiceName";
 const char kTestCellularServicePaymentUrl[] = "testServicePaymentUrl.com";
@@ -71,7 +72,7 @@ class CellularSetupOtaActivatorImplTest : public testing::Test {
     carrier_portal_handler_remote_.Bind(ota_activator_->GenerateRemote());
   }
 
-  void AddCellularDevice(bool has_valid_sim) {
+  void AddCellularDevice(bool has_valid_sim, bool has_physical_slots = true) {
     ShillDeviceClient::TestInterface* device_test = test_helper_.device_test();
     device_test->AddDevice(kTestCellularDevicePath, shill::kTypeCellular,
                            kTestCellularDeviceName);
@@ -80,6 +81,12 @@ class CellularSetupOtaActivatorImplTest : public testing::Test {
       device_test->SetDeviceProperty(
           kTestCellularDevicePath, shill::kSIMPresentProperty,
           base::Value(true), false /* notify_changed */);
+
+      device_test->SetDeviceProperty(
+          kTestCellularDevicePath, shill::kSIMSlotInfoProperty,
+          CreateCellularSIMSlotInfo(kTestCellularServiceIccid),
+          false /* notify_changed */);
+
       base::DictionaryValue home_provider;
       home_provider.SetString(shill::kOperatorNameKey,
                               kTestCellularDeviceCarrier);
@@ -95,6 +102,11 @@ class CellularSetupOtaActivatorImplTest : public testing::Test {
       device_test->SetDeviceProperty(
           kTestCellularDevicePath, shill::kMdnProperty,
           base::Value(kTestCellularDeviceMdn), false /* notify_changed */);
+    } else if (has_physical_slots) {
+      device_test->SetDeviceProperty(
+          kTestCellularDevicePath, shill::kSIMSlotInfoProperty,
+          CreateCellularSIMSlotInfo(kTestCellularServiceIccid),
+          false /* notify_changed */);
     }
 
     base::RunLoop().RunUntilIdle();
@@ -114,6 +126,9 @@ class CellularSetupOtaActivatorImplTest : public testing::Test {
         kTestCellularServicePath, kTestCellularServiceGuid,
         kTestCellularServiceName, shill::kTypeCellular,
         is_connected ? shill::kStateOnline : shill::kStateIdle, true);
+    service_test->SetServiceProperty(kTestCellularServicePath,
+                                     shill::kIccidProperty,
+                                     base::Value(kTestCellularServiceIccid));
     service_test->SetServiceProperty(
         kTestCellularServicePath, shill::kActivationStateProperty,
         is_already_activated
@@ -217,6 +232,16 @@ class CellularSetupOtaActivatorImplTest : public testing::Test {
     is_finished_ = true;
   }
 
+  base::Value CreateCellularSIMSlotInfo(const std::string& iccid) {
+    base::Value::ListStorage sim_slot_infos;
+    base::Value slot_info_item(base::Value::Type::DICTIONARY);
+    slot_info_item.SetStringKey(shill::kSIMSlotInfoEID, std::string());
+    slot_info_item.SetStringKey(shill::kSIMSlotInfoICCID, iccid);
+    slot_info_item.SetBoolKey(shill::kSIMSlotInfoPrimary, false);
+    sim_slot_infos.push_back(std::move(slot_info_item));
+    return base::Value(sim_slot_infos);
+  }
+
   base::test::TaskEnvironment task_environment_;
   NetworkStateTestHelper test_helper_;
 
@@ -256,6 +281,16 @@ TEST_F(CellularSetupOtaActivatorImplTest, Success) {
       mojom::ActivationResult::kSuccessfullyStartedActivation);
 }
 
+TEST_F(CellularSetupOtaActivatorImplTest, HasNoPhysicalSlots) {
+  AddCellularDevice(false /* has_valid_sim */, false /* has_physical_slots */);
+
+  BuildOtaActivator();
+
+  FlushForTesting();
+
+  VerifyActivationFinished(mojom::ActivationResult::kFailedToActivate);
+}
+
 TEST_F(CellularSetupOtaActivatorImplTest,
        SimAndPaymentInfoNotInitiallyPresent_AndNetworkNotConnected) {
   AddCellularDevice(false /* has_valid_sim */);