diff --git a/audio/aidl/aidl_api/android.hardware.audio.core/current/android/hardware/audio/core/IModule.aidl b/audio/aidl/aidl_api/android.hardware.audio.core/current/android/hardware/audio/core/IModule.aidl index e14e9c0f71..07a85f8fd8 100644 --- a/audio/aidl/aidl_api/android.hardware.audio.core/current/android/hardware/audio/core/IModule.aidl +++ b/audio/aidl/aidl_api/android.hardware.audio.core/current/android/hardware/audio/core/IModule.aidl @@ -74,6 +74,7 @@ interface IModule { boolean supportsVariableLatency(); int getAAudioMixerBurstCount(); int getAAudioHardwareBurstMinUsec(); + void prepareToDisconnectExternalDevice(int portId); const int DEFAULT_AAUDIO_MIXER_BURST_COUNT = 2; const int DEFAULT_AAUDIO_HARDWARE_BURST_MIN_DURATION_US = 1000; @VintfStability diff --git a/audio/aidl/android/hardware/audio/core/IModule.aidl b/audio/aidl/android/hardware/audio/core/IModule.aidl index e736c32e98..2d4d283be2 100644 --- a/audio/aidl/android/hardware/audio/core/IModule.aidl +++ b/audio/aidl/android/hardware/audio/core/IModule.aidl @@ -206,8 +206,10 @@ interface IModule { * after successful connection of an external device. * * Handling of a disconnect is done in a reverse order: - * 1. Reset port configuration using the 'resetAudioPortConfig' method. - * 2. Release the connected device port by calling the 'disconnectExternalDevice' + * 1. Notify the HAL module to prepare for device disconnection using + * 'prepareToDisconnectExternalDevice' method. + * 2. Reset port configuration using the 'resetAudioPortConfig' method. + * 3. Release the connected device port by calling the 'disconnectExternalDevice' * method. This also removes the audio routes associated with this * device port. * @@ -234,11 +236,15 @@ interface IModule { * instance previously instantiated using the 'connectExternalDevice' * method. * - * The framework will call this method before closing streams and resetting - * patches. This call can be used by the HAL module to prepare itself to - * device disconnection. If the HAL module indicates an error after the first - * call, the framework will call this method once again after closing associated - * streams and patches. + * On AIDL HAL v1, the framework will call this method before closing streams + * and resetting patches. This call can be used by the HAL module to prepare + * itself to device disconnection. If the HAL module indicates an error after + * the first call, the framework will call this method once again after closing + * associated streams and patches. + * + * On AIDL HAL v2 and later, the framework will call 'prepareToDisconnectExternalDevice' + * method to notify the HAL module to prepare itself for device disconnection. The + * framework will only call this method after closing associated streams and patches. * * @throws EX_ILLEGAL_ARGUMENT In the following cases: * - If the port can not be found by the ID. @@ -912,4 +918,20 @@ interface IModule { * @throw EX_UNSUPPORTED_OPERATION If the module does not support aaudio MMAP. */ int getAAudioHardwareBurstMinUsec(); + + /** + * Notify the HAL module to prepare for disconnecting an external device. + * + * This method is used to inform the HAL module that 'disconnectExternalDevice' will be + * called soon. The HAL module can rely on this method to abort active data operations + * early. The 'portId' must be of a connected device Port instance previously instantiated + * using 'connectExternalDevice' method. 'disconnectExternalDevice' method will be called + * soon after this method with the same 'portId'. + * + * @param portId The ID of the audio port that is about to disconnect + * @throws EX_ILLEGAL_ARGUMENT In the following cases: + * - If the port can not be found by the ID. + * - If this is not a connected device port. + */ + void prepareToDisconnectExternalDevice(int portId); } diff --git a/audio/aidl/default/Module.cpp b/audio/aidl/default/Module.cpp index 89e3d9915d..6c4d7ac975 100644 --- a/audio/aidl/default/Module.cpp +++ b/audio/aidl/default/Module.cpp @@ -758,6 +758,28 @@ ndk::ScopedAStatus Module::disconnectExternalDevice(int32_t in_portId) { return ndk::ScopedAStatus::ok(); } +ndk::ScopedAStatus Module::prepareToDisconnectExternalDevice(int32_t in_portId) { + auto& ports = getConfig().ports; + auto portIt = findById(ports, in_portId); + if (portIt == ports.end()) { + LOG(ERROR) << __func__ << ": port id " << in_portId << " not found"; + return ndk::ScopedAStatus::fromExceptionCode(EX_ILLEGAL_ARGUMENT); + } + if (portIt->ext.getTag() != AudioPortExt::Tag::device) { + LOG(ERROR) << __func__ << ": port id " << in_portId << " is not a device port"; + return ndk::ScopedAStatus::fromExceptionCode(EX_ILLEGAL_ARGUMENT); + } + auto connectedPortsIt = mConnectedDevicePorts.find(in_portId); + if (connectedPortsIt == mConnectedDevicePorts.end()) { + LOG(ERROR) << __func__ << ": port id " << in_portId << " is not a connected device port"; + return ndk::ScopedAStatus::fromExceptionCode(EX_ILLEGAL_ARGUMENT); + } + + onPrepareToDisconnectExternalDevice(*portIt); + + return ndk::ScopedAStatus::ok(); +} + ndk::ScopedAStatus Module::getAudioPatches(std::vector* _aidl_return) { *_aidl_return = getConfig().patches; LOG(DEBUG) << __func__ << ": returning " << _aidl_return->size() << " patches"; @@ -1537,6 +1559,11 @@ void Module::onExternalDeviceConnectionChanged( LOG(DEBUG) << __func__ << ": do nothing and return"; } +void Module::onPrepareToDisconnectExternalDevice( + const ::aidl::android::media::audio::common::AudioPort& audioPort __unused) { + LOG(DEBUG) << __func__ << ": do nothing and return"; +} + ndk::ScopedAStatus Module::onMasterMuteChanged(bool mute __unused) { LOG(VERBOSE) << __func__ << ": do nothing and return ok"; return ndk::ScopedAStatus::ok(); diff --git a/audio/aidl/default/include/core-impl/Module.h b/audio/aidl/default/include/core-impl/Module.h index caf43f1761..572b5979cc 100644 --- a/audio/aidl/default/include/core-impl/Module.h +++ b/audio/aidl/default/include/core-impl/Module.h @@ -70,6 +70,7 @@ class Module : public BnModule { const ::aidl::android::media::audio::common::AudioPort& in_templateIdAndAdditionalData, ::aidl::android::media::audio::common::AudioPort* _aidl_return) override; ndk::ScopedAStatus disconnectExternalDevice(int32_t in_portId) override; + ndk::ScopedAStatus prepareToDisconnectExternalDevice(int32_t in_portId) override; ndk::ScopedAStatus getAudioPatches(std::vector* _aidl_return) override; ndk::ScopedAStatus getAudioPort( int32_t in_portId, @@ -195,6 +196,8 @@ class Module : public BnModule { const std::vector<::aidl::android::media::audio::common::AudioPortConfig*>& sinks); virtual void onExternalDeviceConnectionChanged( const ::aidl::android::media::audio::common::AudioPort& audioPort, bool connected); + virtual void onPrepareToDisconnectExternalDevice( + const ::aidl::android::media::audio::common::AudioPort& audioPort); virtual ndk::ScopedAStatus onMasterMuteChanged(bool mute); virtual ndk::ScopedAStatus onMasterVolumeChanged(float volume); virtual std::vector<::aidl::android::media::audio::common::MicrophoneInfo> getMicrophoneInfos(); diff --git a/audio/aidl/vts/TestUtils.h b/audio/aidl/vts/TestUtils.h index e9f3251c6b..515b8a2a2f 100644 --- a/audio/aidl/vts/TestUtils.h +++ b/audio/aidl/vts/TestUtils.h @@ -69,6 +69,23 @@ inline ::testing::AssertionResult assertResult(const char* exp_expr, const char* << "\n but is has completed with: " << status; } +inline ::testing::AssertionResult assertIsOkOrUnknownTransaction( + const char* expr, const ::ndk::ScopedAStatus& status) { + if (status.getStatus() == STATUS_UNKNOWN_TRANSACTION) { + return ::testing::AssertionSuccess(); + } + return assertIsOk(expr, status); +} + +inline ::testing::AssertionResult assertResultOrUnknownTransaction( + const char* exp_expr, const char* act_expr, int32_t expected, + const ::ndk::ScopedAStatus& status) { + if (status.getStatus() == STATUS_UNKNOWN_TRANSACTION) { + return ::testing::AssertionSuccess(); + } + return assertResult(exp_expr, act_expr, expected, status); +} + } // namespace detail } // namespace android::hardware::audio::common::testing @@ -93,3 +110,15 @@ inline ::testing::AssertionResult assertResult(const char* exp_expr, const char* GTEST_SKIP() << "Skip data path for offload"; \ } \ }) + +// Test that the transaction status 'isOk' if it is a known transaction +#define EXPECT_IS_OK_OR_UNKNOWN_TRANSACTION(ret) \ + EXPECT_PRED_FORMAT1( \ + ::android::hardware::audio::common::testing::detail::assertIsOkOrUnknownTransaction, \ + ret) + +// Test that the transaction status is as expected if it is a known transaction +#define EXPECT_STATUS_OR_UNKNOWN_TRANSACTION(expected, ret) \ + EXPECT_PRED_FORMAT2( \ + ::android::hardware::audio::common::testing::detail::assertResultOrUnknownTransaction, \ + expected, ret) diff --git a/audio/aidl/vts/VtsHalAudioCoreModuleTargetTest.cpp b/audio/aidl/vts/VtsHalAudioCoreModuleTargetTest.cpp index 697aae9a30..f91795bc13 100644 --- a/audio/aidl/vts/VtsHalAudioCoreModuleTargetTest.cpp +++ b/audio/aidl/vts/VtsHalAudioCoreModuleTargetTest.cpp @@ -573,6 +573,8 @@ class WithDevicePortConnectedState { WithDevicePortConnectedState& operator=(const WithDevicePortConnectedState&) = delete; ~WithDevicePortConnectedState() { if (mModule != nullptr) { + EXPECT_IS_OK_OR_UNKNOWN_TRANSACTION(mModule->prepareToDisconnectExternalDevice(getId())) + << "when preparing to disconnect device port ID " << getId(); EXPECT_IS_OK(mModule->disconnectExternalDevice(getId())) << "when disconnecting device port ID " << getId(); } @@ -1753,6 +1755,9 @@ TEST_P(AudioCoreModule, TryConnectMissingDevice) { ScopedAStatus status = module->connectExternalDevice(portWithData, &connectedPort); EXPECT_STATUS(EX_ILLEGAL_STATE, status) << "static port " << portWithData.toString(); if (status.isOk()) { + EXPECT_IS_OK_OR_UNKNOWN_TRANSACTION( + module->prepareToDisconnectExternalDevice(connectedPort.id)) + << "when preparing to disconnect device port ID " << connectedPort.id; EXPECT_IS_OK(module->disconnectExternalDevice(connectedPort.id)) << "when disconnecting device port ID " << connectedPort.id; } @@ -1782,6 +1787,9 @@ TEST_P(AudioCoreModule, ConnectDisconnectExternalDeviceInvalidPorts) { invalidPort.id = portId; EXPECT_STATUS(EX_ILLEGAL_ARGUMENT, module->connectExternalDevice(invalidPort, &ignored)) << "port ID " << portId << ", when setting CONNECTED state"; + EXPECT_STATUS_OR_UNKNOWN_TRANSACTION(EX_ILLEGAL_ARGUMENT, + module->prepareToDisconnectExternalDevice(portId)) + << "port ID " << portId << ", when preparing to disconnect"; EXPECT_STATUS(EX_ILLEGAL_ARGUMENT, module->disconnectExternalDevice(portId)) << "port ID " << portId << ", when setting DISCONNECTED state"; } @@ -1792,6 +1800,9 @@ TEST_P(AudioCoreModule, ConnectDisconnectExternalDeviceInvalidPorts) { if (port.ext.getTag() != AudioPortExt::Tag::device) { EXPECT_STATUS(EX_ILLEGAL_ARGUMENT, module->connectExternalDevice(port, &ignored)) << "non-device port ID " << port.id << " when setting CONNECTED state"; + EXPECT_STATUS_OR_UNKNOWN_TRANSACTION(EX_ILLEGAL_ARGUMENT, + module->prepareToDisconnectExternalDevice(port.id)) + << "non-device port ID " << port.id << " when preparing to disconnect"; EXPECT_STATUS(EX_ILLEGAL_ARGUMENT, module->disconnectExternalDevice(port.id)) << "non-device port ID " << port.id << " when setting DISCONNECTED state"; } else { @@ -1800,6 +1811,10 @@ TEST_P(AudioCoreModule, ConnectDisconnectExternalDeviceInvalidPorts) { EXPECT_STATUS(EX_ILLEGAL_ARGUMENT, module->connectExternalDevice(port, &ignored)) << "for a permanently attached device port ID " << port.id << " when setting CONNECTED state"; + EXPECT_STATUS_OR_UNKNOWN_TRANSACTION( + EX_ILLEGAL_ARGUMENT, module->prepareToDisconnectExternalDevice(port.id)) + << "for a permanently attached device port ID " << port.id + << " when preparing to disconnect"; EXPECT_STATUS(EX_ILLEGAL_ARGUMENT, module->disconnectExternalDevice(port.id)) << "for a permanently attached device port ID " << port.id << " when setting DISCONNECTED state"; @@ -1817,6 +1832,9 @@ TEST_P(AudioCoreModule, ConnectDisconnectExternalDeviceTwice) { GTEST_SKIP() << "No external devices in the module."; } for (const auto& port : ports) { + EXPECT_STATUS_OR_UNKNOWN_TRANSACTION(EX_ILLEGAL_ARGUMENT, + module->prepareToDisconnectExternalDevice(port.id)) + << "when preparing to disconnect already disconnected device port ID " << port.id; EXPECT_STATUS(EX_ILLEGAL_ARGUMENT, module->disconnectExternalDevice(port.id)) << "when disconnecting already disconnected device port ID " << port.id; AudioPort portWithData = GenerateUniqueDeviceAddress(port); @@ -1851,6 +1869,10 @@ TEST_P(AudioCoreModule, DisconnectExternalDeviceNonResetPortConfig) { // Our test assumes that 'getAudioPort' returns at least one profile, and it // is not a dynamic profile. ASSERT_NO_FATAL_FAILURE(config.SetUp(module.get())); + EXPECT_IS_OK_OR_UNKNOWN_TRANSACTION( + module->prepareToDisconnectExternalDevice(portConnected.getId())) + << "when preparing to disconnect device port ID " << port.id + << " with active configuration " << config.getId(); EXPECT_STATUS(EX_ILLEGAL_STATE, module->disconnectExternalDevice(portConnected.getId())) << "when trying to disconnect device port ID " << port.id << " with active configuration " << config.getId();