Improve the comments of Thermal HAL 2.0 API
Make sure (un)register API failed with nullptr
Test: Build
Test: VtsHalThermalV2_0TargetTest
Change-Id: I42a87fde85a38f77faa4d07f29ed3a78501a0dca
diff --git a/thermal/2.0/IThermal.hal b/thermal/2.0/IThermal.hal
index f890694..3ea4590 100644
--- a/thermal/2.0/IThermal.hal
+++ b/thermal/2.0/IThermal.hal
@@ -56,22 +56,24 @@
* they go offline, if these devices exist on boot. The method
* always returns and never removes such temperatures. The thresholds
* are returned as static values and must not change across calls. The actual
- * throttling state is determined in driver and HAL and must not be simply
- * compared with these thresholds. To get accurate throttling status, use
- * getCurrentTemperatures or registerThermalChangedCallback and listen.
+ * throttling state is determined in device thermal mitigation policy/agorithm
+ * which might not be simple thresholds so these values Thermal HAL provided
+ * may not be accurate to detemin the throttling status. To get accurate
+ * throttling status, use getCurrentTemperatures or registerThermalChangedCallback
+ * and listen to the callback.
*/
getTemperatureThresholds(bool filterType, TemperatureType type)
generates (ThermalStatus status, vec<TemperatureThreshold> temperatureThresholds);
/**
* Register an IThermalChangedCallback, used by the Thermal HAL
- * to send thermal events when thermal mitigation status changed.
+ * to receive thermal events when thermal mitigation status changed.
* Multiple registrations with different IThermalChangedCallback must be allowed.
* Multiple registrations with same IThermalChangedCallback is not allowed, client
* should unregister the given IThermalChangedCallback first.
*
- * @param callback the IThermalChangedCallback to use for sending
- * thermal events (cannot be nullptr).
+ * @param callback the IThermalChangedCallback to use for receiving
+ * thermal events (nullptr callback will lead to failure with status code FAILURE).
* @param filterType if filter for given sensor type.
* @param type the type to be filtered.
*
@@ -84,11 +86,11 @@
generates (ThermalStatus status);
/**
- * Register an IThermalChangedCallback, used by the Thermal HAL
- * to send thermal events when thermal mitigation status changed.
+ * Unregister an IThermalChangedCallback, used by the Thermal HAL
+ * to receive thermal events when thermal mitigation status changed.
*
- * @param callback the IThermalChangedCallback to use for sending
- * thermal events, or nullptr to set no callback.
+ * @param callback the IThermalChangedCallback used for receiving
+ * thermal events (nullptr callback will lead to failure with status code FAILURE).
*
* @return status Status of the operation. If status code is FAILURE,
* the status.debugMessage must be populated with a human-readable error message.
diff --git a/thermal/2.0/default/Thermal.cpp b/thermal/2.0/default/Thermal.cpp
index 442af61..0ef4b63 100644
--- a/thermal/2.0/default/Thermal.cpp
+++ b/thermal/2.0/default/Thermal.cpp
@@ -155,7 +155,15 @@
bool filterType, TemperatureType type,
registerThermalChangedCallback_cb _hidl_cb) {
ThermalStatus status;
- status.code = ThermalStatusCode::SUCCESS;
+ if (callback == nullptr) {
+ status.code = ThermalStatusCode::FAILURE;
+ status.debugMessage = "Invalid nullptr callback";
+ LOG(ERROR) << status.debugMessage;
+ _hidl_cb(status);
+ return Void();
+ } else {
+ status.code = ThermalStatusCode::SUCCESS;
+ }
std::lock_guard<std::mutex> _lock(thermal_callback_mutex_);
if (std::any_of(callbacks_.begin(), callbacks_.end(), [&](const CallbackSetting& c) {
return interfacesEqual(c.callback, callback);
@@ -175,7 +183,15 @@
Return<void> Thermal::unregisterThermalChangedCallback(
const sp<IThermalChangedCallback>& callback, unregisterThermalChangedCallback_cb _hidl_cb) {
ThermalStatus status;
- status.code = ThermalStatusCode::SUCCESS;
+ if (callback == nullptr) {
+ status.code = ThermalStatusCode::FAILURE;
+ status.debugMessage = "Invalid nullptr callback";
+ LOG(ERROR) << status.debugMessage;
+ _hidl_cb(status);
+ return Void();
+ } else {
+ status.code = ThermalStatusCode::SUCCESS;
+ }
bool removed = false;
std::lock_guard<std::mutex> _lock(thermal_callback_mutex_);
callbacks_.erase(
diff --git a/thermal/2.0/vts/functional/VtsHalThermalV2_0TargetTest.cpp b/thermal/2.0/vts/functional/VtsHalThermalV2_0TargetTest.cpp
index 3893014..cc2f3df 100644
--- a/thermal/2.0/vts/functional/VtsHalThermalV2_0TargetTest.cpp
+++ b/thermal/2.0/vts/functional/VtsHalThermalV2_0TargetTest.cpp
@@ -132,8 +132,13 @@
TEST_F(ThermalHidlTest, RegisterThermalChangedCallbackTest) {
// Expect to fail with same callback
auto ret = mThermal->registerThermalChangedCallback(
- mThermalCallback, false, TemperatureType::SKIN,
- [](ThermalStatus status) { EXPECT_NE(ThermalStatusCode::SUCCESS, status.code); });
+ mThermalCallback, false, TemperatureType::SKIN,
+ [](ThermalStatus status) { EXPECT_EQ(ThermalStatusCode::FAILURE, status.code); });
+ ASSERT_TRUE(ret.isOk());
+ // Expect to fail with null callback
+ ret = mThermal->registerThermalChangedCallback(
+ nullptr, false, TemperatureType::SKIN,
+ [](ThermalStatus status) { EXPECT_EQ(ThermalStatusCode::FAILURE, status.code); });
ASSERT_TRUE(ret.isOk());
sp<ThermalCallback> localThermalCallback = new (std::nothrow) ThermalCallback();
// Expect to succeed with different callback
@@ -141,11 +146,16 @@
localThermalCallback, false, TemperatureType::SKIN,
[](ThermalStatus status) { EXPECT_EQ(ThermalStatusCode::SUCCESS, status.code); });
ASSERT_TRUE(ret.isOk());
- // Remove the local callback.
+ // Remove the local callback
ret = mThermal->unregisterThermalChangedCallback(
localThermalCallback,
[](ThermalStatus status) { EXPECT_EQ(ThermalStatusCode::SUCCESS, status.code); });
ASSERT_TRUE(ret.isOk());
+ // Expect to fail with null callback
+ ret = mThermal->unregisterThermalChangedCallback(nullptr, [](ThermalStatus status) {
+ EXPECT_EQ(ThermalStatusCode::FAILURE, status.code);
+ });
+ ASSERT_TRUE(ret.isOk());
}
// Test Thermal->unregisterThermalChangedCallback.