hermes: Inhibit during all eSIM operations.
Motivation:
1) Notifications after profile operations were previously disabled.
However, due to certification requirements, these need to be sent. To
send notifications, we need to acquire a channel after a profile
operation. However, MM is starting around the same time, and there is a
race to acquire a channel. Thus, we need to inhibit MM until
notifications are sent.
2) Inhibiting MM while Hermes effectively acquires a lock on the modem.
It prevents fw updates and slot switches while Hermes is active.
3) Inhibit and Uninhibiting makes MM start from a clean state after
eSIM operations. MM was previously required to reprobe after a slot
switch or a profile switch. Since uninhibiting involves a reprobe,
this is no longer a requirement.
BUG=b:202401139
TEST=tast -verbose run ${cr} cellular.HermesMultiProfile
Cq-Depend: chromium:3379869, chromium:3379870
Change-Id: I68b9b186cec2ce911f9585797b671d859303b85f
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform2/+/3330727
Reviewed-by: Eric Caruso <ejcaruso@chromium.org>
Tested-by: Pavan Holla <pholla@google.com>
Commit-Queue: Pavan Holla <pholla@google.com>
diff --git a/hermes/BUILD.gn b/hermes/BUILD.gn
index 7846160..136a349 100644
--- a/hermes/BUILD.gn
+++ b/hermes/BUILD.gn
@@ -72,6 +72,7 @@
"daemon.cc",
"euicc.cc",
"euicc_dbus_adaptor.cc",
+ "euicc_event.cc",
"executor.cc",
"hermes_common.cc",
"lpa_util.cc",
diff --git a/hermes/euicc.cc b/hermes/euicc.cc
index bc30da1..d0909e4 100644
--- a/hermes/euicc.cc
+++ b/hermes/euicc.cc
@@ -29,19 +29,12 @@
const char kDefaultProdRootSmds[] = "lpa.ds.gsma.com";
const char kDefaultTestRootSmds[] = "testrootsmds.example.com";
-template <typename... T>
-void RunOnSuccess(base::OnceCallback<void(DbusResult<T...>)> cb,
- DbusResult<T...> dbus_result,
- int err) {
+void PrintEuiccEventResult(int err) {
if (err) {
- LOG(ERROR) << "Received modem error: " << err;
- auto decoded_error = brillo::Error::Create(
- FROM_HERE, brillo::errors::dbus::kDomain, kErrorUnknown,
- "QMI/MBIM operation failed with code: " + std::to_string(err));
- dbus_result.Error(decoded_error);
+ LOG(ERROR) << "ProcessEuiccEvent failed with err=" << err;
return;
}
- std::move(cb).Run(std::move(dbus_result));
+ VLOG(2) << "ProcessEuiccEvent succeeded";
}
} // namespace
@@ -91,10 +84,11 @@
auto download_profile =
base::BindOnce(&Euicc::DownloadProfile, weak_factory_.GetWeakPtr(),
std::move(activation_code), std::move(confirmation_code));
- context_->modem_control()->StoreAndSetActiveSlot(
- physical_slot_,
- base::BindOnce(&RunOnSuccess<dbus::ObjectPath>,
- std::move(download_profile), std::move(dbus_result)));
+ context_->modem_control()->ProcessEuiccEvent(
+ {physical_slot_, EuiccStep::START},
+ base::BindOnce(&Euicc::RunOnSuccess<dbus::ObjectPath>,
+ weak_factory_.GetWeakPtr(), std::move(download_profile),
+ std::move(dbus_result)));
}
void Euicc::DownloadProfile(std::string activation_code,
@@ -178,9 +172,10 @@
auto delete_profile =
base::BindOnce(&Euicc::DeleteProfile, weak_factory_.GetWeakPtr(),
std::move(profile_path), matching_profile->GetIccid());
- context_->modem_control()->StoreAndSetActiveSlot(
- physical_slot_, base::BindOnce(&RunOnSuccess<>, std::move(delete_profile),
- std::move(dbus_result)));
+ context_->modem_control()->ProcessEuiccEvent(
+ {physical_slot_, EuiccStep::START},
+ base::BindOnce(&Euicc::RunOnSuccess<>, weak_factory_.GetWeakPtr(),
+ std::move(delete_profile), std::move(dbus_result)));
}
void Euicc::DeleteProfile(dbus::ObjectPath profile_path,
@@ -218,7 +213,7 @@
LOG(INFO) << __func__;
auto decoded_error = LpaErrorToBrillo(FROM_HERE, error);
if (decoded_error) {
- dbus_result.Error(decoded_error);
+ EndEuiccOp(dbus_result, std::move(decoded_error));
return;
}
@@ -245,9 +240,10 @@
}
if (!profile) {
- dbus_result.Error(brillo::Error::Create(
+ auto profile_error = brillo::Error::Create(
FROM_HERE, brillo::errors::dbus::kDomain, kErrorInternalLpaFailure,
- "Failed to create Profile object"));
+ "Failed to create Profile object");
+ EndEuiccOp(dbus_result, std::move(profile_error));
return;
}
@@ -267,9 +263,9 @@
// Send notifications has completed, refresh the profile cache.
context_->lpa()->GetInstalledProfiles(
context_->executor(),
- [dbus_result{std::move(dbus_result)}, profile_path](
+ [this, dbus_result{std::move(dbus_result)}, profile_path](
std::vector<lpa::proto::ProfileInfo>& profile_infos,
- int /*error*/) { dbus_result.Success(profile_path); });
+ int /*error*/) { EndEuiccOp(dbus_result, profile_path); });
});
}
@@ -287,7 +283,7 @@
LOG(INFO) << __func__;
auto decoded_error = LpaErrorToBrillo(FROM_HERE, error);
if (decoded_error) {
- dbus_result.Error(decoded_error);
+ EndEuiccOp(dbus_result, std::move(decoded_error));
return;
}
@@ -311,9 +307,9 @@
[this, dbus_result{std::move(dbus_result)}](int /*err*/) {
context_->lpa()->GetInstalledProfiles(
context_->executor(),
- [dbus_result{std::move(dbus_result)}](
+ [this, dbus_result{std::move(dbus_result)}](
std::vector<lpa::proto::ProfileInfo>& profile_infos,
- int /*error*/) { dbus_result.Success(); });
+ int /*error*/) { EndEuiccOp(dbus_result); });
});
}
@@ -329,9 +325,10 @@
}
auto get_installed_profiles =
base::BindOnce(&Euicc::GetInstalledProfiles, weak_factory_.GetWeakPtr());
- context_->modem_control()->StoreAndSetActiveSlot(
- physical_slot_,
- base::BindOnce(&RunOnSuccess<>, std::move(get_installed_profiles),
+ context_->modem_control()->ProcessEuiccEvent(
+ {physical_slot_, EuiccStep::START},
+ base::BindOnce(&Euicc::RunOnSuccess<>, weak_factory_.GetWeakPtr(),
+ std::move(get_installed_profiles),
std::move(dbus_result)));
}
@@ -354,7 +351,7 @@
auto decoded_error = LpaErrorToBrillo(FROM_HERE, error);
if (decoded_error) {
LOG(ERROR) << "Failed to retrieve installed profiles";
- dbus_result.Error(decoded_error);
+ EndEuiccOp(dbus_result, std::move(decoded_error));
return;
}
installed_profiles_.clear();
@@ -372,7 +369,7 @@
}
}
UpdateInstalledProfilesProperty();
- dbus_result.Success();
+ EndEuiccOp(dbus_result);
}
void Euicc::RequestPendingProfiles(DbusResult<> dbus_result,
@@ -390,9 +387,10 @@
auto get_pending_profiles_from_smds =
base::BindOnce(&Euicc::GetPendingProfilesFromSmds,
weak_factory_.GetWeakPtr(), std::move(root_smds));
- context_->modem_control()->StoreAndSetActiveSlot(
- physical_slot_,
- base::BindOnce(&RunOnSuccess<>, std::move(get_pending_profiles_from_smds),
+ context_->modem_control()->ProcessEuiccEvent(
+ {physical_slot_, EuiccStep::START},
+ base::BindOnce(&Euicc::RunOnSuccess<>, weak_factory_.GetWeakPtr(),
+ std::move(get_pending_profiles_from_smds),
std::move(dbus_result)));
}
@@ -418,7 +416,7 @@
auto decoded_error = LpaErrorToBrillo(FROM_HERE, error);
if (decoded_error) {
LOG(ERROR) << "Failed to retrieve pending profiles";
- dbus_result.Error(decoded_error);
+ EndEuiccOp(dbus_result, std::move(decoded_error));
return;
}
@@ -434,7 +432,7 @@
}
}
UpdatePendingProfilesProperty();
- dbus_result.Success();
+ EndEuiccOp(dbus_result);
}
void Euicc::SetTestModeHelper(bool is_test_mode, DbusResult<> dbus_result) {
@@ -442,9 +440,10 @@
is_test_mode_ = is_test_mode;
auto set_test_mode_internal = base::BindOnce(
&Euicc::SetTestMode, weak_factory_.GetWeakPtr(), is_test_mode);
- context_->modem_control()->StoreAndSetActiveSlot(
- physical_slot_,
- base::BindOnce(&RunOnSuccess<>, std::move(set_test_mode_internal),
+ context_->modem_control()->ProcessEuiccEvent(
+ {physical_slot_, EuiccStep::START},
+ base::BindOnce(&Euicc::RunOnSuccess<>, weak_factory_.GetWeakPtr(),
+ std::move(set_test_mode_internal),
std::move(dbus_result)));
}
@@ -452,13 +451,13 @@
VLOG(2) << __func__ << " : is_test_mode" << is_test_mode;
context_->lpa()->SetTestMode(
is_test_mode, context_->executor(),
- [dbus_result{std::move(dbus_result)}](int error) {
+ [this, dbus_result{std::move(dbus_result)}](int error) {
auto decoded_error = LpaErrorToBrillo(FROM_HERE, error);
if (decoded_error) {
- dbus_result.Error(decoded_error);
+ EndEuiccOp(dbus_result, std::move(decoded_error));
return;
}
- dbus_result.Success();
+ EndEuiccOp(dbus_result);
});
}
@@ -483,10 +482,10 @@
auto reset_memory_internal = base::BindOnce(
&Euicc::ResetMemory, weak_factory_.GetWeakPtr(), reset_options);
- context_->modem_control()->StoreAndSetActiveSlot(
- physical_slot_,
- base::BindOnce(&RunOnSuccess<>, std::move(reset_memory_internal),
- std::move(dbus_result)));
+ context_->modem_control()->ProcessEuiccEvent(
+ {physical_slot_, EuiccStep::START},
+ base::BindOnce(&Euicc::RunOnSuccess<>, weak_factory_.GetWeakPtr(),
+ std::move(reset_memory_internal), std::move(dbus_result)));
}
void Euicc::ResetMemory(int reset_options, DbusResult<> dbus_result) {
@@ -496,7 +495,7 @@
[this, dbus_result{std::move(dbus_result)}](int error) {
auto decoded_error = LpaErrorToBrillo(FROM_HERE, error);
if (decoded_error) {
- dbus_result.Error(decoded_error);
+ EndEuiccOp(dbus_result, std::move(decoded_error));
return;
}
installed_profiles_.clear();
@@ -510,10 +509,10 @@
auto get_euicc_info_1 =
base::BindOnce(&Euicc::GetEuiccInfo1, weak_factory_.GetWeakPtr());
- context_->modem_control()->StoreAndSetActiveSlot(
- physical_slot_,
- base::BindOnce(&RunOnSuccess<bool>, std::move(get_euicc_info_1),
- std::move(dbus_result)));
+ context_->modem_control()->ProcessEuiccEvent(
+ {physical_slot_, EuiccStep::START},
+ base::BindOnce(&Euicc::RunOnSuccess<bool>, weak_factory_.GetWeakPtr(),
+ std::move(get_euicc_info_1), std::move(dbus_result)));
}
void Euicc::GetEuiccInfo1(DbusResult<bool> dbus_result) {
@@ -521,24 +520,63 @@
context_->lpa()->GetEuiccInfo1(
context_->executor(),
- [dbus_result{std::move(dbus_result)}](
+ [this, dbus_result{std::move(dbus_result)}](
lpa::proto::EuiccInfo1& euicc_info_1, int error) {
LOG(INFO) << "euicc_info_1:" << euicc_info_1.DebugString();
auto decoded_error = LpaErrorToBrillo(FROM_HERE, error);
if (decoded_error) {
- dbus_result.Error(decoded_error);
+ EndEuiccOp(dbus_result, std::move(decoded_error));
return;
}
constexpr char kTestEuiccInfo1[] =
"665A1433D67C1A2C5DB8B52C967F10A057BA5CB2";
for (const auto& pkid : euicc_info_1.pkid_for_verif()) {
if (pkid == kTestEuiccInfo1) {
- dbus_result.Success(true);
+ EndEuiccOp(dbus_result, true);
return;
}
}
- dbus_result.Success(false);
+ EndEuiccOp(dbus_result, false);
});
}
+template <typename... T>
+void Euicc::EndEuiccOp(DbusResult<T...> dbus_result, T... object) {
+ auto send_dbus_response = base::BindOnce(
+ [](const DbusResult<T...>& dbus_result, const T&... object, int err) {
+ PrintEuiccEventResult(err);
+ dbus_result.Success(object...);
+ },
+ dbus_result, object...);
+ context_->modem_control()->ProcessEuiccEvent({physical_slot_, EuiccStep::END},
+ std::move(send_dbus_response));
+}
+
+template <typename... T>
+void Euicc::EndEuiccOp(DbusResult<T...> dbus_result, brillo::ErrorPtr error) {
+ auto send_dbus_response = base::BindOnce(
+ [](const DbusResult<T...>& dbus_result, brillo::ErrorPtr error, int err) {
+ PrintEuiccEventResult(err);
+ dbus_result.Error(error);
+ },
+ dbus_result, std::move(error));
+ context_->modem_control()->ProcessEuiccEvent({physical_slot_, EuiccStep::END},
+ std::move(send_dbus_response));
+}
+
+template <typename... T>
+void Euicc::RunOnSuccess(base::OnceCallback<void(DbusResult<T...>)> cb,
+ DbusResult<T...> dbus_result,
+ int err) {
+ if (err) {
+ LOG(ERROR) << "Received modem error: " << err;
+ auto decoded_error = brillo::Error::Create(
+ FROM_HERE, brillo::errors::dbus::kDomain, kErrorUnknown,
+ "QMI/MBIM operation failed with code: " + std::to_string(err));
+ EndEuiccOp(dbus_result, std::move(decoded_error));
+ return;
+ }
+ std::move(cb).Run(std::move(dbus_result));
+}
+
} // namespace hermes
diff --git a/hermes/euicc.h b/hermes/euicc.h
index 7e47a36..2e2ffbd 100644
--- a/hermes/euicc.h
+++ b/hermes/euicc.h
@@ -105,6 +105,15 @@
std::vector<std::unique_ptr<Profile>> pending_profiles_;
base::WeakPtrFactory<Euicc> weak_factory_;
+
+ template <typename... T>
+ void EndEuiccOp(DbusResult<T...> dbus_result, T... object);
+ template <typename... T>
+ void EndEuiccOp(DbusResult<T...> dbus_result, brillo::ErrorPtr error);
+ template <typename... T>
+ void RunOnSuccess(base::OnceCallback<void(DbusResult<T...>)> cb,
+ DbusResult<T...> dbus_result,
+ int err);
};
} // namespace hermes
diff --git a/hermes/euicc_event.cc b/hermes/euicc_event.cc
new file mode 100644
index 0000000..3d20707
--- /dev/null
+++ b/hermes/euicc_event.cc
@@ -0,0 +1,58 @@
+// Copyright 2022 The Chromium OS Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include <ostream>
+
+#include "hermes/euicc_event.h"
+
+namespace hermes {
+
+EuiccEvent::EuiccEvent(uint32_t slot, EuiccStep step, EuiccOp op)
+ : slot(slot), step(step), op(op) {}
+
+EuiccEvent::EuiccEvent(uint32_t slot, EuiccStep step)
+ : slot(slot), step(step), op(EuiccOp::UNKNOWN) {}
+
+std::ostream& operator<<(std::ostream& os, const EuiccStep& rhs) {
+ switch (rhs) {
+ case (EuiccStep::START):
+ os << "START";
+ break;
+ case (EuiccStep::PENDING_NOTIFICATIONS):
+ os << "PENDING_NOTIFICATIONS";
+ break;
+ case (EuiccStep::END):
+ os << "END";
+ break;
+ default:
+ os << rhs;
+ break;
+ }
+ return os;
+}
+
+std::ostream& operator<<(std::ostream& os, const EuiccOp& rhs) {
+ switch (rhs) {
+ case (EuiccOp::UNKNOWN):
+ os << "UNKNOWN";
+ break;
+ case (EuiccOp::ENABLE):
+ os << "ENABLE";
+ break;
+ case (EuiccOp::DISABLE):
+ os << "DISABLE";
+ break;
+ default:
+ os << rhs;
+ break;
+ }
+ return os;
+}
+
+std::ostream& operator<<(std::ostream& os, const EuiccEvent& rhs) {
+ os << "Slot: " << rhs.slot << ", Step:" << rhs.step << ", Op:" << rhs.op;
+ return os;
+}
+
+} // namespace hermes
diff --git a/hermes/euicc_event.h b/hermes/euicc_event.h
new file mode 100644
index 0000000..d91adb4
--- /dev/null
+++ b/hermes/euicc_event.h
@@ -0,0 +1,33 @@
+// Copyright 2022 The Chromium OS Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef HERMES_EUICC_EVENT_H_
+#define HERMES_EUICC_EVENT_H_
+
+namespace hermes {
+
+enum class EuiccStep {
+ START,
+ PENDING_NOTIFICATIONS,
+ END,
+};
+
+enum class EuiccOp { UNKNOWN, DISABLE, ENABLE };
+
+struct EuiccEvent {
+ uint32_t slot;
+ EuiccStep step;
+ EuiccOp op;
+
+ EuiccEvent(uint32_t slot, EuiccStep step);
+ EuiccEvent(uint32_t slot, EuiccStep step, EuiccOp op);
+};
+
+std::ostream& operator<<(std::ostream& os, const EuiccStep& rhs);
+std::ostream& operator<<(std::ostream& os, const EuiccOp& rhs);
+std::ostream& operator<<(std::ostream& os, const EuiccEvent& rhs);
+
+} // namespace hermes
+
+#endif // HERMES_EUICC_EVENT_H_
diff --git a/hermes/euicc_interface.h b/hermes/euicc_interface.h
index def7886..2a83248 100644
--- a/hermes/euicc_interface.h
+++ b/hermes/euicc_interface.h
@@ -34,9 +34,8 @@
// from ModemControlInterface
virtual void StoreAndSetActiveSlot(uint32_t physical_slot,
ResultCallback cb) = 0;
+ virtual void ProcessEuiccEvent(EuiccEvent event, ResultCallback cb) = 0;
virtual void RestoreActiveSlot(ResultCallback cb) = 0;
- virtual void StartProfileOp(uint32_t physical_slot, ResultCallback cb) = 0;
- virtual void FinishProfileOp(ResultCallback cb) = 0;
// from lpa::card::EuiccCard
virtual void SendApdus(std::vector<lpa::card::Apdu> apdus,
diff --git a/hermes/hermes_common.h b/hermes/hermes_common.h
index 2cda4ea..9037401 100644
--- a/hermes/hermes_common.h
+++ b/hermes/hermes_common.h
@@ -17,7 +17,14 @@
constexpr char bcd_chars[] = "0123456789\0\0\0\0\0\0";
constexpr auto kLpaRetryDelay = base::TimeDelta::FromSeconds(2);
+// Duration that Chrome waits for Hermes to return a DBus response
+constexpr auto kHermesTimeout = base::TimeDelta::FromMinutes(4);
+
constexpr int kSuccess = 0;
+// This error will be returned when a received mbim/qmi message cannot be parsed
+// or when it is received in an unexpected state.
+constexpr int kModemMessageProcessingError = -1;
+constexpr int kModemManagerError = -2;
std::string GetTrailingChars(const std::string& pii, int num_chars);
// Used to redact PII in logs
diff --git a/hermes/modem.h b/hermes/modem.h
index 8cdf2b2..c9f02c4 100644
--- a/hermes/modem.h
+++ b/hermes/modem.h
@@ -26,11 +26,12 @@
constexpr auto kInitRetryDelay = base::TimeDelta::FromSeconds(10);
constexpr uint8_t kInvalidChannel = 0;
+// Schedule an uninhibit 2 seconds after an eSIM operation succeeds. If another
+// eSIM operation makes an inhibit call during these 2 seconds, the scheduled
+// uninhibit is cancelled.
+constexpr auto kUninhibitDelay = base::TimeDelta::FromSeconds(2);
+
constexpr int kModemSuccess = 0;
-// This error will be returned when a received mbim/qmi message cannot be parsed
-// or when it is received in an unexpected state.
-constexpr int kModemMessageProcessingError = -1;
-constexpr int kModemManagerError = -2;
constexpr int kDefault3GPPRelease = 11;
diff --git a/hermes/modem_control_interface.h b/hermes/modem_control_interface.h
index b967bc1..745ff5f 100644
--- a/hermes/modem_control_interface.h
+++ b/hermes/modem_control_interface.h
@@ -5,6 +5,7 @@
#ifndef HERMES_MODEM_CONTROL_INTERFACE_H_
#define HERMES_MODEM_CONTROL_INTERFACE_H_
+#include "hermes/euicc_event.h"
#include "hermes/hermes_common.h"
namespace hermes {
@@ -18,8 +19,7 @@
// Restore the value stored by StoreAndSetActiveSlot
virtual void RestoreActiveSlot(ResultCallback cb) = 0;
- virtual void StartProfileOp(uint32_t physical_slot, ResultCallback cb) = 0;
- virtual void FinishProfileOp(ResultCallback cb) = 0;
+ virtual void ProcessEuiccEvent(EuiccEvent event, ResultCallback cb) = 0;
virtual ~ModemControlInterface() = default;
};
diff --git a/hermes/modem_manager_proxy.cc b/hermes/modem_manager_proxy.cc
index a511121..6d54397 100644
--- a/hermes/modem_manager_proxy.cc
+++ b/hermes/modem_manager_proxy.cc
@@ -12,10 +12,13 @@
ModemManagerProxy::ModemManagerProxy(const scoped_refptr<dbus::Bus>& bus)
: bus_(bus),
- proxy_(std::make_unique<org::freedesktop::DBus::ObjectManagerProxy>(
- bus,
- modemmanager::kModemManager1ServiceName,
- dbus::ObjectPath(modemmanager::kModemManager1ServicePath))),
+ object_manager_proxy_(
+ std::make_unique<org::freedesktop::DBus::ObjectManagerProxy>(
+ bus,
+ modemmanager::kModemManager1ServiceName,
+ dbus::ObjectPath(modemmanager::kModemManager1ServicePath))),
+ mm_proxy_(std::make_unique<org::freedesktop::ModemManager1Proxy>(
+ bus, modemmanager::kModemManager1ServiceName)),
weak_factory_(this) {
auto on_interface_added = base::BindRepeating(
&ModemManagerProxy::OnInterfaceAdded, weak_factory_.GetWeakPtr());
@@ -26,13 +29,14 @@
LOG(ERROR) << "Failed to connect to signal " << interface << "."
<< signal;
});
- proxy_->RegisterInterfacesAddedSignalHandler(
+ object_manager_proxy_->RegisterInterfacesAddedSignalHandler(
std::move(on_interface_added), std::move(on_dbus_signal_connected));
}
ModemManagerProxy::ModemManagerProxy() : weak_factory_(this) {}
void ModemManagerProxy::RegisterModemAppearedCallback(base::OnceClosure cb) {
+ VLOG(2) << __func__;
on_modem_appeared_cb_ = std::move(cb);
}
@@ -41,7 +45,7 @@
auto on_service_available =
base::BindOnce(&ModemManagerProxy::WaitForModemStepGetObjects,
weak_factory_.GetWeakPtr(), std::move(cb));
- proxy_->GetObjectProxy()->WaitForServiceToBeAvailable(
+ object_manager_proxy_->GetObjectProxy()->WaitForServiceToBeAvailable(
std::move((on_service_available)));
}
@@ -51,7 +55,7 @@
LOG(ERROR) << "Could not get MM managed objects: " << err->GetDomain()
<< ": " << err->GetCode() << ": " << err->GetMessage();
});
- proxy_->GetManagedObjectsAsync(
+ object_manager_proxy_->GetManagedObjectsAsync(
base::BindOnce(&ModemManagerProxy::WaitForModemStepLast,
weak_factory_.GetWeakPtr(), std::move(cb)),
std::move(error_cb));
@@ -99,7 +103,7 @@
void ModemManagerProxy::OnPropertiesChanged(
org::freedesktop::ModemManager1::ModemProxyInterface* /*unused*/,
const std::string& prop) {
- VLOG(2) << __func__ << " : " << prop << " changed.";
+ VLOG(3) << __func__ << " : " << prop << " changed.";
if (!modem_proxy_->GetProperties()->primary_port.is_valid() ||
!modem_proxy_->GetProperties()->device_identifier.is_valid())
return;
@@ -111,4 +115,92 @@
return modem_proxy_->primary_port();
}
+void ModemManagerProxy::Uninhibit() {
+ uninhibit_cb_.Cancel();
+ if (inhibited_uid_.has_value()) {
+ InhibitDevice(false, base::BindOnce([](int err) {
+ VLOG(2) << "Uninhibit completed with err: " << err;
+ }));
+ }
+}
+
+void ModemManagerProxy::ScheduleUninhibit(base::TimeDelta timeout) {
+ LOG(INFO) << "Uninhibiting in " << timeout.InSeconds() << " seconds.";
+ uninhibit_cb_.Cancel();
+
+ uninhibit_cb_.Reset(base::BindOnce(&ModemManagerProxy::Uninhibit,
+ weak_factory_.GetWeakPtr()));
+ base::SequencedTaskRunnerHandle::Get()->PostDelayedTask(
+ FROM_HERE, uninhibit_cb_.callback(), timeout);
+}
+
+void ModemManagerProxy::WaitForModemAndInhibit(ResultCallback cb) {
+ // wait for modem if another daemon has inhibited MM. If Hermes has inhibited
+ // MM, then we needn't wait for the modem.
+ if (inhibited_uid_.has_value()) {
+ LOG(INFO) << inhibited_uid_.value() << " is already inhibited.";
+ OnInhibitSuccess(/*inhibit*/ true, inhibited_uid_.value(), std::move(cb));
+ return;
+ }
+ WaitForModem(base::BindOnce(&ModemManagerProxy::InhibitDevice,
+ weak_factory_.GetWeakPtr(), true, std::move(cb)));
+}
+
+void ModemManagerProxy::InhibitDevice(bool inhibit, ResultCallback cb) {
+ LOG(INFO) << __func__ << ": inhibit = " << inhibit;
+ if (!inhibit && !inhibited_uid_.has_value()) {
+ LOG(ERROR) << "No inhibited device found.";
+ std::move(cb).Run(kModemManagerError);
+ return;
+ }
+
+ if (inhibit && (!modem_proxy_ || modem_proxy_->device().empty())) {
+ LOG(ERROR) << __func__ << ": Device identifier unavailable.";
+ std::move(cb).Run(kModemManagerError);
+ return;
+ }
+
+ auto uid = inhibit ? modem_proxy_->device() : inhibited_uid_.value();
+
+ constexpr int kInhibitTimeoutMilliseconds = 1000;
+ // convert cb into a repeating callback, so that it can be used by either
+ // on_inhibit_success or on_inhibit_fail
+ auto return_inhibit_result = base::BindRepeating(
+ [](ResultCallback cb, int err) { std::move(cb).Run(err); },
+ base::Passed(std::move(cb)));
+
+ auto on_inhibit_success = base::BindOnce(&ModemManagerProxy::OnInhibitSuccess,
+ weak_factory_.GetWeakPtr(), inhibit,
+ uid, return_inhibit_result);
+ auto on_inhibit_fail = base::BindOnce(
+ [](ResultCallback cb, brillo::Error* error) {
+ LOG(ERROR) << error->GetMessage();
+ std::move(cb).Run(kModemManagerError);
+ },
+ return_inhibit_result);
+
+ mm_proxy_->InhibitDeviceAsync(uid, inhibit, std::move(on_inhibit_success),
+ std::move(on_inhibit_fail),
+ kInhibitTimeoutMilliseconds);
+}
+
+void ModemManagerProxy::OnInhibitSuccess(bool inhibit,
+ std::basic_string<char> uid,
+ ResultCallback cb) {
+ VLOG(2) << __func__;
+ inhibited_uid_ =
+ inhibit ? std::optional<std::basic_string<char>>{uid} : std::nullopt;
+
+ // uninhibit automatically if we exceed the max duration allowed for a
+ // Hermes operation.
+ uninhibit_cb_.Cancel();
+ if (inhibit) {
+ ScheduleUninhibit(kHermesTimeout);
+ std::move(cb).Run(kSuccess);
+ return;
+ }
+
+ std::move(cb).Run(kSuccess);
+}
+
} // namespace hermes
diff --git a/hermes/modem_manager_proxy.h b/hermes/modem_manager_proxy.h
index 1680308..bb12fb9 100644
--- a/hermes/modem_manager_proxy.h
+++ b/hermes/modem_manager_proxy.h
@@ -11,8 +11,10 @@
#include <chromeos/dbus/service_constants.h>
#include <dbus/bus.h>
+#include <base/cancelable_callback.h>
#include "hermes/dbus_bindings/mm-proxies.h"
+#include "hermes/executor.h"
#include "hermes/hermes_common.h"
namespace hermes {
@@ -33,6 +35,9 @@
std::string GetPrimaryPort() const;
+ void ScheduleUninhibit(base::TimeDelta timeout);
+ void WaitForModemAndInhibit(ResultCallback cb);
+
protected:
// To be used by mocks only
ModemManagerProxy();
@@ -49,11 +54,22 @@
org::freedesktop::ModemManager1::ModemProxyInterface* /*unused*/,
const std::string& prop);
+ void Uninhibit();
+ void InhibitDevice(bool inhibit, ResultCallback cb);
+ void OnInhibitSuccess(bool inhibit,
+ std::basic_string<char> uid,
+ ResultCallback cb);
+
scoped_refptr<dbus::Bus> bus_;
- std::unique_ptr<org::freedesktop::DBus::ObjectManagerProxy> proxy_;
+ std::unique_ptr<org::freedesktop::DBus::ObjectManagerProxy>
+ object_manager_proxy_;
+ std::unique_ptr<org::freedesktop::ModemManager1Proxy> mm_proxy_;
std::unique_ptr<org::freedesktop::ModemManager1::ModemProxy> modem_proxy_;
base::OnceClosure on_modem_appeared_cb_;
+ std::optional<std::basic_string<char>> inhibited_uid_;
+ base::CancelableOnceClosure uninhibit_cb_;
+
base::WeakPtrFactory<ModemManagerProxy> weak_factory_;
};
} // namespace hermes
diff --git a/hermes/modem_mbim.cc b/hermes/modem_mbim.cc
index 29e119e..5568ffd 100644
--- a/hermes/modem_mbim.cc
+++ b/hermes/modem_mbim.cc
@@ -332,6 +332,14 @@
TransmitFromQueue();
}
+void ModemMbim::CloseChannel(base::OnceCallback<void(int)> cb) {
+ tx_queue_.push_back(
+ {std::unique_ptr<TxInfo>(), AllocateId(),
+ std::make_unique<MbimCmd>(MbimCmd::MbimType::kMbimCloseLogicalChannel),
+ std::move(cb)});
+ TransmitFromQueue();
+}
+
void ModemMbim::AcquireChannel(base::OnceCallback<void(int)> cb) {
LOG(INFO) << __func__;
tx_queue_.push_back(
@@ -346,12 +354,10 @@
LOG(INFO) << __func__ << " with physical_slot: " << physical_slot;
auto acquire_channel =
base::BindOnce(&ModemMbim::AcquireChannel, weak_factory_.GetWeakPtr());
- tx_queue_.push_back(
- {std::unique_ptr<TxInfo>(), AllocateId(),
- std::make_unique<MbimCmd>(MbimCmd::MbimType::kMbimCloseLogicalChannel),
- base::BindOnce(&RunNextStep, std::move(acquire_channel),
- std::move(cb))});
- TransmitFromQueue();
+ // Close any existing channel, and acquire a new one. Run cb after a new
+ // channel has been acquired.
+ CloseChannel(
+ base::BindOnce(&RunNextStep, std::move(acquire_channel), std::move(cb)));
}
void ModemMbim::GetEidFromSim(ResultCallback cb) {
@@ -727,15 +733,33 @@
(GAsyncReadyCallback)MbimCreateNewDeviceCb, this);
}
-void ModemMbim::StartProfileOp(uint32_t physical_slot, ResultCallback cb) {
- LOG(INFO) << __func__ << " physical_slot:" << physical_slot;
- retry_count_ = 0;
- is_ready_state_valid = false;
- StoreAndSetActiveSlot(physical_slot, std::move(cb));
+void ModemMbim::ProcessEuiccEvent(EuiccEvent event, ResultCallback cb) {
+ LOG(INFO) << __func__ << ", " << event;
+ if (event.step == EuiccStep::START) {
+ auto store_and_set_active_slot =
+ base::BindOnce(&ModemMbim::StoreAndSetActiveSlot,
+ weak_factory_.GetWeakPtr(), event.slot);
+ modem_manager_proxy_->WaitForModemAndInhibit(base::BindOnce(
+ &RunNextStep, std::move(store_and_set_active_slot), std::move(cb)));
+ return;
+ }
+ if (event.step == EuiccStep::PENDING_NOTIFICATIONS) {
+ AcquireChannelAfterCardReady(event, std::move(cb));
+ return;
+ }
+ if (event.step == EuiccStep::END) {
+ auto close_device_and_uninhibit = base::BindOnce(
+ &ModemMbim::CloseDeviceAndUninhibit, weak_factory_.GetWeakPtr());
+ // Close channel, followed by close device and uninhibit, and then execute
+ // cb
+ CloseChannel(base::BindOnce(
+ &RunNextStep, std::move(close_device_and_uninhibit), std::move(cb)));
+ return;
+ }
}
-void ModemMbim::FinishProfileOp(ResultCallback cb) {
- LOG(INFO) << __func__;
+void ModemMbim::AcquireChannelAfterCardReady(EuiccEvent event,
+ ResultCallback cb) {
const guint MBIM_SUBSCRIBER_READY_STATE_NO_ESIM_PROFILE = 7;
if (!is_ready_state_valid ||
!(ready_state_ == MBIM_SUBSCRIBER_READY_STATE_NOT_INITIALIZED ||
@@ -751,18 +775,20 @@
retry_count_++;
executor_->PostDelayedTask(
FROM_HERE,
- base::BindOnce(&ModemMbim::FinishProfileOp, weak_factory_.GetWeakPtr(),
+ base::BindOnce(&ModemMbim::AcquireChannelAfterCardReady,
+ weak_factory_.GetWeakPtr(), std::move(event),
std::move(cb)),
kSimRefreshDelay);
return;
}
retry_count_ = 0;
- // Ideally we would acquire a channel to send notifications here. However,
- // acquiring a channel could cause MM to stop reporting the EID due to a fw
- // bug in L850. Thus we skip sending profile enable/disable notifications
- // until b/195589882, and b/202401139 are fixed.
+ AcquireChannel(std::move(cb));
+}
+
+void ModemMbim::CloseDeviceAndUninhibit(ResultCallback cb) {
CloseDevice();
- std::move(cb).Run(kModemMessageProcessingError);
+ modem_manager_proxy_->ScheduleUninhibit(kUninhibitDelay);
+ std::move(cb).Run(kModemSuccess);
}
void ModemMbim::RestoreActiveSlot(ResultCallback cb) {
diff --git a/hermes/modem_mbim.h b/hermes/modem_mbim.h
index 256520b..f8ea0ad 100644
--- a/hermes/modem_mbim.h
+++ b/hermes/modem_mbim.h
@@ -43,8 +43,7 @@
ResultCallback cb) override;
void StoreAndSetActiveSlot(const uint32_t physical_slot,
ResultCallback cb) override;
- void StartProfileOp(const uint32_t physical_slot, ResultCallback cb) override;
- void FinishProfileOp(ResultCallback cb) override;
+ void ProcessEuiccEvent(EuiccEvent event, ResultCallback cb) override;
void RestoreActiveSlot(ResultCallback cb) override;
bool IsSimValidAfterEnable() override;
bool IsSimValidAfterDisable() override;
@@ -74,6 +73,7 @@
void TransmitMbimSendEidApdu();
void TransmitMbimSendApdu(TxElement* tx_element);
void QueryCurrentMbimCapabilities(ResultCallback cb);
+ void CloseChannel(base::OnceCallback<void(int)> cb);
void AcquireChannel(base::OnceCallback<void(int)> cb);
void ReacquireChannel(const uint32_t physical_slot, ResultCallback cb);
void GetEidFromSim(ResultCallback cb);
@@ -112,6 +112,7 @@
void CloseDevice();
+ void CloseDeviceAndUninhibit(ResultCallback cb);
///////////////////
// State Diagram //
///////////////////
@@ -165,7 +166,11 @@
MbimSubscriberReadyState ready_state_;
GFile* file_ = NULL;
bool is_ready_state_valid;
+
+ base::CancelableOnceClosure scheduled_uninhibit_;
+
base::WeakPtrFactory<ModemMbim> weak_factory_;
+ void AcquireChannelAfterCardReady(EuiccEvent event, ResultCallback cb);
};
} // namespace hermes
diff --git a/hermes/modem_qrtr.cc b/hermes/modem_qrtr.cc
index 3956366..14527c3 100644
--- a/hermes/modem_qrtr.cc
+++ b/hermes/modem_qrtr.cc
@@ -883,21 +883,36 @@
TransmitFromQueue();
}
-void ModemQrtr::StartProfileOp(uint32_t physical_slot, ResultCallback cb) {
- LOG(INFO) << __func__ << " physical_slot:" << physical_slot;
- // The card triggers a refresh after profile enable. This refresh can cause
- // response apdu's with intermediate bytes to be flushed during a qmi
- // transaction. Since, we don't use these intermediate bytes, disable
- // them to avoid qmi errors as per QC's recommendation. b/169954635
- SetProcedureBytes(ProcedureBytesMode::DisableIntermediateBytes);
- StoreAndSetActiveSlot(physical_slot, std::move(cb));
-}
+void ModemQrtr::ProcessEuiccEvent(EuiccEvent event, ResultCallback cb) {
+ LOG(INFO) << __func__ << ", " << event;
+ if (event.step == EuiccStep::START) {
+ if (event.op == EuiccOp::DISABLE || event.op == EuiccOp::ENABLE) {
+ // The card triggers a refresh after profile enable. This refresh can
+ // cause response apdu's with intermediate bytes to be flushed during a
+ // qmi transaction. Since, we don't use these intermediate bytes, disable
+ // them to avoid qmi errors as per QC's recommendation. b/169954635
+ SetProcedureBytes(ProcedureBytesMode::DisableIntermediateBytes);
+ }
-void ModemQrtr::FinishProfileOp(ResultCallback cb) {
- LOG(INFO) << __func__;
- DisableQmi(kSimRefreshDelay);
- SetProcedureBytes(ProcedureBytesMode::EnableIntermediateBytes);
- AcquireChannel(std::move(cb));
+ auto store_and_set_active_slot =
+ base::BindOnce(&ModemQrtr::StoreAndSetActiveSlot,
+ weak_factory_.GetWeakPtr(), event.slot);
+ modem_manager_proxy_->WaitForModemAndInhibit(base::BindOnce(
+ &RunNextStep, std::move(store_and_set_active_slot), std::move(cb)));
+ return;
+ }
+ if (event.step == EuiccStep::PENDING_NOTIFICATIONS) {
+ SetProcedureBytes(ProcedureBytesMode::EnableIntermediateBytes);
+ DisableQmi(kSimRefreshDelay);
+ AcquireChannel(std::move(cb));
+ return;
+ }
+ if (event.step == EuiccStep::END) {
+ SetProcedureBytes(ProcedureBytesMode::EnableIntermediateBytes);
+ modem_manager_proxy_->ScheduleUninhibit(kUninhibitDelay);
+ std::move(cb).Run(kModemSuccess);
+ return;
+ }
}
void ModemQrtr::QrtrTable::Insert(QmiCmdInterface::Service service,
diff --git a/hermes/modem_qrtr.h b/hermes/modem_qrtr.h
index ff82967..06e820c 100644
--- a/hermes/modem_qrtr.h
+++ b/hermes/modem_qrtr.h
@@ -46,9 +46,7 @@
void StoreAndSetActiveSlot(uint32_t physical_slot,
ResultCallback cb) override;
void RestoreActiveSlot(ResultCallback cb) override;
- void StartProfileOp(uint32_t physical_slot, ResultCallback cb) override;
- void FinishProfileOp(ResultCallback cb) override;
-
+ void ProcessEuiccEvent(EuiccEvent event, ResultCallback cb) override;
private:
struct SwitchSlotTxInfo : public TxInfo {
diff --git a/hermes/profile.cc b/hermes/profile.cc
index 58b4eab..0d6b950 100644
--- a/hermes/profile.cc
+++ b/hermes/profile.cc
@@ -52,6 +52,15 @@
}
}
+void SendDBusError(std::shared_ptr<Profile::DBusResponse<>> response,
+ brillo::ErrorPtr lpa_error,
+ int modem_error) {
+ if (modem_error != kSuccess) {
+ LOG(ERROR) << "Modem finished with error code: " << modem_error;
+ }
+ response->ReplyWithError(lpa_error.get());
+}
+
template <typename T>
void RunOnSuccess(base::OnceCallback<void(T)> cb, T response, int err) {
if (err) {
@@ -146,8 +155,8 @@
LOG(INFO) << "Enabling profile: " << GetObjectPathForLog(object_path_);
auto enable_profile =
base::BindOnce(&Profile::EnableProfile, weak_factory_.GetWeakPtr());
- context_->modem_control()->StartProfileOp(
- physical_slot_,
+ context_->modem_control()->ProcessEuiccEvent(
+ {physical_slot_, EuiccStep::START, EuiccOp::ENABLE},
base::BindOnce(&RunOnSuccess<std::unique_ptr<DBusResponse<>>>,
std::move(enable_profile), std::move(response)));
}
@@ -185,8 +194,8 @@
LOG(INFO) << "Disabling profile: " << GetObjectPathForLog(object_path_);
auto disable_profile =
base::BindOnce(&Profile::DisableProfile, weak_factory_.GetWeakPtr());
- context_->modem_control()->StartProfileOp(
- physical_slot_,
+ context_->modem_control()->ProcessEuiccEvent(
+ {physical_slot_, EuiccStep::START, EuiccOp::DISABLE},
base::BindOnce(&RunOnSuccess<std::unique_ptr<DBusResponse<>>>,
std::move(disable_profile), std::move(response)));
}
@@ -210,7 +219,10 @@
if (decoded_error) {
LOG(ERROR) << "Failed enabling profile: " << object_path_.value()
<< " (error " << decoded_error << ")";
- response->ReplyWithError(decoded_error.get());
+ context_->modem_control()->ProcessEuiccEvent(
+ {physical_slot_, EuiccStep::END},
+ base::BindOnce(&SendDBusError, std::move(response),
+ std::move(decoded_error)));
return;
}
on_profile_enabled_cb_.Run(GetIccid());
@@ -218,7 +230,9 @@
auto send_notifs =
base::BindOnce(&Profile::FinishProfileOpCb, weak_factory_.GetWeakPtr(),
std::move(response));
- context_->modem_control()->FinishProfileOp(std::move(send_notifs));
+ context_->modem_control()->ProcessEuiccEvent(
+ {physical_slot_, EuiccStep::PENDING_NOTIFICATIONS},
+ std::move(send_notifs));
}
void Profile::OnDisabled(int error, std::shared_ptr<DBusResponse<>> response) {
@@ -227,7 +241,10 @@
if (decoded_error) {
LOG(ERROR) << "Failed disabling profile: " << object_path_.value()
<< " (error " << decoded_error << ")";
- response->ReplyWithError(decoded_error.get());
+ context_->modem_control()->ProcessEuiccEvent(
+ {physical_slot_, EuiccStep::END},
+ base::BindOnce(&SendDBusError, std::move(response),
+ std::move(decoded_error)));
return;
}
LOG(INFO) << "Disabled profile: " << object_path_.value();
@@ -236,11 +253,14 @@
auto send_notifs =
base::BindOnce(&Profile::FinishProfileOpCb, weak_factory_.GetWeakPtr(),
std::move(response));
- context_->modem_control()->FinishProfileOp(std::move(send_notifs));
+ context_->modem_control()->ProcessEuiccEvent(
+ {physical_slot_, EuiccStep::PENDING_NOTIFICATIONS},
+ std::move(send_notifs));
}
void Profile::FinishProfileOpCb(std::shared_ptr<DBusResponse<>> response,
int err) {
+ LOG(INFO) << __func__;
if (err) {
LOG(WARNING) << "Could not finish profile op: " << object_path_.value();
// Notifications are optional by the standard. Since FinishProfileOp failed,
@@ -251,7 +271,18 @@
}
context_->lpa()->SendNotifications(
context_->executor(),
- [response{std::move(response)}](int /*error*/) { response->Return(); });
+ [this, response{std::move(response)}](int /*error*/) {
+ VLOG(2) << "FinishProfileOpCb: sent all notifications";
+ context_->modem_control()->ProcessEuiccEvent(
+ {physical_slot_, EuiccStep::END},
+ base::BindOnce(
+ [](std::shared_ptr<DBusResponse<>> response, int error) {
+ response->Return();
+ LOG(INFO)
+ << "FinishProfileOpCb: completed with err = " << error;
+ },
+ response));
+ });
}
void Profile::Rename(std::unique_ptr<DBusResponse<>> response,
@@ -269,8 +300,8 @@
auto set_nickname =
base::BindOnce(&Profile::SetNicknameMethod, weak_factory_.GetWeakPtr(),
std::move(nickname));
- context_->modem_control()->StoreAndSetActiveSlot(
- physical_slot_,
+ context_->modem_control()->ProcessEuiccEvent(
+ {physical_slot_, EuiccStep::START},
base::BindOnce(&RunOnSuccess<std::unique_ptr<DBusResponse<>>>,
std::move(set_nickname), std::move(response)));
}
@@ -322,13 +353,22 @@
return;
}
this->SetNickname(nickname);
- auto report_success =
- base::BindOnce([](std::shared_ptr<DBusResponse<>> response) {
- response->Return();
- });
- context_->modem_control()->RestoreActiveSlot(
- base::BindOnce(&RunOnSuccess<std::shared_ptr<DBusResponse<>>>,
- std::move(report_success), std::move(response)));
+
+ auto restore_slot_and_dbus_return = base::BindOnce(
+ [](ModemControlInterface* modem_control,
+ std::shared_ptr<DBusResponse<>> response, int error) {
+ auto report_success =
+ base::BindOnce([](std::shared_ptr<DBusResponse<>> response) {
+ response->Return();
+ });
+ modem_control->RestoreActiveSlot(base::BindOnce(
+ &RunOnSuccess<std::shared_ptr<DBusResponse<>>>,
+ std::move(report_success), std::move(response)));
+ },
+ context_->modem_control(), response);
+ context_->modem_control()->ProcessEuiccEvent(
+ {physical_slot_, EuiccStep::END},
+ std::move(restore_slot_and_dbus_return));
});
}
