Merge "IDumpstateDevice 1.1 tweak: "device" -> "verbose""
diff --git a/current.txt b/current.txt
index 2fa5dca..150a07b 100644
--- a/current.txt
+++ b/current.txt
@@ -646,7 +646,7 @@
66931c2506fbb5af61f20138cb05e0a09e7bf67d6964c231d27c648933bb33ec android.hardware.drm@1.3::ICryptoFactory
994d08ab27d613022c258a9ec48cece7adf2a305e92df5d76ef923e2c6665f64 android.hardware.drm@1.3::IDrmFactory
446287268831f4ddfac4a51bb1c32ae1e48e47bccd535fccc2c4546d0e7c4013 android.hardware.dumpstate@1.1::types
-f284ffde7cadf5a1364b75ab313baf22401eeca289bdde2a2dc7a27ea4ab98d7 android.hardware.dumpstate@1.1::IDumpstateDevice
+186bc152ae189ab48f3a761a44ddf5edd0d483073c5b6ca1f802f8b50488b754 android.hardware.dumpstate@1.1::IDumpstateDevice
769d346927a94fd40ee80a5a976d8d15cf022ef99c5900738f4a82f26c0ed229 android.hardware.gnss@2.1::types
626db710bf917ecf551a0b0b1f25be10bf52758f43e0fc808b148b6aae2ef73e android.hardware.gnss@2.1::IGnss
ba5ac712b2a656dc07c83ab4a7a2c2f3bee1bbcb752e8b8ffa9b672f3b5b0728 android.hardware.gnss@2.1::IGnssAntennaInfo
diff --git a/dumpstate/1.1/IDumpstateDevice.hal b/dumpstate/1.1/IDumpstateDevice.hal
index 502c460..183404d 100644
--- a/dumpstate/1.1/IDumpstateDevice.hal
+++ b/dumpstate/1.1/IDumpstateDevice.hal
@@ -29,8 +29,9 @@
* The 1.0 version of #dumpstateBoard(handle) should just delegate to this new method and pass
* DumpstateMode::DEFAULT and a timeout of 30,000ms (30 seconds).
*
- * This method may still be called by the dumpstate routine even if getDeviceLoggingEnabled
- * returns false. In this case, it must return DumpstateStatus::DEVICE_LOGGING_NOT_ENABLED.
+ * This method may still be called by the dumpstate routine even if getVerboseLoggingEnabled
+ * returns false. In this case, it may include essential information but must not include
+ * information that identifies the user.
*
* @param h A native handle with one or two valid file descriptors. The first FD is for text
* output, the second (if present) is for binary output.
@@ -44,7 +45,7 @@
generates (DumpstateStatus status);
/**
- * Turns device vendor logging on or off.
+ * Turns verbose device vendor logging on or off.
*
* The setting should be persistent across reboots. Underlying implementations may need to start
* vendor logging daemons, set system properties, or change logging masks, for example. Given
@@ -52,20 +53,21 @@
* memory/storage/battery impacts, calling this method on a user build should only be done after
* user consent has been obtained, e.g. from a toggle in developer settings.
*
- * Even if device logging has been disabled, dumpstateBoard may still be called by the dumpstate
- * routine. In this case, it must return DumpstateStatus::DEVICE_LOGGING_NOT_ENABLED.
+ * Even if verbose logging has been disabled, dumpstateBoard may still be called by the
+ * dumpstate routine, and essential information that does not identify the user may be included.
*
- * @param enable Whether to enable or disable device vendor logging.
+ * @param enable Whether to enable or disable verbose vendor logging.
*/
- setDeviceLoggingEnabled(bool enable);
+ setVerboseLoggingEnabled(bool enable);
/**
- * Queries the current state of device logging. Primarily for UI and informative purposes.
+ * Queries the current state of verbose device logging. Primarily for UI and informative
+ * purposes.
*
- * Even if device logging has been disabled, dumpstateBoard may still be called by the dumpstate
- * routine. In this case, it must return DumpstateStatus::DEVICE_LOGGING_NOT_ENABLED.
+ * Even if verbose logging has been disabled, dumpstateBoard may still be called by the
+ * dumpstate routine, and essential information that does not identify the user may be included.
*
- * @return enabled Whether or not vendor logging is currently enabled.
+ * @return enabled Whether or not verbose vendor logging is currently enabled.
*/
- getDeviceLoggingEnabled() generates (bool enabled);
+ getVerboseLoggingEnabled() generates (bool enabled);
};
diff --git a/dumpstate/1.1/vts/functional/VtsHalDumpstateV1_1TargetTest.cpp b/dumpstate/1.1/vts/functional/VtsHalDumpstateV1_1TargetTest.cpp
index 089b039..51dce5e 100644
--- a/dumpstate/1.1/vts/functional/VtsHalDumpstateV1_1TargetTest.cpp
+++ b/dumpstate/1.1/vts/functional/VtsHalDumpstateV1_1TargetTest.cpp
@@ -48,8 +48,8 @@
ASSERT_NE(dumpstate, nullptr) << "Could not get HIDL instance";
}
- void ToggleDeviceLogging(bool enable) {
- Return<void> status = dumpstate->setDeviceLoggingEnabled(enable);
+ void ToggleVerboseLogging(bool enable) {
+ Return<void> status = dumpstate->setVerboseLoggingEnabled(enable);
ASSERT_TRUE(status.isOk()) << "Status should be ok: " << status.description();
if (!dumpstate->ping().isOk()) {
@@ -58,11 +58,11 @@
GetService();
}
- Return<bool> logging_enabled = dumpstate->getDeviceLoggingEnabled();
+ Return<bool> logging_enabled = dumpstate->getVerboseLoggingEnabled();
ASSERT_TRUE(logging_enabled.isOk())
<< "Status should be ok: " << logging_enabled.description();
ASSERT_EQ(logging_enabled, enable)
- << "Device logging should now be " << (enable ? "enabled" : "disabled");
+ << "Verbose logging should now be " << (enable ? "enabled" : "disabled");
if (!dumpstate->ping().isOk()) {
ALOGW("IDumpstateDevice service appears to have exited lazily, attempting to get "
@@ -71,9 +71,9 @@
}
}
- void EnableDeviceLogging() { ToggleDeviceLogging(true); }
+ void EnableVerboseLogging() { ToggleVerboseLogging(true); }
- void DisableDeviceLogging() { ToggleDeviceLogging(false); }
+ void DisableVerboseLogging() { ToggleVerboseLogging(false); }
sp<IDumpstateDevice> dumpstate;
};
@@ -122,7 +122,7 @@
// Negative test: make sure dumpstateBoard() doesn't crash when passed a null pointer.
TEST_FOR_ALL_DUMPSTATE_MODES(TestNullHandle, [this](DumpstateMode mode) {
- EnableDeviceLogging();
+ EnableVerboseLogging();
Return<DumpstateStatus> status =
dumpstate->dumpstateBoard_1_1(nullptr, mode, kDefaultTimeoutMillis);
@@ -132,7 +132,7 @@
// Negative test: make sure dumpstateBoard() ignores a handle with no FD.
TEST_FOR_ALL_DUMPSTATE_MODES(TestHandleWithNoFd, [this](DumpstateMode mode) {
- EnableDeviceLogging();
+ EnableVerboseLogging();
native_handle_t* handle = native_handle_create(0, 0);
ASSERT_NE(handle, nullptr) << "Could not create native_handle";
@@ -148,7 +148,7 @@
// Positive test: make sure dumpstateBoard() writes something to the FD.
TEST_FOR_ALL_DUMPSTATE_MODES(TestOk, [this](DumpstateMode mode) {
- EnableDeviceLogging();
+ EnableVerboseLogging();
// Index 0 corresponds to the read end of the pipe; 1 to the write end.
int fds[2];
@@ -173,7 +173,7 @@
// Positive test: make sure dumpstateBoard() doesn't crash with two FDs.
TEST_FOR_ALL_DUMPSTATE_MODES(TestHandleWithTwoFds, [this](DumpstateMode mode) {
- EnableDeviceLogging();
+ EnableVerboseLogging();
int fds1[2];
int fds2[2];
@@ -203,7 +203,7 @@
// Make sure dumpstateBoard_1_1 actually validates its arguments.
TEST_P(DumpstateHidl1_1Test, TestInvalidModeArgument_Negative) {
- EnableDeviceLogging();
+ EnableVerboseLogging();
int fds[2];
ASSERT_EQ(0, pipe2(fds, O_NONBLOCK)) << errno;
@@ -225,7 +225,7 @@
}
TEST_P(DumpstateHidl1_1Test, TestInvalidModeArgument_Undefined) {
- EnableDeviceLogging();
+ EnableVerboseLogging();
int fds[2];
ASSERT_EQ(0, pipe2(fds, O_NONBLOCK)) << errno;
@@ -248,7 +248,7 @@
// Positive test: make sure dumpstateBoard() from 1.0 doesn't fail.
TEST_P(DumpstateHidl1_1Test, Test1_0MethodOk) {
- EnableDeviceLogging();
+ EnableVerboseLogging();
int fds[2];
ASSERT_EQ(0, pipe2(fds, O_NONBLOCK)) << errno;
@@ -269,9 +269,10 @@
native_handle_delete(handle);
}
-// Make sure disabling device logging behaves correctly.
-TEST_FOR_ALL_DUMPSTATE_MODES(TestDeviceLoggingDisabled, [this](DumpstateMode mode) {
- DisableDeviceLogging();
+// Make sure disabling verbose logging behaves correctly. Some info is still allowed to be emitted,
+// but it can't have privacy/storage/battery impacts.
+TEST_FOR_ALL_DUMPSTATE_MODES(TestVerboseLoggingDisabled, [this](DumpstateMode mode) {
+ DisableVerboseLogging();
// Index 0 corresponds to the read end of the pipe; 1 to the write end.
int fds[2];
@@ -284,11 +285,10 @@
Return<DumpstateStatus> status =
dumpstate->dumpstateBoard_1_1(handle, mode, kDefaultTimeoutMillis);
- AssertStatusForMode(mode, status, DumpstateStatus::DEVICE_LOGGING_NOT_ENABLED, [&fds]() {
- // Check that nothing was written. Could return 0 or -1.
- char buff;
- ASSERT_NE(1, read(fds[0], &buff, 1)) << "Dumped something when device logging is disabled";
- });
+ // We don't include additional assertions here about the file passed in. If verbose logging is
+ // disabled, the OEM may choose to include nothing at all, but it is allowed to include some
+ // essential information based on the mode as long as it isn't private user information.
+ AssertStatusForMode(mode, status, DumpstateStatus::OK);
native_handle_close(handle);
native_handle_delete(handle);
@@ -296,22 +296,22 @@
// Double-enable is perfectly valid, but the second call shouldn't do anything.
TEST_P(DumpstateHidl1_1Test, TestRepeatedEnable) {
- EnableDeviceLogging();
- EnableDeviceLogging();
+ EnableVerboseLogging();
+ EnableVerboseLogging();
}
// Double-disable is perfectly valid, but the second call shouldn't do anything.
TEST_P(DumpstateHidl1_1Test, TestRepeatedDisable) {
- DisableDeviceLogging();
- DisableDeviceLogging();
+ DisableVerboseLogging();
+ DisableVerboseLogging();
}
// Toggling in short order is perfectly valid.
TEST_P(DumpstateHidl1_1Test, TestRepeatedToggle) {
- EnableDeviceLogging();
- DisableDeviceLogging();
- EnableDeviceLogging();
- DisableDeviceLogging();
+ EnableVerboseLogging();
+ DisableVerboseLogging();
+ EnableVerboseLogging();
+ DisableVerboseLogging();
}
INSTANTIATE_TEST_SUITE_P(