Skip to content

Commit

Permalink
[CrOS Cellular] Add multi-SIM support in OtaActivator
Browse files Browse the repository at this point in the history 
This CL updates OtaActivatorImpl so that it handles multiple
SIM slots correctly.

Bug: 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-Commit-Position: refs/heads/master@{#858272}
  • Loading branch information
Azeem Arshad authored and Chromium LUCI CQ committed Feb 26, 2021
1 parent a493642 commit fb1a7ab
Show file tree
Hide file tree
Showing 3 changed files with 96 additions and 35 deletions.
93 changes: 59 additions & 34 deletions chromeos/services/cellular_setup/ota_activator_impl.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -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()),
Expand All @@ -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);
}

Expand All @@ -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() ==
Expand All @@ -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.
Expand All @@ -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);
}

Expand Down
1 change: 1 addition & 0 deletions chromeos/services/cellular_setup/ota_activator_impl.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
37 changes: 36 additions & 1 deletion chromeos/services/cellular_setup/ota_activator_impl_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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);
Expand All @@ -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);
Expand All @@ -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();
Expand All @@ -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
Expand Down Expand Up @@ -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_;

Expand Down Expand Up @@ -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 */);
Expand Down

0 comments on commit fb1a7ab

Please sign in to comment.