Fix the Thermal AIDL example and VTS test
The AIDL proxy objects can't be compared directly but their internal IBinder
Bug: b/264224315
Test: atest VtsHalThermalTargetTest
Change-Id: Icf6174a0ba79fa5efeaec7778d27c18d957cd88d
diff --git a/thermal/aidl/android/hardware/thermal/IThermal.aidl b/thermal/aidl/android/hardware/thermal/IThermal.aidl
index 7c23c17..c94edcd 100644
--- a/thermal/aidl/android/hardware/thermal/IThermal.aidl
+++ b/thermal/aidl/android/hardware/thermal/IThermal.aidl
@@ -36,9 +36,8 @@
* exist on boot. The method always returns and never removes from
* the list such cooling devices.
*
- * @throws ScopedAStatus Status of the operation. If status code is not
- * STATUS_OK, getMessage() must be populated with the human-readable
- * error message.
+ * @throws EX_ILLEGAL_STATE If the Thermal HAL is not initialized successfully. And the
+ * getMessage() must be populated with human-readable error message.
*/
CoolingDevice[] getCoolingDevices();
@@ -54,9 +53,8 @@
* exist on boot. The method always returns and never removes from
* the list such cooling devices.
*
- * @throws ScopedAStatus Status of the operation. If status code is not
- * STATUS_OK, the getMessage() must be populated with the human-readable
- * error message.
+ * @throws EX_ILLEGAL_STATE If the Thermal HAL is not initialized successfully. And the
+ * getMessage() must be populated with human-readable error message.
*/
CoolingDevice[] getCoolingDevicesWithType(in CoolingType type);
@@ -70,9 +68,8 @@
* they go offline, if these devices exist on boot. The method
* always returns and never removes such temperatures.
*
- * @throws ScopedAStatus Status of the operation. If status code is not
- * STATUS_OK, the getMessage() must be populated with the human-readable
- * error message.
+ * @throws EX_ILLEGAL_STATE If the Thermal HAL is not initialized successfully. And the
+ * getMessage() must be populated with human-readable error message.
*/
Temperature[] getTemperatures();
@@ -88,9 +85,8 @@
* they go offline, if these devices exist on boot. The method
* always returns and never removes such temperatures.
*
- * @throws ScopedAStatus Status of the operation. If status code is not
- * STATUS_OK, the getMessage() must be populated with the human-readable
- * error message.
+ * @throws EX_ILLEGAL_STATE If the Thermal HAL is not initialized successfully. And the
+ * getMessage() must be populated with human-readable error message.
*/
Temperature[] getTemperaturesWithType(in TemperatureType type);
@@ -110,9 +106,8 @@
* throttling status, use getTemperatures or registerThermalChangedCallback
* and listen to the callback.
*
- * @throws ScopedAStatus Status of the operation. If status code is not
- * STATUS_OK, the getMessage() must be populated with the human-readable
- * error message.
+ * @throws EX_ILLEGAL_STATE If the Thermal HAL is not initialized successfully. And the
+ * getMessage() must be populated with human-readable error message.
*/
TemperatureThreshold[] getTemperatureThresholds();
@@ -135,9 +130,8 @@
* throttling status, use getTemperatures or registerThermalChangedCallback
* and listen to the callback.
*
- * @throws ScopedAStatus Status of the operation. If status code is not
- * STATUS_OK, the getMessage() must be populated with the human-readable
- * error message.
+ * @throws EX_ILLEGAL_STATE If the Thermal HAL is not initialized successfully. And the
+ * getMessage() must be populated with human-readable error message.
*/
TemperatureThreshold[] getTemperatureThresholdsWithType(in TemperatureType type);
@@ -152,12 +146,10 @@
* thermal events. if nullptr callback is given, the status code will be
* STATUS_BAD_VALUE and the operation will fail.
*
- * @throws ScopedAStatus Status of the operation. If status code is not
- * STATUS_OK, the getMessage() must be populated with the human-readable
- * error message. If callback is given nullptr, the returned status code
- * will be STATUS_BAD_VALUE and the exception will be EX_ILLEGAL_ARGUMENT.
- * if callback is already registered, the returned status code will be
- * STATUS_INVALID_OPERATION, the exception will be EX_ILLEGAL_ARGUMENT.
+ * @throws EX_ILLEGAL_ARGUMENT If the callback is given nullptr or already registered. And the
+ * getMessage() must be populated with human-readable error message.
+ * @throws EX_ILLEGAL_STATE If the Thermal HAL is not initialized successfully. And the
+ * getMessage() must be populated with human-readable error message.
*/
void registerThermalChangedCallback(in IThermalChangedCallback callback);
@@ -174,12 +166,10 @@
* STATUS_BAD_VALUE and the operation will fail.
* @param type the type to be filtered.
*
- * @throws ScopedAStatus Status of the operation. If status code is not
- * STATUS_OK, the getMessage() must be populated with the human-readable
- * error message. If callback is given nullptr, the returned status code
- * will be STATUS_BAD_VALUE and the exception will be EX_ILLEGAL_ARGUMENT.
- * if callback is already registered, the returned status code will be
- * STATUS_INVALID_OPERATION, the exception will be EX_ILLEGAL_ARGUMENT.
+ * @throws EX_ILLEGAL_ARGUMENT If the callback is given nullptr or already registered. And the
+ * getMessage() must be populated with human-readable error message.
+ * @throws EX_ILLEGAL_STATE If the Thermal HAL is not initialized successfully. And the
+ * getMessage() must be populated with human-readable error message.
*/
void registerThermalChangedCallbackWithType(
in IThermalChangedCallback callback, in TemperatureType type);
@@ -192,12 +182,10 @@
* thermal events. if nullptr callback is given, the status code will be
* STATUS_BAD_VALUE and the operation will fail.
*
- * @throws ScopedAStatus Status of the operation. If status code is not
- * STATUS_OK, the getMessage() must be populated with the human-readable
- * error message. If callback is given nullptr, the returned status code
- * will be STATUS_BAD_VALUE and the exception will be EX_ILLEGAL_ARGUMENT.
- * if callback is not registered, the returned status code will be
- * STATUS_INVALID_OPERATION, the exception will be EX_ILLEGAL_ARGUMENT.
+ * @throws EX_ILLEGAL_ARGUMENT If the callback is given nullptr or not previously registered.
+ * And the getMessage() must be populated with human-readable error message.
+ * @throws EX_ILLEGAL_STATE If the Thermal HAL is not initialized successfully. And the
+ * getMessage() must be populated with human-readable error message.
*/
void unregisterThermalChangedCallback(in IThermalChangedCallback callback);
}
diff --git a/thermal/aidl/default/Thermal.cpp b/thermal/aidl/default/Thermal.cpp
index 5771e0e..f643d22 100644
--- a/thermal/aidl/default/Thermal.cpp
+++ b/thermal/aidl/default/Thermal.cpp
@@ -14,6 +14,8 @@
* limitations under the License.
*/
+#define LOG_TAG "thermal_service_example"
+
#include "Thermal.h"
#include <android-base/logging.h>
@@ -22,6 +24,18 @@
using ndk::ScopedAStatus;
+namespace {
+
+bool interfacesEqual(const std::shared_ptr<::ndk::ICInterface>& left,
+ const std::shared_ptr<::ndk::ICInterface>& right) {
+ if (left == nullptr || right == nullptr || !left->isRemote() || !right->isRemote()) {
+ return left == right;
+ }
+ return left->asBinder() == right->asBinder();
+}
+
+} // namespace
+
ScopedAStatus Thermal::getCoolingDevices(std::vector<CoolingDevice>* /* out_devices */) {
LOG(VERBOSE) << __func__;
return ScopedAStatus::ok();
@@ -61,12 +75,20 @@
const std::shared_ptr<IThermalChangedCallback>& in_callback) {
LOG(VERBOSE) << __func__ << " IThermalChangedCallback: " << in_callback;
if (in_callback == nullptr) {
- return ScopedAStatus::fromStatus(STATUS_BAD_VALUE);
+ return ndk::ScopedAStatus::fromExceptionCodeWithMessage(EX_ILLEGAL_ARGUMENT,
+ "Invalid nullptr callback");
}
- if (mCallbacks.find(in_callback) != mCallbacks.end()) {
- return ScopedAStatus::fromStatus(STATUS_INVALID_OPERATION);
+ {
+ std::lock_guard<std::mutex> _lock(thermal_callback_mutex_);
+ if (std::any_of(thermal_callbacks_.begin(), thermal_callbacks_.end(),
+ [&](const std::shared_ptr<IThermalChangedCallback>& c) {
+ return interfacesEqual(c, in_callback);
+ })) {
+ return ndk::ScopedAStatus::fromExceptionCodeWithMessage(EX_ILLEGAL_ARGUMENT,
+ "Callback already registered");
+ }
+ thermal_callbacks_.push_back(in_callback);
}
- mCallbacks.insert(in_callback);
return ScopedAStatus::ok();
}
@@ -75,26 +97,48 @@
LOG(VERBOSE) << __func__ << " IThermalChangedCallback: " << in_callback
<< ", TemperatureType: " << static_cast<int32_t>(in_type);
if (in_callback == nullptr) {
- return ScopedAStatus::fromStatus(STATUS_BAD_VALUE);
+ return ndk::ScopedAStatus::fromExceptionCodeWithMessage(EX_ILLEGAL_ARGUMENT,
+ "Invalid nullptr callback");
}
- if (mCallbacks.find(in_callback) != mCallbacks.end()) {
- return ScopedAStatus::fromStatus(STATUS_INVALID_OPERATION);
+ {
+ std::lock_guard<std::mutex> _lock(thermal_callback_mutex_);
+ if (std::any_of(thermal_callbacks_.begin(), thermal_callbacks_.end(),
+ [&](const std::shared_ptr<IThermalChangedCallback>& c) {
+ return interfacesEqual(c, in_callback);
+ })) {
+ return ndk::ScopedAStatus::fromExceptionCodeWithMessage(EX_ILLEGAL_ARGUMENT,
+ "Callback already registered");
+ }
+ thermal_callbacks_.push_back(in_callback);
}
- mCallbacks.insert(in_callback);
return ScopedAStatus::ok();
}
ScopedAStatus Thermal::unregisterThermalChangedCallback(
const std::shared_ptr<IThermalChangedCallback>& in_callback) {
LOG(VERBOSE) << __func__ << " IThermalChangedCallback: " << in_callback;
- bool found = false;
if (in_callback == nullptr) {
- return ScopedAStatus::fromStatus(STATUS_BAD_VALUE);
+ return ndk::ScopedAStatus::fromExceptionCodeWithMessage(EX_ILLEGAL_ARGUMENT,
+ "Invalid nullptr callback");
}
- if (mCallbacks.find(in_callback) == mCallbacks.end()) {
- return ScopedAStatus::fromStatus(STATUS_INVALID_OPERATION);
+ {
+ std::lock_guard<std::mutex> _lock(thermal_callback_mutex_);
+ bool removed = false;
+ thermal_callbacks_.erase(
+ std::remove_if(thermal_callbacks_.begin(), thermal_callbacks_.end(),
+ [&](const std::shared_ptr<IThermalChangedCallback>& c) {
+ if (interfacesEqual(c, in_callback)) {
+ removed = true;
+ return true;
+ }
+ return false;
+ }),
+ thermal_callbacks_.end());
+ if (!removed) {
+ return ndk::ScopedAStatus::fromExceptionCodeWithMessage(EX_ILLEGAL_ARGUMENT,
+ "Callback wasn't registered");
+ }
}
- mCallbacks.erase(in_callback);
return ScopedAStatus::ok();
}
diff --git a/thermal/aidl/default/Thermal.h b/thermal/aidl/default/Thermal.h
index 788af4a..8885e63 100644
--- a/thermal/aidl/default/Thermal.h
+++ b/thermal/aidl/default/Thermal.h
@@ -54,7 +54,8 @@
const std::shared_ptr<IThermalChangedCallback>& in_callback) override;
private:
- std::set<std::shared_ptr<IThermalChangedCallback>> mCallbacks;
+ std::mutex thermal_callback_mutex_;
+ std::vector<std::shared_ptr<IThermalChangedCallback>> thermal_callbacks_;
};
} // namespace example
diff --git a/thermal/aidl/default/main.cpp b/thermal/aidl/default/main.cpp
index 61d8ad0..9f4ddb8 100644
--- a/thermal/aidl/default/main.cpp
+++ b/thermal/aidl/default/main.cpp
@@ -14,6 +14,8 @@
* limitations under the License.
*/
+#define LOG_TAG "thermal_service_example"
+
#include "Thermal.h"
#include <android-base/logging.h>
diff --git a/thermal/aidl/vts/VtsHalThermalTargetTest.cpp b/thermal/aidl/vts/VtsHalThermalTargetTest.cpp
index b93250e..73c5dd2 100644
--- a/thermal/aidl/vts/VtsHalThermalTargetTest.cpp
+++ b/thermal/aidl/vts/VtsHalThermalTargetTest.cpp
@@ -95,21 +95,21 @@
mThermalCallback = ndk::SharedRefBase::make<ThermalCallback>();
ASSERT_NE(mThermalCallback, nullptr);
- auto ret = mThermal->registerThermalChangedCallback(mThermalCallback);
- ASSERT_TRUE(ret.isOk());
+ auto status = mThermal->registerThermalChangedCallback(mThermalCallback);
+ ASSERT_TRUE(status.isOk());
// Expect to fail if register again
- ret = mThermal->registerThermalChangedCallback(mThermalCallback);
- ASSERT_FALSE(ret.isOk());
- ASSERT_TRUE(ret.getStatus() == STATUS_INVALID_OPERATION);
+ status = mThermal->registerThermalChangedCallback(mThermalCallback);
+ ASSERT_FALSE(status.isOk());
+ ASSERT_EQ(EX_ILLEGAL_ARGUMENT, status.getExceptionCode());
}
void TearDown() override {
- auto ret = mThermal->unregisterThermalChangedCallback(mThermalCallback);
- ASSERT_TRUE(ret.isOk());
+ auto status = mThermal->unregisterThermalChangedCallback(mThermalCallback);
+ ASSERT_TRUE(status.isOk());
// Expect to fail if unregister again
- ret = mThermal->unregisterThermalChangedCallback(mThermalCallback);
- ASSERT_FALSE(ret.isOk());
- ASSERT_TRUE(ret.getStatus() == STATUS_INVALID_OPERATION);
+ status = mThermal->unregisterThermalChangedCallback(mThermalCallback);
+ ASSERT_FALSE(status.isOk());
+ ASSERT_EQ(EX_ILLEGAL_ARGUMENT, status.getExceptionCode());
}
protected: