Merge "IDumpstateDevice@1.1 polish" (d05efa2c) · Commits · e / os / android_hardware_interfaces

current.txt

+3 −3

Original line number	Diff line number	Diff line
		@@ -612,8 +612,8 @@ dd377f404a8e71f6191d295e10067db629b0f0c28e594af906f2bea5d87fe2cc android.hardwar
		40ab2c6866c18d32baf6e49e3053949e79601f56963a791e93e68b9ee18f718d android.hardware.bluetooth@1.1::IBluetoothHciCallbacks
		07d0a252b2d8fa35887908a996ba395cf392968395fc30afab791f46e0c22a52 android.hardware.boot@1.1::IBootControl
		74049a402be913963edfdd80828a53736570e9d8124a1bf18166b6ed46a6b0ab android.hardware.boot@1.1::types
		881aa8720fb1d69aa9843bfab69d810ab7654a61d2f5ab5e2626cbf240f24eaf android.hardware.dumpstate@1.1::types
		13b33f623521ded51a6c0f7ea5b77e97066d0aa1e38a83c2873f08ad67294f89 android.hardware.dumpstate@1.1::IDumpstateDevice
		446287268831f4ddfac4a51bb1c32ae1e48e47bccd535fccc2c4546d0e7c4013 android.hardware.dumpstate@1.1::types
		f284ffde7cadf5a1364b75ab313baf22401eeca289bdde2a2dc7a27ea4ab98d7 android.hardware.dumpstate@1.1::IDumpstateDevice
		ce8dbe76eb9ee94b46ef98f725be992e760a5751073d4f4912484026541371f3 android.hardware.health@2.1::IHealth
		26f04510a0b57aba5167c5c0a7c2f077c2acbb98b81902a072517829fd9fd67f android.hardware.health@2.1::IHealthInfoCallback
		db47f4ceceb1f06c656f39caa70c557b0f8471ef59fd58611bea667ffca20101 android.hardware.health@2.1::types

dumpstate/1.1/IDumpstateDevice.hal

+21 −3

Original line number	Diff line number	Diff line
		@@ -13,6 +13,7 @@
		* See the License for the specific language governing permissions and
		* limitations under the License.
		*/

		package android.hardware.dumpstate@1.1;

		import @1.0::IDumpstateDevice;
		@@ -28,14 +29,19 @@ interface IDumpstateDevice extends @1.0::IDumpstateDevice {
		* 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.
		*
		* @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.
		* @param mode A mode value to restrict dumped content.
		* @param timeoutMillis An approximate "budget" for how much time this call has been allotted.
		* If execution runs longer than this, the IDumpstateDevice service may be killed and only
		* partial information will be included in the report.
		* @return status A DumpstateStatus value indicating the final result.
		*/
		dumpstateBoard_1_1(handle h, DumpstateMode mode, uint64_t timeoutMillis);
		dumpstateBoard_1_1(handle h, DumpstateMode mode, uint64_t timeoutMillis)
		generates (DumpstateStatus status);

		/**
		* Turns device vendor logging on or off.
		@@ -46,8 +52,20 @@ interface IDumpstateDevice extends @1.0::IDumpstateDevice {
		* 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.
		*
		* @param enable Whether to enable or disable device vendor logging.
		* @return success Whether or not the change took effect.
		*/
		setDeviceLoggingEnabled(bool enable) generates (bool success);
		setDeviceLoggingEnabled(bool enable);

		/**
		* Queries the current state of 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.
		*
		* @return enabled Whether or not vendor logging is currently enabled.
		*/
		getDeviceLoggingEnabled() generates (bool enabled);
		};

dumpstate/1.1/types.hal

+24 −7

