From 45c5ad3bec4dfccca0e75b289f1e21c0478d5ee7 Mon Sep 17 00:00:00 2001 From: Marvin Ramin Date: Fri, 24 Jul 2020 12:43:04 +0200 Subject: [PATCH] Fix issue with internal Active Source state tracking Only set the internal Active Source device after the message has been sent. This prevents an issue where the internal state of a Source device gets set with an incorrect logical address while address allocation is still in progress. This should in turn resolve issues with source devices not sending Standby messages as they might believe they are not active source (due to invalid LA). Bug: 158441116 Test: atest HdmiCecLocalDevicePlaybackTest Change-Id: Ibd78925a8bdc826341068c89e7f621f58dd41933 --- .../server/hdmi/ActiveSourceAction.java | 10 +++++++--- .../server/hdmi/HdmiControlService.java | 4 ---- .../server/hdmi/ActiveSourceActionTest.java | 20 +++++++++++++++++++ .../hdmi/HdmiCecLocalDevicePlaybackTest.java | 15 ++++++++++++++ 4 files changed, 42 insertions(+), 7 deletions(-) diff --git a/services/core/java/com/android/server/hdmi/ActiveSourceAction.java b/services/core/java/com/android/server/hdmi/ActiveSourceAction.java index 3c4dae0f6467c..c90f297e1485e 100644 --- a/services/core/java/com/android/server/hdmi/ActiveSourceAction.java +++ b/services/core/java/com/android/server/hdmi/ActiveSourceAction.java @@ -39,15 +39,19 @@ public class ActiveSourceAction extends HdmiCecFeatureAction { @Override boolean start() { mState = STATE_STARTED; - sendCommand(HdmiCecMessageBuilder.buildActiveSource(getSourceAddress(), - source().mService.getPhysicalAddress())); + int logicalAddress = getSourceAddress(); + int physicalAddress = getSourcePath(); + + sendCommand(HdmiCecMessageBuilder.buildActiveSource(logicalAddress, physicalAddress)); if (source().getType() == HdmiDeviceInfo.DEVICE_PLAYBACK) { // Reports menu-status active to receive . sendCommand( - HdmiCecMessageBuilder.buildReportMenuStatus(getSourceAddress(), mDestination, + HdmiCecMessageBuilder.buildReportMenuStatus(logicalAddress, mDestination, Constants.MENU_STATE_ACTIVATED)); } + + source().setActiveSource(logicalAddress, physicalAddress); mState = STATE_FINISHED; finish(); return true; diff --git a/services/core/java/com/android/server/hdmi/HdmiControlService.java b/services/core/java/com/android/server/hdmi/HdmiControlService.java index 30d7d5494a7e2..c55c1ed850bb6 100644 --- a/services/core/java/com/android/server/hdmi/HdmiControlService.java +++ b/services/core/java/com/android/server/hdmi/HdmiControlService.java @@ -3244,7 +3244,6 @@ public class HdmiControlService extends SystemService { playback.setIsActiveSource(true); playback.wakeUpIfActiveSource(); playback.maySendActiveSource(source); - setActiveSource(playback.mAddress, physicalAddress); } if (deviceType == HdmiDeviceInfo.DEVICE_AUDIO_SYSTEM) { @@ -3255,7 +3254,6 @@ public class HdmiControlService extends SystemService { audioSystem.setIsActiveSource(true); audioSystem.wakeUpIfActiveSource(); audioSystem.maySendActiveSource(source); - setActiveSource(audioSystem.mAddress, physicalAddress); } } } @@ -3278,13 +3276,11 @@ public class HdmiControlService extends SystemService { if (audioSystem != null) { audioSystem.setIsActiveSource(false); } - setActiveSource(playback.mAddress, physicalAddress); } else { if (audioSystem != null) { audioSystem.setIsActiveSource(true); audioSystem.wakeUpIfActiveSource(); audioSystem.maySendActiveSource(sourceAddress); - setActiveSource(audioSystem.mAddress, physicalAddress); } } } diff --git a/services/tests/servicestests/src/com/android/server/hdmi/ActiveSourceActionTest.java b/services/tests/servicestests/src/com/android/server/hdmi/ActiveSourceActionTest.java index 50086affcbc56..870a27417cd23 100644 --- a/services/tests/servicestests/src/com/android/server/hdmi/ActiveSourceActionTest.java +++ b/services/tests/servicestests/src/com/android/server/hdmi/ActiveSourceActionTest.java @@ -143,6 +143,26 @@ public class ActiveSourceActionTest { assertThat(mNativeWrapper.getResultMessages()).contains(menuStatus); } + @Test + public void playbackDevice_updatesActiveSourceState() { + HdmiCecLocalDevicePlayback playbackDevice = new HdmiCecLocalDevicePlayback( + mHdmiControlService); + playbackDevice.init(); + mLocalDevices.add(playbackDevice); + mHdmiControlService.allocateLogicalAddress(mLocalDevices, INITIATED_BY_ENABLE_CEC); + mTestLooper.dispatchAll(); + + HdmiCecFeatureAction action = new com.android.server.hdmi.ActiveSourceAction( + playbackDevice, ADDR_TV); + playbackDevice.addAndStartAction(action); + mTestLooper.dispatchAll(); + + assertThat(playbackDevice.getActiveSource().logicalAddress).isEqualTo( + playbackDevice.mAddress); + assertThat(playbackDevice.getActiveSource().physicalAddress).isEqualTo(mPhysicalAddress); + assertThat(playbackDevice.mIsActiveSource).isTrue(); + } + @Test public void audioDevice_sendsActiveSource_noMenuStatus() { HdmiCecLocalDeviceAudioSystem audioDevice = new HdmiCecLocalDeviceAudioSystem( 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 35f9ca5cce681..7d3f81c6af357 100644 --- a/services/tests/servicestests/src/com/android/server/hdmi/HdmiCecLocalDevicePlaybackTest.java +++ b/services/tests/servicestests/src/com/android/server/hdmi/HdmiCecLocalDevicePlaybackTest.java @@ -526,4 +526,19 @@ public class HdmiCecLocalDevicePlaybackTest { assertThat(mNativeWrapper.getResultMessages()).contains(activeSource); } + + @Test + public void handleSetStreamPath_afterHotplug_hasCorrectActiveSource() { + mHdmiControlService.onHotplug(1, false); + mHdmiControlService.onHotplug(1, true); + + HdmiCecMessage setStreamPath = HdmiCecMessageBuilder.buildSetStreamPath(ADDR_TV, + mPlaybackPhysicalAddress); + mHdmiCecLocalDevicePlayback.dispatchMessage(setStreamPath); + mTestLooper.dispatchAll(); + + assertThat(mHdmiCecLocalDevicePlayback.getActiveSource().logicalAddress).isEqualTo( + mHdmiCecLocalDevicePlayback.getDeviceInfo().getLogicalAddress()); + assertThat(mHdmiCecLocalDevicePlayback.mIsActiveSource).isTrue(); + } }