Clean up connection to sensor hidl service
getService() would block until hidl service up and running. Replace
retry delay with service registration listener to speed up.
Moreover, when getService() return nullptr, it means sensor hidl
service does not exist. Remove retry in this case and return failure.
Bug: 66916774
Test: test device boot
Test: adb shell "stop; start"
Test: kill sensor hidl process
Test: mod manifest file to remove sensor entry
Change-Id: I685971425e97e314699de4630d9adf400aba4af2
diff --git a/services/sensorservice/Android.bp b/services/sensorservice/Android.bp
index 8d381b1..a7f3a52 100644
--- a/services/sensorservice/Android.bp
+++ b/services/sensorservice/Android.bp
@@ -14,6 +14,7 @@
"RecentEventLogger.cpp",
"RotationVectorSensor.cpp",
"SensorDevice.cpp",
+ "SensorDeviceUtils.cpp",
"SensorDirectConnection.cpp",
"SensorEventConnection.cpp",
"SensorFusion.cpp",
diff --git a/services/sensorservice/SensorDevice.cpp b/services/sensorservice/SensorDevice.cpp
index da3b275..fdb69d3 100644
--- a/services/sensorservice/SensorDevice.cpp
+++ b/services/sensorservice/SensorDevice.cpp
@@ -26,11 +26,10 @@
#include <cinttypes>
#include <thread>
-using android::hardware::hidl_vec;
-
using namespace android::hardware::sensors::V1_0;
using namespace android::hardware::sensors::V1_0::implementation;
-
+using android::hardware::hidl_vec;
+using android::SensorDeviceUtils::HidlServiceRegistrationWaiter;
namespace android {
// ---------------------------------------------------------------------------
@@ -52,7 +51,8 @@
}
}
-SensorDevice::SensorDevice() : mHidlTransportErrors(20) {
+SensorDevice::SensorDevice()
+ : mHidlTransportErrors(20), mRestartWaiter(new HidlServiceRegistrationWaiter()) {
if (!connectHidlService()) {
return;
}
@@ -87,35 +87,29 @@
}
bool SensorDevice::connectHidlService() {
- // SensorDevice may wait upto 100ms * 10 = 1s for hidl service.
- constexpr auto RETRY_DELAY = std::chrono::milliseconds(100);
+ // SensorDevice will wait for HAL service to start if HAL is declared in device manifest.
size_t retry = 10;
- while (true) {
- int initStep = 0;
+ while (retry-- > 0) {
mSensors = ISensors::getService();
- if (mSensors != nullptr) {
- ++initStep;
- // Poke ISensor service. If it has lingering connection from previous generation of
- // system server, it will kill itself. There is no intention to handle the poll result,
- // which will be done since the size is 0.
- if(mSensors->poll(0, [](auto, const auto &, const auto &) {}).isOk()) {
- // ok to continue
- break;
- }
- // hidl service is restarting, pointer is invalid.
- mSensors = nullptr;
- }
-
- if (--retry <= 0) {
- ALOGE("Cannot connect to ISensors hidl service!");
+ if (mSensors == nullptr) {
+ // no sensor hidl service found
break;
}
- // Delay 100ms before retry, hidl service is expected to come up in short time after
- // crash.
- ALOGI("%s unsuccessful, try again soon (remaining retry %zu).",
- (initStep == 0) ? "getService()" : "poll() check", retry);
- std::this_thread::sleep_for(RETRY_DELAY);
+
+ mRestartWaiter->reset();
+ // Poke ISensor service. If it has lingering connection from previous generation of
+ // system server, it will kill itself. There is no intention to handle the poll result,
+ // which will be done since the size is 0.
+ if(mSensors->poll(0, [](auto, const auto &, const auto &) {}).isOk()) {
+ // ok to continue
+ break;
+ }
+
+ // hidl service is restarting, pointer is invalid.
+ mSensors = nullptr;
+ ALOGI("%s unsuccessful, remaining retry %zu.", __FUNCTION__, retry);
+ mRestartWaiter->wait();
}
return (mSensors != nullptr);
}
@@ -173,7 +167,7 @@
}
status_t SensorDevice::initCheck() const {
- return mSensors != NULL ? NO_ERROR : NO_INIT;
+ return mSensors != nullptr ? NO_ERROR : NO_INIT;
}
ssize_t SensorDevice::poll(sensors_event_t* buffer, size_t count) {
diff --git a/services/sensorservice/SensorDevice.h b/services/sensorservice/SensorDevice.h
index fd6cee6..6d75051 100644
--- a/services/sensorservice/SensorDevice.h
+++ b/services/sensorservice/SensorDevice.h
@@ -17,6 +17,7 @@
#ifndef ANDROID_SENSOR_DEVICE_H
#define ANDROID_SENSOR_DEVICE_H
+#include "SensorDeviceUtils.h"
#include "SensorServiceUtils.h"
#include <sensor/Sensor.h>
@@ -39,12 +40,9 @@
namespace android {
// ---------------------------------------------------------------------------
-using SensorServiceUtil::Dumpable;
-using hardware::Return;
-class SensorDevice : public Singleton<SensorDevice>, public Dumpable {
+class SensorDevice : public Singleton<SensorDevice>, public SensorServiceUtil::Dumpable {
public:
-
class HidlTransportErrorLog {
public:
@@ -105,7 +103,7 @@
private:
friend class Singleton<SensorDevice>;
- sp<android::hardware::sensors::V1_0::ISensors> mSensors;
+ sp<hardware::sensors::V1_0::ISensors> mSensors;
Vector<sensor_t> mSensorList;
std::unordered_map<int32_t, sensor_t*> mConnectedDynamicSensors;
@@ -174,6 +172,8 @@
}
return std::move(ret);
}
+ //TODO(b/67425500): remove waiter after bug is resolved.
+ sp<SensorDeviceUtils::HidlServiceRegistrationWaiter> mRestartWaiter;
bool isClientDisabled(void* ident);
bool isClientDisabledLocked(void* ident);
diff --git a/services/sensorservice/SensorDeviceUtils.cpp b/services/sensorservice/SensorDeviceUtils.cpp
new file mode 100644
index 0000000..b1344be
--- /dev/null
+++ b/services/sensorservice/SensorDeviceUtils.cpp
@@ -0,0 +1,70 @@
+/*
+ * Copyright (C) 2017 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 "SensorDeviceUtils.h"
+
+#include <android/hardware/sensors/1.0/ISensors.h>
+#include <utils/Log.h>
+
+#include <chrono>
+#include <thread>
+
+using ::android::hardware::Void;
+using namespace android::hardware::sensors::V1_0;
+
+namespace android {
+namespace SensorDeviceUtils {
+
+HidlServiceRegistrationWaiter::HidlServiceRegistrationWaiter()
+ : mRegistered(ISensors::registerForNotifications("default", this)) {
+}
+
+Return<void> HidlServiceRegistrationWaiter::onRegistration(
+ const hidl_string &fqName, const hidl_string &name, bool preexisting) {
+ ALOGV("onRegistration fqName %s, name %s, preexisting %d",
+ fqName.c_str(), name.c_str(), preexisting);
+
+ {
+ std::lock_guard<std::mutex> lk(mLock);
+ mRestartObserved = true;
+ }
+ mCondition.notify_all();
+ return Void();
+}
+
+void HidlServiceRegistrationWaiter::reset() {
+ std::lock_guard<std::mutex> lk(mLock);
+ mRestartObserved = false;
+}
+
+bool HidlServiceRegistrationWaiter::wait() {
+ constexpr int DEFAULT_WAIT_MS = 100;
+ constexpr int TIMEOUT_MS = 1000;
+
+ if (!mRegistered) {
+ ALOGW("Cannot register service notification, use default wait(%d ms)", DEFAULT_WAIT_MS);
+ std::this_thread::sleep_for(std::chrono::milliseconds(DEFAULT_WAIT_MS));
+ // not sure if service is actually restarted
+ return false;
+ }
+
+ std::unique_lock<std::mutex> lk(mLock);
+ return mCondition.wait_for(lk, std::chrono::milliseconds(TIMEOUT_MS),
+ [this]{return mRestartObserved;});
+}
+
+} // namespace SensorDeviceUtils
+} // namespace android
diff --git a/services/sensorservice/SensorDeviceUtils.h b/services/sensorservice/SensorDeviceUtils.h
new file mode 100644
index 0000000..da36928
--- /dev/null
+++ b/services/sensorservice/SensorDeviceUtils.h
@@ -0,0 +1,60 @@
+/*
+ * Copyright (C) 2017 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.
+ */
+
+#ifndef ANDROID_SENSOR_DEVICE_UTIL
+#define ANDROID_SENSOR_DEVICE_UTIL
+
+#include <android/hidl/manager/1.0/IServiceNotification.h>
+
+#include <condition_variable>
+#include <thread>
+
+using ::android::hardware::hidl_string;
+using ::android::hardware::Return;
+using ::android::hidl::manager::V1_0::IServiceNotification;
+
+namespace android {
+namespace SensorDeviceUtils {
+
+class HidlServiceRegistrationWaiter : public IServiceNotification {
+public:
+
+ HidlServiceRegistrationWaiter();
+
+ Return<void> onRegistration(const hidl_string &fqName,
+ const hidl_string &name,
+ bool preexisting) override;
+
+ void reset();
+
+ /**
+ * Wait for service restart
+ *
+ * @return true if service is restart since last reset(); false otherwise.
+ */
+ bool wait();
+private:
+ const bool mRegistered;
+
+ std::mutex mLock;
+ std::condition_variable mCondition;
+ bool mRestartObserved;
+};
+
+} // namespace SensorDeviceUtils
+} // namespace android;
+
+#endif // ANDROID_SENSOR_SERVICE_UTIL