Original line number	Diff line number	Diff line
		@@ -13,6 +13,7 @@
		* See the License for the specific language governing permissions and
		* limitations under the License.
		*/

		package android.hardware.dumpstate@1.1;

		/**
		@@ -23,22 +24,18 @@ enum DumpstateMode : uint32_t {
		* Takes a bug report without user interference.
		*/
		FULL = 0,

		/**
		* Interactive bug report, i.e. triggered by the user.
		*/
		INTERACTIVE = 1,

		/**
		* Remote bug report triggered by DevicePolicyManager, for example.
		*/
		REMOTE = 2,

		/**
		* Bug report triggered on a wear device.
		*/
		WEAR = 3,

		/**
		* Bug report limited to only connectivity info (cellular, wifi, and networking). Sometimes
		* called "telephony" in legacy contexts.
		@@ -49,14 +46,34 @@ enum DumpstateMode : uint32_t {
		* user application traffic.
		*/
		CONNECTIVITY = 4,

		/**
		* Bug report limited to only wifi info.
		*/
		WIFI = 5,
		/**
		* Default mode, essentially analogous to calling @1.0::IDumpstateDevice.dumpstateBoard(handle).
		* This mode MUST be supported if the dumpstate HAL is implemented.
		*/
		DEFAULT = 6,
		};

		/**
		* Default mode.
		* A simple return enum for use with dumpstateBoard_1_1.
		*/
		enum DumpstateStatus : uint32_t {
		OK = 0,
		/**
		* Returned for cases where the device doesn't support the given DumpstateMode (e.g. a phone
		* trying to use DumpstateMode::WEAR).
		*/
		UNSUPPORTED_MODE = 1,
		/**
		* Returned for cases where an IllegalArgumentException is typically appropriate, e.g. missing
		* file descriptors.
		*/
		ILLEGAL_ARGUMENT = 2,
		/**
		* Returned when device logging is not enabled.
		*/
		DEFAULT = 6
		DEVICE_LOGGING_NOT_ENABLED = 3,
		};

dumpstate/1.1/vts/functional/VtsHalDumpstateV1_1TargetTest.cpp

+173 −27

