Set HAL response functions after framework sets all of theirs.
Bug: 207695009
Test: Build with ag/16322062 and a handful of patches from b/207695009,
try to use telephony stack
Change-Id: I74afe105a22a24efa0c38de20c75beffc58b144f
Merged-In: I74afe105a22a24efa0c38de20c75beffc58b144f
diff --git a/radio/aidl/compat/libradiocompat/Android.bp b/radio/aidl/compat/libradiocompat/Android.bp
index 9d568c4..487d91b 100644
--- a/radio/aidl/compat/libradiocompat/Android.bp
+++ b/radio/aidl/compat/libradiocompat/Android.bp
@@ -55,6 +55,7 @@
"libutils",
],
srcs: [
+ "CallbackManager.cpp",
"DriverContext.cpp",
"RadioCompatBase.cpp",
"RadioIndication.cpp",
diff --git a/radio/aidl/compat/libradiocompat/CallbackManager.cpp b/radio/aidl/compat/libradiocompat/CallbackManager.cpp
new file mode 100644
index 0000000..c2eaed1
--- /dev/null
+++ b/radio/aidl/compat/libradiocompat/CallbackManager.cpp
@@ -0,0 +1,84 @@
+/*
+ * Copyright (C) 2021 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include <libradiocompat/CallbackManager.h>
+
+#include <android-base/logging.h>
+
+using namespace std::literals::chrono_literals;
+
+namespace android::hardware::radio::compat {
+
+/**
+ * How much setter thread will wait with setting response functions after the last
+ * setResponseFunctions call from the framework. Subsequent calls from the framework reset the
+ * clock, so this number should be larger than the longest time between setResponseFunctions calls
+ * from the framework.
+ *
+ * Real world measurements with Cuttlefish give <10ms delay between Modem and Data and <2ms delays
+ * between all others.
+ */
+static constexpr auto kDelayedSetterDelay = 100ms;
+
+CallbackManager::CallbackManager(std::shared_ptr<DriverContext> context, sp<V1_5::IRadio> hidlHal)
+ : mHidlHal(hidlHal),
+ mRadioResponse(sp<compat::RadioResponse>::make(context)),
+ mRadioIndication(sp<compat::RadioIndication>::make(context)),
+ mDelayedSetterThread(&CallbackManager::delayedSetterThread, this) {}
+
+CallbackManager::~CallbackManager() {
+ {
+ std::unique_lock<std::mutex> lock(mDelayedSetterGuard);
+ mDelayedSetterDeadline = std::nullopt;
+ mDestroy = true;
+ mDelayedSetterCv.notify_all();
+ }
+ mDelayedSetterThread.join();
+}
+
+RadioResponse& CallbackManager::response() const {
+ return *mRadioResponse;
+}
+
+void CallbackManager::setResponseFunctionsDelayed() {
+ std::unique_lock<std::mutex> lock(mDelayedSetterGuard);
+ mDelayedSetterDeadline = std::chrono::steady_clock::now() + kDelayedSetterDelay;
+ mDelayedSetterCv.notify_all();
+}
+
+void CallbackManager::delayedSetterThread() {
+ while (!mDestroy) {
+ std::unique_lock<std::mutex> lock(mDelayedSetterGuard);
+ auto deadline = mDelayedSetterDeadline;
+
+ // not waiting to set response functions
+ if (!deadline) {
+ mDelayedSetterCv.wait(lock);
+ continue;
+ }
+
+ // waiting to set response functions, but not yet
+ if (*deadline > std::chrono::steady_clock::now()) {
+ mDelayedSetterCv.wait_until(lock, *deadline);
+ continue;
+ }
+
+ mHidlHal->setResponseFunctions(mRadioResponse, mRadioIndication).assertOk();
+ mDelayedSetterDeadline = std::nullopt;
+ }
+}
+
+} // namespace android::hardware::radio::compat
diff --git a/radio/aidl/compat/libradiocompat/RadioCompatBase.cpp b/radio/aidl/compat/libradiocompat/RadioCompatBase.cpp
index 2364484..2a2d7a3 100644
--- a/radio/aidl/compat/libradiocompat/RadioCompatBase.cpp
+++ b/radio/aidl/compat/libradiocompat/RadioCompatBase.cpp
@@ -21,11 +21,10 @@
namespace android::hardware::radio::compat {
RadioCompatBase::RadioCompatBase(std::shared_ptr<DriverContext> context, sp<V1_5::IRadio> hidlHal,
- sp<RadioResponse> radioResponse, sp<RadioIndication> radioInd)
+ std::shared_ptr<CallbackManager> cbMgr)
: mContext(context),
mHal1_5(hidlHal),
mHal1_6(V1_6::IRadio::castFrom(hidlHal)),
- mRadioResponse(radioResponse),
- mRadioIndication(radioInd) {}
+ mCallbackManager(cbMgr) {}
} // namespace android::hardware::radio::compat
diff --git a/radio/aidl/compat/libradiocompat/config/RadioConfig.cpp b/radio/aidl/compat/libradiocompat/config/RadioConfig.cpp
index d6399bf..5b22dbe 100644
--- a/radio/aidl/compat/libradiocompat/config/RadioConfig.cpp
+++ b/radio/aidl/compat/libradiocompat/config/RadioConfig.cpp
@@ -88,7 +88,7 @@
mRadioConfigResponse->setResponseFunction(radioConfigResponse);
mRadioConfigIndication->setResponseFunction(radioConfigIndication);
- mHal1_1->setResponseFunctions(mRadioConfigResponse, mRadioConfigIndication);
+ mHal1_1->setResponseFunctions(mRadioConfigResponse, mRadioConfigIndication).assertOk();
return ok();
}
diff --git a/radio/aidl/compat/libradiocompat/data/RadioData.cpp b/radio/aidl/compat/libradiocompat/data/RadioData.cpp
index 91529c9..d2f3687 100644
--- a/radio/aidl/compat/libradiocompat/data/RadioData.cpp
+++ b/radio/aidl/compat/libradiocompat/data/RadioData.cpp
@@ -32,7 +32,7 @@
constexpr auto ok = &ScopedAStatus::ok;
std::shared_ptr<aidl::IRadioDataResponse> RadioData::respond() {
- return mRadioResponse->dataCb();
+ return mCallbackManager->response().dataCb();
}
ScopedAStatus RadioData::allocatePduSessionId(int32_t serial) {
@@ -129,16 +129,10 @@
}
ScopedAStatus RadioData::setResponseFunctions(
- const std::shared_ptr<aidl::IRadioDataResponse>& dataResponse,
- const std::shared_ptr<aidl::IRadioDataIndication>& dataIndication) {
- LOG_CALL << dataResponse << ' ' << dataIndication;
-
- CHECK(dataResponse);
- CHECK(dataIndication);
-
- mRadioResponse->setResponseFunction(dataResponse);
- mRadioIndication->setResponseFunction(dataIndication);
-
+ const std::shared_ptr<aidl::IRadioDataResponse>& response,
+ const std::shared_ptr<aidl::IRadioDataIndication>& indication) {
+ LOG_CALL << response << ' ' << indication;
+ mCallbackManager->setResponseFunctions(response, indication);
return ok();
}
diff --git a/radio/aidl/compat/libradiocompat/include/libradiocompat/CallbackManager.h b/radio/aidl/compat/libradiocompat/include/libradiocompat/CallbackManager.h
new file mode 100644
index 0000000..f1a7b49
--- /dev/null
+++ b/radio/aidl/compat/libradiocompat/include/libradiocompat/CallbackManager.h
@@ -0,0 +1,62 @@
+/*
+ * Copyright (C) 2021 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+#pragma once
+
+#include "DriverContext.h"
+#include "RadioIndication.h"
+#include "RadioResponse.h"
+
+#include <android-base/logging.h>
+#include <android/hardware/radio/1.6/IRadio.h>
+
+#include <thread>
+
+namespace android::hardware::radio::compat {
+
+class CallbackManager {
+ sp<V1_5::IRadio> mHidlHal;
+ sp<RadioResponse> mRadioResponse;
+ sp<RadioIndication> mRadioIndication;
+
+ std::thread mDelayedSetterThread;
+ std::mutex mDelayedSetterGuard;
+ std::optional<std::chrono::time_point<std::chrono::steady_clock>> mDelayedSetterDeadline
+ GUARDED_BY(mDelayedSetterGuard);
+ std::condition_variable mDelayedSetterCv GUARDED_BY(mDelayedSetterGuard);
+ bool mDestroy GUARDED_BY(mDelayedSetterGuard) = false;
+
+ void setResponseFunctionsDelayed();
+ void delayedSetterThread();
+
+ public:
+ CallbackManager(std::shared_ptr<DriverContext> context, sp<V1_5::IRadio> hidlHal);
+ ~CallbackManager();
+
+ RadioResponse& response() const;
+
+ template <typename ResponseType, typename IndicationType>
+ void setResponseFunctions(const std::shared_ptr<ResponseType>& response,
+ const std::shared_ptr<IndicationType>& indication) {
+ CHECK(response);
+ CHECK(indication);
+
+ mRadioResponse->setResponseFunction(response);
+ mRadioIndication->setResponseFunction(indication);
+ setResponseFunctionsDelayed();
+ }
+};
+
+} // namespace android::hardware::radio::compat
diff --git a/radio/aidl/compat/libradiocompat/include/libradiocompat/RadioCompatBase.h b/radio/aidl/compat/libradiocompat/include/libradiocompat/RadioCompatBase.h
index cd17541..eb22fff 100644
--- a/radio/aidl/compat/libradiocompat/include/libradiocompat/RadioCompatBase.h
+++ b/radio/aidl/compat/libradiocompat/include/libradiocompat/RadioCompatBase.h
@@ -15,9 +15,8 @@
*/
#pragma once
+#include "CallbackManager.h"
#include "DriverContext.h"
-#include "RadioIndication.h"
-#include "RadioResponse.h"
#include <android/hardware/radio/1.6/IRadio.h>
@@ -30,12 +29,11 @@
sp<V1_5::IRadio> mHal1_5;
sp<V1_6::IRadio> mHal1_6;
- sp<RadioResponse> mRadioResponse;
- sp<RadioIndication> mRadioIndication;
+ std::shared_ptr<CallbackManager> mCallbackManager;
public:
RadioCompatBase(std::shared_ptr<DriverContext> context, sp<V1_5::IRadio> hidlHal,
- sp<RadioResponse> radioResponse, sp<RadioIndication> radioIndication);
+ std::shared_ptr<CallbackManager> cbMgr);
};
} // namespace android::hardware::radio::compat
diff --git a/radio/aidl/compat/libradiocompat/messaging/RadioMessaging.cpp b/radio/aidl/compat/libradiocompat/messaging/RadioMessaging.cpp
index 650d191..4d94e17 100644
--- a/radio/aidl/compat/libradiocompat/messaging/RadioMessaging.cpp
+++ b/radio/aidl/compat/libradiocompat/messaging/RadioMessaging.cpp
@@ -30,7 +30,7 @@
constexpr auto ok = &ScopedAStatus::ok;
std::shared_ptr<aidl::IRadioMessagingResponse> RadioMessaging::respond() {
- return mRadioResponse->messagingCb();
+ return mCallbackManager->response().messagingCb();
}
ScopedAStatus RadioMessaging::acknowledgeIncomingGsmSmsWithPdu( //
@@ -181,16 +181,10 @@
}
ScopedAStatus RadioMessaging::setResponseFunctions(
- const std::shared_ptr<aidl::IRadioMessagingResponse>& messagingResponse,
- const std::shared_ptr<aidl::IRadioMessagingIndication>& messagingIndication) {
- LOG_CALL << messagingResponse << ' ' << messagingIndication;
-
- CHECK(messagingResponse);
- CHECK(messagingIndication);
-
- mRadioResponse->setResponseFunction(messagingResponse);
- mRadioIndication->setResponseFunction(messagingIndication);
-
+ const std::shared_ptr<aidl::IRadioMessagingResponse>& response,
+ const std::shared_ptr<aidl::IRadioMessagingIndication>& indication) {
+ LOG_CALL << response << ' ' << indication;
+ mCallbackManager->setResponseFunctions(response, indication);
return ok();
}
diff --git a/radio/aidl/compat/libradiocompat/modem/RadioModem.cpp b/radio/aidl/compat/libradiocompat/modem/RadioModem.cpp
index 3fff5ee..d28b940 100644
--- a/radio/aidl/compat/libradiocompat/modem/RadioModem.cpp
+++ b/radio/aidl/compat/libradiocompat/modem/RadioModem.cpp
@@ -28,7 +28,7 @@
constexpr auto ok = &ScopedAStatus::ok;
std::shared_ptr<aidl::IRadioModemResponse> RadioModem::respond() {
- return mRadioResponse->modemCb();
+ return mCallbackManager->response().modemCb();
}
ScopedAStatus RadioModem::enableModem(int32_t serial, bool on) {
@@ -133,16 +133,10 @@
}
ScopedAStatus RadioModem::setResponseFunctions(
- const std::shared_ptr<aidl::IRadioModemResponse>& modemResponse,
- const std::shared_ptr<aidl::IRadioModemIndication>& modemIndication) {
- LOG_CALL << modemResponse << ' ' << modemIndication;
-
- CHECK(modemResponse);
- CHECK(modemIndication);
-
- mRadioResponse->setResponseFunction(modemResponse);
- mRadioIndication->setResponseFunction(modemIndication);
-
+ const std::shared_ptr<aidl::IRadioModemResponse>& response,
+ const std::shared_ptr<aidl::IRadioModemIndication>& indication) {
+ LOG_CALL << response << ' ' << indication;
+ mCallbackManager->setResponseFunctions(response, indication);
return ok();
}
diff --git a/radio/aidl/compat/libradiocompat/network/RadioNetwork.cpp b/radio/aidl/compat/libradiocompat/network/RadioNetwork.cpp
index 34e889d..c94be63 100644
--- a/radio/aidl/compat/libradiocompat/network/RadioNetwork.cpp
+++ b/radio/aidl/compat/libradiocompat/network/RadioNetwork.cpp
@@ -34,7 +34,7 @@
constexpr auto ok = &ScopedAStatus::ok;
std::shared_ptr<aidl::IRadioNetworkResponse> RadioNetwork::respond() {
- return mRadioResponse->networkCb();
+ return mCallbackManager->response().networkCb();
}
ScopedAStatus RadioNetwork::getAllowedNetworkTypesBitmap(int32_t serial) {
@@ -245,16 +245,10 @@
}
ScopedAStatus RadioNetwork::setResponseFunctions(
- const std::shared_ptr<aidl::IRadioNetworkResponse>& networkResponse,
- const std::shared_ptr<aidl::IRadioNetworkIndication>& networkIndication) {
- LOG_CALL << networkResponse << ' ' << networkIndication;
-
- CHECK(networkResponse);
- CHECK(networkIndication);
-
- mRadioResponse->setResponseFunction(networkResponse);
- mRadioIndication->setResponseFunction(networkIndication);
-
+ const std::shared_ptr<aidl::IRadioNetworkResponse>& response,
+ const std::shared_ptr<aidl::IRadioNetworkIndication>& indication) {
+ LOG_CALL << response << ' ' << indication;
+ mCallbackManager->setResponseFunctions(response, indication);
return ok();
}
diff --git a/radio/aidl/compat/libradiocompat/sim/RadioSim.cpp b/radio/aidl/compat/libradiocompat/sim/RadioSim.cpp
index e72ac7e..b43f64f 100644
--- a/radio/aidl/compat/libradiocompat/sim/RadioSim.cpp
+++ b/radio/aidl/compat/libradiocompat/sim/RadioSim.cpp
@@ -31,7 +31,7 @@
constexpr auto ok = &ScopedAStatus::ok;
std::shared_ptr<aidl::IRadioSimResponse> RadioSim::respond() {
- return mRadioResponse->simCb();
+ return mCallbackManager->response().simCb();
}
ScopedAStatus RadioSim::areUiccApplicationsEnabled(int32_t serial) {
@@ -221,16 +221,10 @@
}
ScopedAStatus RadioSim::setResponseFunctions(
- const std::shared_ptr<aidl::IRadioSimResponse>& simResponse,
- const std::shared_ptr<aidl::IRadioSimIndication>& simIndication) {
- LOG_CALL << simResponse << ' ' << simIndication;
-
- CHECK(simResponse);
- CHECK(simIndication);
-
- mRadioResponse->setResponseFunction(simResponse);
- mRadioIndication->setResponseFunction(simIndication);
-
+ const std::shared_ptr<aidl::IRadioSimResponse>& response,
+ const std::shared_ptr<aidl::IRadioSimIndication>& indication) {
+ LOG_CALL << response << ' ' << indication;
+ mCallbackManager->setResponseFunctions(response, indication);
return ok();
}
diff --git a/radio/aidl/compat/libradiocompat/voice/RadioVoice.cpp b/radio/aidl/compat/libradiocompat/voice/RadioVoice.cpp
index 742fdbe..7b1d1fa 100644
--- a/radio/aidl/compat/libradiocompat/voice/RadioVoice.cpp
+++ b/radio/aidl/compat/libradiocompat/voice/RadioVoice.cpp
@@ -31,7 +31,7 @@
constexpr auto ok = &ScopedAStatus::ok;
std::shared_ptr<aidl::IRadioVoiceResponse> RadioVoice::respond() {
- return mRadioResponse->voiceCb();
+ return mCallbackManager->response().voiceCb();
}
ScopedAStatus RadioVoice::acceptCall(int32_t serial) {
@@ -238,16 +238,10 @@
}
ScopedAStatus RadioVoice::setResponseFunctions(
- const std::shared_ptr<aidl::IRadioVoiceResponse>& voiceResponse,
- const std::shared_ptr<aidl::IRadioVoiceIndication>& voiceIndication) {
- LOG_CALL << voiceResponse << ' ' << voiceIndication;
-
- CHECK(voiceResponse);
- CHECK(voiceIndication);
-
- mRadioResponse->setResponseFunction(voiceResponse);
- mRadioIndication->setResponseFunction(voiceIndication);
-
+ const std::shared_ptr<aidl::IRadioVoiceResponse>& response,
+ const std::shared_ptr<aidl::IRadioVoiceIndication>& indication) {
+ LOG_CALL << response << ' ' << indication;
+ mCallbackManager->setResponseFunctions(response, indication);
return ok();
}
diff --git a/radio/aidl/compat/service/service.cpp b/radio/aidl/compat/service/service.cpp
index 6bcd4b0..7433fee 100644
--- a/radio/aidl/compat/service/service.cpp
+++ b/radio/aidl/compat/service/service.cpp
@@ -19,13 +19,12 @@
#include <android-base/logging.h>
#include <android/binder_manager.h>
#include <android/binder_process.h>
+#include <libradiocompat/CallbackManager.h>
#include <libradiocompat/RadioConfig.h>
#include <libradiocompat/RadioData.h>
-#include <libradiocompat/RadioIndication.h>
#include <libradiocompat/RadioMessaging.h>
#include <libradiocompat/RadioModem.h>
#include <libradiocompat/RadioNetwork.h>
-#include <libradiocompat/RadioResponse.h>
#include <libradiocompat/RadioSim.h>
#include <libradiocompat/RadioVoice.h>
@@ -36,9 +35,8 @@
static std::vector<std::shared_ptr<ndk::ICInterface>> gPublishedHals;
template <typename T>
-static void publishRadioHal(std::shared_ptr<compat::DriverContext> context,
- sp<V1_5::IRadio> hidlHal, sp<compat::RadioResponse> responseCb,
- sp<compat::RadioIndication> indicationCb, const std::string& slot) {
+static void publishRadioHal(std::shared_ptr<compat::DriverContext> ctx, sp<V1_5::IRadio> hidlHal,
+ std::shared_ptr<compat::CallbackManager> cm, const std::string& slot) {
const auto instance = T::descriptor + "/"s + slot;
if (!AServiceManager_isDeclared(instance.c_str())) {
LOG(INFO) << instance << " is not declared in VINTF (this may be intentional)";
@@ -46,7 +44,7 @@
}
LOG(DEBUG) << "Publishing " << instance;
- auto aidlHal = ndk::SharedRefBase::make<T>(context, hidlHal, responseCb, indicationCb);
+ auto aidlHal = ndk::SharedRefBase::make<T>(ctx, hidlHal, cm);
gPublishedHals.push_back(aidlHal);
const auto status = AServiceManager_addService(aidlHal->asBinder().get(), instance.c_str());
CHECK_EQ(status, STATUS_OK);
@@ -59,17 +57,14 @@
hidl_utils::linkDeathToDeath(radioHidl);
auto context = std::make_shared<compat::DriverContext>();
+ auto callbackMgr = std::make_shared<compat::CallbackManager>(context, radioHidl);
- auto responseCb = sp<compat::RadioResponse>::make(context);
- auto indicationCb = sp<compat::RadioIndication>::make(context);
- radioHidl->setResponseFunctions(responseCb, indicationCb).assertOk();
-
- publishRadioHal<compat::RadioData>(context, radioHidl, responseCb, indicationCb, slot);
- publishRadioHal<compat::RadioMessaging>(context, radioHidl, responseCb, indicationCb, slot);
- publishRadioHal<compat::RadioModem>(context, radioHidl, responseCb, indicationCb, slot);
- publishRadioHal<compat::RadioNetwork>(context, radioHidl, responseCb, indicationCb, slot);
- publishRadioHal<compat::RadioSim>(context, radioHidl, responseCb, indicationCb, slot);
- publishRadioHal<compat::RadioVoice>(context, radioHidl, responseCb, indicationCb, slot);
+ publishRadioHal<compat::RadioData>(context, radioHidl, callbackMgr, slot);
+ publishRadioHal<compat::RadioMessaging>(context, radioHidl, callbackMgr, slot);
+ publishRadioHal<compat::RadioModem>(context, radioHidl, callbackMgr, slot);
+ publishRadioHal<compat::RadioNetwork>(context, radioHidl, callbackMgr, slot);
+ publishRadioHal<compat::RadioSim>(context, radioHidl, callbackMgr, slot);
+ publishRadioHal<compat::RadioVoice>(context, radioHidl, callbackMgr, slot);
}
static void publishRadioConfig() {