From 900536cf9a8f963ec83923275790430e929f4f8f Mon Sep 17 00:00:00 2001 From: Nathalie Le Clair Date: Wed, 23 Mar 2022 09:02:20 +0100 Subject: [PATCH] Don't remove devices on onHotplug() This CL affects TV panels and Audio Systems only. Before HdmiCecNetwork existed: devices were removed when HotplugDetectionAction detected a hotplug out, and TIF was informed. Before this CL: devices were removed from the CEC network onHotplug, but the listener to inform TIF wasn't invoked. This was causing multiple issues. After: on TV panels and Audio Systems, only remove devices when HotplugDetectionAction detects a hotplug out, just like before HdmiCecNetwork existed. Test: atest Bug:213417037 Change-Id: I4181b4650b70101da44fd205e4df9eb566f74496 --- .../hdmi/IHdmiDeviceEventListener.aidl | 2 + .../hdmi/HdmiCecLocalDevicePlayback.java | 1 + .../server/hdmi/HdmiCecLocalDeviceTv.java | 5 + .../android/server/hdmi/HdmiCecNetwork.java | 7 +- .../server/hdmi/HdmiControlService.java | 4 - .../hdmi/HdmiCecLocalDevicePlaybackTest.java | 28 ++++ .../server/hdmi/HdmiCecLocalDeviceTvTest.java | 122 ++++++++++++++++++ 7 files changed, 164 insertions(+), 5 deletions(-) diff --git a/core/java/android/hardware/hdmi/IHdmiDeviceEventListener.aidl b/core/java/android/hardware/hdmi/IHdmiDeviceEventListener.aidl index 69f29111635c5..b2ddef9c29a61 100644 --- a/core/java/android/hardware/hdmi/IHdmiDeviceEventListener.aidl +++ b/core/java/android/hardware/hdmi/IHdmiDeviceEventListener.aidl @@ -22,6 +22,8 @@ import android.hardware.hdmi.HdmiDeviceInfo; * Callback interface definition for HDMI client to get informed of * the CEC logical device status change event. * + * Only to be used on TV panel and Audio System devices (b/226317598). + * * @hide */ oneway interface IHdmiDeviceEventListener { diff --git a/services/core/java/com/android/server/hdmi/HdmiCecLocalDevicePlayback.java b/services/core/java/com/android/server/hdmi/HdmiCecLocalDevicePlayback.java index 27f64ecbe99a7..ccb27eef20757 100644 --- a/services/core/java/com/android/server/hdmi/HdmiCecLocalDevicePlayback.java +++ b/services/core/java/com/android/server/hdmi/HdmiCecLocalDevicePlayback.java @@ -214,6 +214,7 @@ public class HdmiCecLocalDevicePlayback extends HdmiCecLocalDeviceSource { } else { // We'll not invalidate the active source on the hotplug event to pass CETC 11.2.2-2 ~ 3 getWakeLock().release(); + mService.getHdmiCecNetwork().removeDevicesConnectedToPort(portId); mDelayedStandbyHandler.removeCallbacksAndMessages(null); mDelayedStandbyHandler.postDelayed(new DelayedStandbyRunnable(), diff --git a/services/core/java/com/android/server/hdmi/HdmiCecLocalDeviceTv.java b/services/core/java/com/android/server/hdmi/HdmiCecLocalDeviceTv.java index d249d578cf67a..9212fb6047212 100644 --- a/services/core/java/com/android/server/hdmi/HdmiCecLocalDeviceTv.java +++ b/services/core/java/com/android/server/hdmi/HdmiCecLocalDeviceTv.java @@ -1245,6 +1245,11 @@ final class HdmiCecLocalDeviceTv extends HdmiCecLocalDevice { @ServiceThreadOnly void onHotplug(int portId, boolean connected) { assertRunOnServiceThread(); + + if (!connected) { + mService.getHdmiCecNetwork().removeCecSwitches(portId); + } + // Turning System Audio Mode off when the AVR is unlugged or standby. // When the device is not unplugged but reawaken from standby, we check if the System // Audio Control Feature is enabled or not then decide if turning SAM on/off accordingly. diff --git a/services/core/java/com/android/server/hdmi/HdmiCecNetwork.java b/services/core/java/com/android/server/hdmi/HdmiCecNetwork.java index 3a0c0886e7080..16ade37aa8e63 100644 --- a/services/core/java/com/android/server/hdmi/HdmiCecNetwork.java +++ b/services/core/java/com/android/server/hdmi/HdmiCecNetwork.java @@ -730,7 +730,7 @@ public class HdmiCecNetwork { return mCecSwitches; } - void removeDevicesConnectedToPort(int portId) { + void removeCecSwitches(int portId) { Iterator it = mCecSwitches.iterator(); while (it.hasNext()) { int path = it.next(); @@ -739,6 +739,11 @@ public class HdmiCecNetwork { it.remove(); } } + } + + void removeDevicesConnectedToPort(int portId) { + removeCecSwitches(portId); + List toRemove = new ArrayList<>(); for (int i = 0; i < mDeviceInfos.size(); i++) { int key = mDeviceInfos.keyAt(i); diff --git a/services/core/java/com/android/server/hdmi/HdmiControlService.java b/services/core/java/com/android/server/hdmi/HdmiControlService.java index 8ac233114b483..12380abd0d380 100644 --- a/services/core/java/com/android/server/hdmi/HdmiControlService.java +++ b/services/core/java/com/android/server/hdmi/HdmiControlService.java @@ -1363,10 +1363,6 @@ public class HdmiControlService extends SystemService { device.onHotplug(portId, connected); } - if (!connected) { - mHdmiCecNetwork.removeDevicesConnectedToPort(portId); - } - announceHotplugEvent(portId, connected); } diff --git a/services/tests/servicestests/src/com/android/server/hdmi/HdmiCecLocalDevicePlaybackTest.java b/services/tests/servicestests/src/com/android/server/hdmi/HdmiCecLocalDevicePlaybackTest.java index 6fc3354f07e7d..86130daf4aac7 100644 --- a/services/tests/servicestests/src/com/android/server/hdmi/HdmiCecLocalDevicePlaybackTest.java +++ b/services/tests/servicestests/src/com/android/server/hdmi/HdmiCecLocalDevicePlaybackTest.java @@ -2033,4 +2033,32 @@ public class HdmiCecLocalDevicePlaybackTest { mTestLooper.dispatchAll(); assertThat(mPowerManager.isInteractive()).isFalse(); } + + @Test + public void onHotplugClearsDevices() { + mHdmiControlService.getHdmiCecNetwork().clearDeviceList(); + assertThat(mHdmiControlService.getHdmiCecNetwork().getDeviceInfoList(false)) + .isEmpty(); + // Add a device to the network and assert that this device is included in the list of + // devices. + HdmiDeviceInfo infoPlayback = HdmiDeviceInfo.cecDeviceBuilder() + .setLogicalAddress(Constants.ADDR_PLAYBACK_3) + .setPhysicalAddress(0x1000) + .setPortId(PORT_1) + .setDeviceType(HdmiDeviceInfo.DEVICE_PLAYBACK) + .setVendorId(0x1000) + .setDisplayName("Playback 3") + .build(); + mHdmiControlService.getHdmiCecNetwork().addCecDevice(infoPlayback); + mTestLooper.dispatchAll(); + assertThat(mHdmiControlService.getHdmiCecNetwork().getDeviceInfoList(false)) + .hasSize(1); + + // HAL detects a hotplug out. Assert that this device gets removed from the list of devices. + mHdmiControlService.onHotplug(PORT_1, false); + mTestLooper.dispatchAll(); + + assertThat(mHdmiControlService.getHdmiCecNetwork().getDeviceInfoList(false)) + .isEmpty(); + } } diff --git a/services/tests/servicestests/src/com/android/server/hdmi/HdmiCecLocalDeviceTvTest.java b/services/tests/servicestests/src/com/android/server/hdmi/HdmiCecLocalDeviceTvTest.java index d104871f488a6..f1ad957eb4124 100644 --- a/services/tests/servicestests/src/com/android/server/hdmi/HdmiCecLocalDeviceTvTest.java +++ b/services/tests/servicestests/src/com/android/server/hdmi/HdmiCecLocalDeviceTvTest.java @@ -53,6 +53,7 @@ import org.mockito.MockitoAnnotations; import java.util.ArrayList; import java.util.Collections; +import java.util.List; import java.util.concurrent.TimeUnit; @SmallTest @@ -61,6 +62,7 @@ import java.util.concurrent.TimeUnit; /** Tests for {@link HdmiCecLocalDeviceTv} class. */ public class HdmiCecLocalDeviceTvTest { private static final int TIMEOUT_MS = HdmiConfig.TIMEOUT_MS + 1; + private static final int PORT_1 = 1; private static final String[] SADS_NOT_TO_QUERY = new String[]{ HdmiControlManager.CEC_SETTING_NAME_QUERY_SAD_MPEG1, @@ -90,6 +92,25 @@ public class HdmiCecLocalDeviceTvTest { private int mTvPhysicalAddress; private int mTvLogicalAddress; private boolean mWokenUp; + private List mDeviceEventListeners = new ArrayList<>(); + + private class DeviceEventListener { + private HdmiDeviceInfo mDevice; + private int mStatus; + + DeviceEventListener(HdmiDeviceInfo device, int status) { + this.mDevice = device; + this.mStatus = status; + } + + int getStatus() { + return mStatus; + } + + HdmiDeviceInfo getDeviceInfo() { + return mDevice; + } + } @Mock private AudioManager mAudioManager; @@ -133,6 +154,11 @@ public class HdmiCecLocalDeviceTvTest { AudioManager getAudioManager() { return mAudioManager; } + + @Override + void invokeDeviceEventListeners(HdmiDeviceInfo device, int status) { + mDeviceEventListeners.add(new DeviceEventListener(device, status)); + } }; mHdmiCecLocalDeviceTv = new HdmiCecLocalDeviceTv(mHdmiControlService); @@ -609,4 +635,100 @@ public class HdmiCecLocalDeviceTvTest { ADDR_TV, ADDR_PLAYBACK_1); assertThat(mNativeWrapper.getResultMessages()).contains(givePhysicalAddress); } + + @Test + public void hotplugDetectionActionClearsDevices() { + mHdmiControlService.getHdmiCecNetwork().clearDeviceList(); + assertThat(mHdmiControlService.getHdmiCecNetwork().getDeviceInfoList(false)) + .isEmpty(); + // Add a device to the network and assert that this device is included in the list of + // devices. + HdmiDeviceInfo infoPlayback = HdmiDeviceInfo.cecDeviceBuilder() + .setLogicalAddress(Constants.ADDR_PLAYBACK_2) + .setPhysicalAddress(0x1000) + .setPortId(PORT_1) + .setDeviceType(HdmiDeviceInfo.DEVICE_PLAYBACK) + .setVendorId(0x1000) + .setDisplayName("Playback 2") + .build(); + mHdmiControlService.getHdmiCecNetwork().addCecDevice(infoPlayback); + mTestLooper.dispatchAll(); + assertThat(mHdmiControlService.getHdmiCecNetwork().getDeviceInfoList(false)) + .hasSize(1); + mDeviceEventListeners.clear(); + assertThat(mDeviceEventListeners.size()).isEqualTo(0); + + // HAL detects a hotplug out. Assert that this device stays in the list of devices. + mHdmiControlService.onHotplug(PORT_1, false); + assertThat(mHdmiControlService.getHdmiCecNetwork().getDeviceInfoList(false)) + .hasSize(1); + assertThat(mDeviceEventListeners).isEmpty(); + mTestLooper.dispatchAll(); + // Make the device not acknowledge the poll message sent by the HotplugDetectionAction. + // Assert that this device is removed from the list of devices. + mNativeWrapper.setPollAddressResponse(Constants.ADDR_PLAYBACK_2, SendMessageResult.NACK); + for (int pollCount = 0; pollCount < HotplugDetectionAction.TIMEOUT_COUNT; pollCount++) { + mTestLooper.moveTimeForward(HotplugDetectionAction.POLLING_INTERVAL_MS_FOR_TV); + mTestLooper.dispatchAll(); + } + + assertThat(mHdmiControlService.getHdmiCecNetwork().getDeviceInfoList(false)) + .isEmpty(); + assertThat(mDeviceEventListeners.size()).isEqualTo(1); + assertThat(mDeviceEventListeners.get(0).getStatus()) + .isEqualTo(HdmiControlManager.DEVICE_EVENT_REMOVE_DEVICE); + HdmiDeviceInfo removedDeviceInfo = mDeviceEventListeners.get(0).getDeviceInfo(); + assertThat(removedDeviceInfo.getPortId()).isEqualTo(PORT_1); + assertThat(removedDeviceInfo.getLogicalAddress()).isEqualTo(Constants.ADDR_PLAYBACK_2); + assertThat(removedDeviceInfo.getPhysicalAddress()).isEqualTo(0x1000); + assertThat(removedDeviceInfo.getDeviceType()).isEqualTo(HdmiDeviceInfo.DEVICE_PLAYBACK); + } + + @Test + public void hotplugDetectionActionClearsDevices_AudioSystem() { + mHdmiControlService.getHdmiCecNetwork().clearDeviceList(); + assertThat(mHdmiControlService.getHdmiCecNetwork().getDeviceInfoList(false)) + .isEmpty(); + // Add a device to the network and assert that this device is included in the list of + // devices. + HdmiDeviceInfo infoAudioSystem = HdmiDeviceInfo.cecDeviceBuilder() + .setLogicalAddress(ADDR_AUDIO_SYSTEM) + .setPhysicalAddress(0x1000) + .setPortId(PORT_1) + .setDeviceType(HdmiDeviceInfo.DEVICE_AUDIO_SYSTEM) + .setVendorId(0x1000) + .setDisplayName("Audio System") + .build(); + mHdmiControlService.getHdmiCecNetwork().addCecDevice(infoAudioSystem); + mTestLooper.dispatchAll(); + assertThat(mHdmiControlService.getHdmiCecNetwork().getDeviceInfoList(false)) + .hasSize(1); + mDeviceEventListeners.clear(); + assertThat(mDeviceEventListeners.size()).isEqualTo(0); + + // HAL detects a hotplug out. Assert that this device stays in the list of devices. + mHdmiControlService.onHotplug(PORT_1, false); + assertThat(mHdmiControlService.getHdmiCecNetwork().getDeviceInfoList(false)) + .hasSize(1); + assertThat(mDeviceEventListeners).isEmpty(); + mTestLooper.dispatchAll(); + // Make the device not acknowledge the poll message sent by the HotplugDetectionAction. + // Assert that this device is removed from the list of devices. + mNativeWrapper.setPollAddressResponse(ADDR_AUDIO_SYSTEM, SendMessageResult.NACK); + for (int pollCount = 0; pollCount < HotplugDetectionAction.TIMEOUT_COUNT; pollCount++) { + mTestLooper.moveTimeForward(HotplugDetectionAction.POLLING_INTERVAL_MS_FOR_TV); + mTestLooper.dispatchAll(); + } + + assertThat(mHdmiControlService.getHdmiCecNetwork().getDeviceInfoList(false)) + .isEmpty(); + assertThat(mDeviceEventListeners.size()).isEqualTo(1); + assertThat(mDeviceEventListeners.get(0).getStatus()) + .isEqualTo(HdmiControlManager.DEVICE_EVENT_REMOVE_DEVICE); + HdmiDeviceInfo removedDeviceInfo = mDeviceEventListeners.get(0).getDeviceInfo(); + assertThat(removedDeviceInfo.getPortId()).isEqualTo(PORT_1); + assertThat(removedDeviceInfo.getLogicalAddress()).isEqualTo(Constants.ADDR_AUDIO_SYSTEM); + assertThat(removedDeviceInfo.getPhysicalAddress()).isEqualTo(0x1000); + assertThat(removedDeviceInfo.getDeviceType()).isEqualTo(HdmiDeviceInfo.DEVICE_AUDIO_SYSTEM); + } }