Original line number	Diff line number	Diff line
		@@ -19,6 +19,7 @@
		#include <fcntl.h>
		#include <unistd.h>

		#include <functional>
		#include <vector>

		#include <android/hardware/dumpstate/1.1/IDumpstateDevice.h>
		@@ -34,21 +35,56 @@ namespace {
		using ::android::sp;
		using ::android::hardware::Return;
		using ::android::hardware::dumpstate::V1_1::DumpstateMode;
		using ::android::hardware::dumpstate::V1_1::DumpstateStatus;
		using ::android::hardware::dumpstate::V1_1::IDumpstateDevice;
		using ::android::hardware::dumpstate::V1_1::toString;

		class DumpstateHidl1_1Test : public ::testing::TestWithParam<std::string> {
		public:
		virtual void SetUp() override {
		protected:
		virtual void SetUp() override { GetService(); }

		void GetService() {
		dumpstate = IDumpstateDevice::getService(GetParam());
		ASSERT_NE(dumpstate, nullptr) << "Could not get HIDL instance";
		}

		void ToggleDeviceLogging(bool enable) {
		Return<void> status = dumpstate->setDeviceLoggingEnabled(enable);
		ASSERT_TRUE(status.isOk()) << "Status should be ok: " << status.description();

		if (!dumpstate->ping().isOk()) {
		ALOGW("IDumpstateDevice service appears to have exited lazily, attempting to get "
		"again");
		GetService();
		}

		Return<bool> logging_enabled = dumpstate->getDeviceLoggingEnabled();
		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");

		if (!dumpstate->ping().isOk()) {
		ALOGW("IDumpstateDevice service appears to have exited lazily, attempting to get "
		"again");
		GetService();
		}
		}

		void EnableDeviceLogging() { ToggleDeviceLogging(true); }

		void DisableDeviceLogging() { ToggleDeviceLogging(false); }

		sp<IDumpstateDevice> dumpstate;
		};

		#define TEST_FOR_DUMPSTATE_MODE(name, body, mode) \
		TEST_P(DumpstateHidl1_1Test, name##_##mode) { body(DumpstateMode::mode); }

		// We use a macro to define individual test cases instead of hidl_enum_range<> because some HAL
		// implementations are lazy and may call exit() at the end of dumpstateBoard(), which would cause
		// DEAD_OBJECT errors after the first iteration. Separate cases re-get the service each time as part
		// of SetUp(), and also provide better separation of concerns when specific modes are problematic.
		#define TEST_FOR_ALL_DUMPSTATE_MODES(name, body) \
		TEST_FOR_DUMPSTATE_MODE(name, body, FULL); \
		TEST_FOR_DUMPSTATE_MODE(name, body, INTERACTIVE); \
		@@ -60,21 +96,51 @@ class DumpstateHidl1_1Test : public ::testing::TestWithParam<std::string> {

		constexpr uint64_t kDefaultTimeoutMillis = 30 * 1000; // 30 seconds

		// Will only execute additional_assertions when status == expected.
		void AssertStatusForMode(const DumpstateMode mode, const Return<DumpstateStatus>& status,
		const DumpstateStatus expected,
		std::function<void()> additional_assertions = nullptr) {
		ASSERT_TRUE(status.isOk()) << "Status should be ok and return a more specific DumpstateStatus: "
		<< status.description();
		if (mode == DumpstateMode::DEFAULT) {
		ASSERT_EQ(expected, status) << "Required mode (DumpstateMode::" << toString(mode)
		<< "): status should be DumpstateStatus::" << toString(expected)
		<< ", but got DumpstateStatus::" << toString(status);
		} else {
		// The rest of the modes are optional to support, but they MUST return either the expected
		// value or UNSUPPORTED_MODE.
		ASSERT_TRUE(status == expected \|\| status == DumpstateStatus::UNSUPPORTED_MODE)
		<< "Optional mode (DumpstateMode::" << toString(mode)
		<< "): status should be DumpstateStatus::" << toString(expected)
		<< " or DumpstateStatus::UNSUPPORTED_MODE, but got DumpstateStatus::"
		<< toString(status);
		}
		if (status == expected && additional_assertions != nullptr) {
		additional_assertions();
		}
		}

		// Negative test: make sure dumpstateBoard() doesn't crash when passed a null pointer.
		TEST_FOR_ALL_DUMPSTATE_MODES(TestNullHandle, [this](DumpstateMode mode) {
		Return<void> status = dumpstate->dumpstateBoard_1_1(nullptr, mode, kDefaultTimeoutMillis);
		EnableDeviceLogging();

		ASSERT_TRUE(status.isOk()) << "Status should be ok: " << status.description();
		Return<DumpstateStatus> status =
		dumpstate->dumpstateBoard_1_1(nullptr, mode, kDefaultTimeoutMillis);

		AssertStatusForMode(mode, status, DumpstateStatus::ILLEGAL_ARGUMENT);
		});

		// Negative test: make sure dumpstateBoard() ignores a handle with no FD.
		TEST_FOR_ALL_DUMPSTATE_MODES(TestHandleWithNoFd, [this](DumpstateMode mode) {
		EnableDeviceLogging();

		native_handle_t* handle = native_handle_create(0, 0);
		ASSERT_NE(handle, nullptr) << "Could not create native_handle";

		Return<void> status = dumpstate->dumpstateBoard_1_1(handle, mode, kDefaultTimeoutMillis);
		Return<DumpstateStatus> status =
		dumpstate->dumpstateBoard_1_1(handle, mode, kDefaultTimeoutMillis);

		ASSERT_TRUE(status.isOk()) << "Status should be ok: " << status.description();
		AssertStatusForMode(mode, status, DumpstateStatus::ILLEGAL_ARGUMENT);

		native_handle_close(handle);
		native_handle_delete(handle);
		@@ -82,6 +148,8 @@ TEST_FOR_ALL_DUMPSTATE_MODES(TestHandleWithNoFd, [this](DumpstateMode mode) {

		// Positive test: make sure dumpstateBoard() writes something to the FD.
		TEST_FOR_ALL_DUMPSTATE_MODES(TestOk, [this](DumpstateMode mode) {
		EnableDeviceLogging();

		// Index 0 corresponds to the read end of the pipe; 1 to the write end.
		int fds[2];
		ASSERT_EQ(0, pipe2(fds, O_NONBLOCK)) << errno;
		@@ -90,12 +158,14 @@ TEST_FOR_ALL_DUMPSTATE_MODES(TestOk, [this](DumpstateMode mode) {
		ASSERT_NE(handle, nullptr) << "Could not create native_handle";
		handle->data[0] = fds[1];

		Return<void> status = dumpstate->dumpstateBoard_1_1(handle, mode, kDefaultTimeoutMillis);
		ASSERT_TRUE(status.isOk()) << "Status should be ok: " << status.description();
		Return<DumpstateStatus> status =
		dumpstate->dumpstateBoard_1_1(handle, mode, kDefaultTimeoutMillis);

		// Check that at least one byte was written
		AssertStatusForMode(mode, status, DumpstateStatus::OK, [&fds]() {
		// Check that at least one byte was written.
		char buff;
		ASSERT_EQ(1, read(fds[0], &buff, 1)) << "dumped nothing";
		ASSERT_EQ(1, read(fds[0], &buff, 1)) << "Dumped nothing";
		});

		native_handle_close(handle);
		native_handle_delete(handle);
		@@ -103,6 +173,8 @@ TEST_FOR_ALL_DUMPSTATE_MODES(TestOk, [this](DumpstateMode mode) {

		// Positive test: make sure dumpstateBoard() doesn't crash with two FDs.
		TEST_FOR_ALL_DUMPSTATE_MODES(TestHandleWithTwoFds, [this](DumpstateMode mode) {
		EnableDeviceLogging();

		int fds1[2];
		int fds2[2];
		ASSERT_EQ(0, pipe2(fds1, O_NONBLOCK)) << errno;
		@@ -113,8 +185,17 @@ TEST_FOR_ALL_DUMPSTATE_MODES(TestHandleWithTwoFds, [this](DumpstateMode mode) {
		handle->data[0] = fds1[1];
		handle->data[1] = fds2[1];

		Return<void> status = dumpstate->dumpstateBoard_1_1(handle, mode, kDefaultTimeoutMillis);
		ASSERT_TRUE(status.isOk()) << "Status should be ok: " << status.description();
		Return<DumpstateStatus> status =
		dumpstate->dumpstateBoard_1_1(handle, mode, kDefaultTimeoutMillis);

		AssertStatusForMode(mode, status, DumpstateStatus::OK, [&fds1, &fds2]() {
		// Check that at least one byte was written to one of the FDs.
		char buff;
		size_t read1 = read(fds1[0], &buff, 1);
		size_t read2 = read(fds2[0], &buff, 1);
		// Sometimes read returns -1, so we can't just add them together and expect >= 1.
		ASSERT_TRUE(read1 == 1 \|\| read2 == 1) << "Dumped nothing";
		});

		native_handle_close(handle);
		native_handle_delete(handle);
		@@ -122,6 +203,8 @@ TEST_FOR_ALL_DUMPSTATE_MODES(TestHandleWithTwoFds, [this](DumpstateMode mode) {

		// Make sure dumpstateBoard_1_1 actually validates its arguments.
		TEST_P(DumpstateHidl1_1Test, TestInvalidModeArgument_Negative) {
		EnableDeviceLogging();

		int fds[2];
		ASSERT_EQ(0, pipe2(fds, O_NONBLOCK)) << errno;

		@@ -129,16 +212,21 @@ TEST_P(DumpstateHidl1_1Test, TestInvalidModeArgument_Negative) {
		ASSERT_NE(handle, nullptr) << "Could not create native_handle";
		handle->data[0] = fds[1];

		Return<void> status = dumpstate->dumpstateBoard_1_1(handle, static_cast<DumpstateMode>(-100),
		kDefaultTimeoutMillis);
		ASSERT_FALSE(status.isOk()) << "Status should not be ok with invalid mode param: "
		Return<DumpstateStatus> status = dumpstate->dumpstateBoard_1_1(
		handle, static_cast<DumpstateMode>(-100), kDefaultTimeoutMillis);

		ASSERT_TRUE(status.isOk()) << "Status should be ok and return a more specific DumpstateStatus: "
		<< status.description();
		ASSERT_EQ(status, DumpstateStatus::ILLEGAL_ARGUMENT)
		<< "Should return DumpstateStatus::ILLEGAL_ARGUMENT for invalid mode param";

		native_handle_close(handle);
		native_handle_delete(handle);
		}

		TEST_P(DumpstateHidl1_1Test, TestInvalidModeArgument_Undefined) {
		EnableDeviceLogging();

		int fds[2];
		ASSERT_EQ(0, pipe2(fds, O_NONBLOCK)) << errno;

		@@ -146,26 +234,84 @@ TEST_P(DumpstateHidl1_1Test, TestInvalidModeArgument_Undefined) {
		ASSERT_NE(handle, nullptr) << "Could not create native_handle";
		handle->data[0] = fds[1];

		Return<void> status = dumpstate->dumpstateBoard_1_1(handle, static_cast<DumpstateMode>(9001),
		kDefaultTimeoutMillis);
		ASSERT_FALSE(status.isOk()) << "Status should not be ok with invalid mode param: "
		Return<DumpstateStatus> status = dumpstate->dumpstateBoard_1_1(
		handle, static_cast<DumpstateMode>(9001), kDefaultTimeoutMillis);

		ASSERT_TRUE(status.isOk()) << "Status should be ok and return a more specific DumpstateStatus: "
		<< status.description();
		ASSERT_EQ(status, DumpstateStatus::ILLEGAL_ARGUMENT)
		<< "Should return DumpstateStatus::ILLEGAL_ARGUMENT for invalid mode param";

		native_handle_close(handle);
		native_handle_delete(handle);
		}

		// Make sure toggling device logging doesn't crash.
		TEST_P(DumpstateHidl1_1Test, TestEnableDeviceLogging) {
		Return<bool> status = dumpstate->setDeviceLoggingEnabled(true);
		// Positive test: make sure dumpstateBoard() from 1.0 doesn't fail.
		TEST_P(DumpstateHidl1_1Test, Test1_0MethodOk) {
		EnableDeviceLogging();

		int fds[2];
		ASSERT_EQ(0, pipe2(fds, O_NONBLOCK)) << errno;

		native_handle_t* handle = native_handle_create(1, 0);
		ASSERT_NE(handle, nullptr) << "Could not create native_handle";
		handle->data[0] = fds[1];

		Return<void> status = dumpstate->dumpstateBoard(handle);

		ASSERT_TRUE(status.isOk()) << "Status should be ok: " << status.description();

		// Check that at least one byte was written.
		char buff;
		ASSERT_EQ(1, read(fds[0], &buff, 1)) << "Dumped nothing";

		native_handle_close(handle);
		native_handle_delete(handle);
		}

		TEST_P(DumpstateHidl1_1Test, TestDisableDeviceLogging) {
		Return<bool> status = dumpstate->setDeviceLoggingEnabled(false);
		// Make sure disabling device logging behaves correctly.
		TEST_FOR_ALL_DUMPSTATE_MODES(TestDeviceLoggingDisabled, [this](DumpstateMode mode) {
		DisableDeviceLogging();

		ASSERT_TRUE(status.isOk()) << "Status should be ok: " << status.description();
		// Index 0 corresponds to the read end of the pipe; 1 to the write end.
		int fds[2];
		ASSERT_EQ(0, pipe2(fds, O_NONBLOCK)) << errno;

		native_handle_t* handle = native_handle_create(1, 0);
		ASSERT_NE(handle, nullptr) << "Could not create native_handle";
		handle->data[0] = fds[1];

		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";
		});

		native_handle_close(handle);
		native_handle_delete(handle);
		});

		// Double-enable is perfectly valid, but the second call shouldn't do anything.
		TEST_P(DumpstateHidl1_1Test, TestRepeatedEnable) {
		EnableDeviceLogging();
		EnableDeviceLogging();
		}

		// Double-disable is perfectly valid, but the second call shouldn't do anything.
		TEST_P(DumpstateHidl1_1Test, TestRepeatedDisable) {
		DisableDeviceLogging();
		DisableDeviceLogging();
		}

		// Toggling in short order is perfectly valid.
		TEST_P(DumpstateHidl1_1Test, TestRepeatedToggle) {
		EnableDeviceLogging();
		DisableDeviceLogging();
		EnableDeviceLogging();
		DisableDeviceLogging();
		}

		INSTANTIATE_TEST_SUITE_P(