From 307ec6b663d1c928ce563b1db835352e2b4a77c8 Mon Sep 17 00:00:00 2001 From: Paul Colta Date: Tue, 4 Jul 2023 15:53:15 +0200 Subject: [PATCH] HDMI: Improve logic for sending and resending source changing messages Rename SendCecCommandAction to ResendCecCommandAction. The action is now only created if the first attempt to send the message fails. Use the action when sending a source changing message together with a callback. Test: atest com.android.server.hdmi && atest android.hdmicec.cts.playback.HdmiCecRoutingControlTest#cect_11_2_2_4_InactiveSourceOnStandby Bug: 289470244 Change-Id: Ib1b124751b9dbdd9ce3d2d1496c5451dc48b0309 --- .../server/hdmi/HdmiCecFeatureAction.java | 5 ++ .../server/hdmi/HdmiCecLocalDevice.java | 7 ++ .../server/hdmi/HdmiControlService.java | 79 +++++++++------- ...ction.java => ResendCecCommandAction.java} | 34 ++++--- ...> ResendCecCommandActionPlaybackTest.java} | 90 ++++++++++++++++++- ...java => ResendCecCommandActionTvTest.java} | 6 +- 6 files changed, 171 insertions(+), 50 deletions(-) rename services/core/java/com/android/server/hdmi/{SendCecCommandAction.java => ResendCecCommandAction.java} (62%) rename services/tests/servicestests/src/com/android/server/hdmi/{SendCecCommandActionPlaybackTest.java => ResendCecCommandActionPlaybackTest.java} (74%) rename services/tests/servicestests/src/com/android/server/hdmi/{SendCecCommandActionTvTest.java => ResendCecCommandActionTvTest.java} (98%) diff --git a/services/core/java/com/android/server/hdmi/HdmiCecFeatureAction.java b/services/core/java/com/android/server/hdmi/HdmiCecFeatureAction.java index 2e1ff038931d2..5db114b9c2eb9 100644 --- a/services/core/java/com/android/server/hdmi/HdmiCecFeatureAction.java +++ b/services/core/java/com/android/server/hdmi/HdmiCecFeatureAction.java @@ -191,6 +191,11 @@ abstract class HdmiCecFeatureAction { mService.sendCecCommand(cmd, callback); } + protected final void sendCommandWithoutRetries(HdmiCecMessage cmd, + HdmiControlService.SendMessageCallback callback) { + mService.sendCecCommandWithoutRetries(cmd, callback); + } + protected final void addAndStartAction(HdmiCecFeatureAction action) { mSource.addAndStartAction(action); } diff --git a/services/core/java/com/android/server/hdmi/HdmiCecLocalDevice.java b/services/core/java/com/android/server/hdmi/HdmiCecLocalDevice.java index b5e819511ed4d..207d38eb17ac9 100644 --- a/services/core/java/com/android/server/hdmi/HdmiCecLocalDevice.java +++ b/services/core/java/com/android/server/hdmi/HdmiCecLocalDevice.java @@ -1013,6 +1013,12 @@ abstract class HdmiCecLocalDevice extends HdmiLocalDevice { assertRunOnServiceThread(); mActions.add(action); if (mService.isPowerStandby() || !mService.isAddressAllocated()) { + if (action.getClass() == ResendCecCommandAction.class) { + Slog.i(TAG, "Not ready to start ResendCecCommandAction. " + + "This action is cancelled."); + removeAction(action); + return; + } Slog.i(TAG, "Not ready to start action. Queued for deferred start:" + action); return; } @@ -1295,6 +1301,7 @@ abstract class HdmiCecLocalDevice extends HdmiLocalDevice { removeAction(AbsoluteVolumeAudioStatusAction.class); removeAction(SetAudioVolumeLevelDiscoveryAction.class); removeAction(ActiveSourceAction.class); + removeAction(ResendCecCommandAction.class); mPendingActionClearedCallback = new PendingActionClearedCallback() { diff --git a/services/core/java/com/android/server/hdmi/HdmiControlService.java b/services/core/java/com/android/server/hdmi/HdmiControlService.java index 91cf50319bcab..a7e78be0578b9 100644 --- a/services/core/java/com/android/server/hdmi/HdmiControlService.java +++ b/services/core/java/com/android/server/hdmi/HdmiControlService.java @@ -1518,6 +1518,51 @@ public class HdmiControlService extends SystemService { } } + @ServiceThreadOnly + void sendCecCommand(HdmiCecMessage command) { + sendCecCommand(command, null); + } + + @ServiceThreadOnly + void sendCecCommand(HdmiCecMessage command, @Nullable SendMessageCallback callback) { + switch (command.getOpcode()) { + case Constants.MESSAGE_ACTIVE_SOURCE: + case Constants.MESSAGE_IMAGE_VIEW_ON: + case Constants.MESSAGE_INACTIVE_SOURCE: + case Constants.MESSAGE_ROUTING_CHANGE: + case Constants.MESSAGE_SET_STREAM_PATH: + case Constants.MESSAGE_TEXT_VIEW_ON: + sendCecCommandWithRetries(command, callback); + break; + default: + sendCecCommandWithoutRetries(command, callback); + } + } + + /** + * Create a {@link ResendCecCommandAction} that will retry sending the CEC message if it fails. + * @param command command to be sent on the CEC bus. + * @param callback callback for handling the result of sending the command. + */ + @ServiceThreadOnly + private void sendCecCommandWithRetries(HdmiCecMessage command, + @Nullable SendMessageCallback callback) { + assertRunOnServiceThread(); + HdmiCecLocalDevice localDevice = getAllCecLocalDevices().get(0); + if (localDevice != null) { + sendCecCommandWithoutRetries(command, new SendMessageCallback() { + @Override + public void onSendCompleted(int result) { + if (result != SendMessageResult.SUCCESS) { + localDevice.addAndStartAction(new + ResendCecCommandAction(localDevice, command, callback)); + } + } + }); + } + } + + /** * Transmit a CEC command to CEC bus. * @@ -1525,7 +1570,8 @@ public class HdmiControlService extends SystemService { * @param callback interface used to the result of send command */ @ServiceThreadOnly - void sendCecCommand(HdmiCecMessage command, @Nullable SendMessageCallback callback) { + void sendCecCommandWithoutRetries(HdmiCecMessage command, + @Nullable SendMessageCallback callback) { assertRunOnServiceThread(); if (command.getValidationResult() == HdmiCecMessageValidator.OK && verifyPhysicalAddresses(command)) { @@ -1538,37 +1584,6 @@ public class HdmiControlService extends SystemService { } } - @ServiceThreadOnly - void sendCecCommand(HdmiCecMessage command) { - assertRunOnServiceThread(); - switch (command.getOpcode()) { - case Constants.MESSAGE_ACTIVE_SOURCE: - case Constants.MESSAGE_IMAGE_VIEW_ON: - case Constants.MESSAGE_INACTIVE_SOURCE: - case Constants.MESSAGE_ROUTING_CHANGE: - case Constants.MESSAGE_SET_STREAM_PATH: - case Constants.MESSAGE_TEXT_VIEW_ON: - sendCecCommandWithRetries(command); - break; - default: - sendCecCommand(command, null); - } - } - - /** - * Create a {@link SendCecCommandAction} that will retry to send the CEC message in case it - * fails. - * @param command that has to be sent in the CEC bus. - */ - @ServiceThreadOnly - public void sendCecCommandWithRetries(HdmiCecMessage command) { - assertRunOnServiceThread(); - HdmiCecLocalDevice localDevice = getAllCecLocalDevices().get(0); - if (localDevice != null) { - localDevice.addAndStartAction(new SendCecCommandAction(localDevice, command)); - } - } - /** * Send command on the given CEC message if possible. * If the aborted message is invalid, then it wont send the message. diff --git a/services/core/java/com/android/server/hdmi/SendCecCommandAction.java b/services/core/java/com/android/server/hdmi/ResendCecCommandAction.java similarity index 62% rename from services/core/java/com/android/server/hdmi/SendCecCommandAction.java rename to services/core/java/com/android/server/hdmi/ResendCecCommandAction.java index 000ada89f38e8..b130c46642221 100644 --- a/services/core/java/com/android/server/hdmi/SendCecCommandAction.java +++ b/services/core/java/com/android/server/hdmi/ResendCecCommandAction.java @@ -16,23 +16,29 @@ package com.android.server.hdmi; -import android.hardware.tv.cec.V1_0.SendMessageResult; +import android.hardware.tv.hdmi.cec.SendMessageResult; import android.util.Slog; import com.android.server.hdmi.HdmiControlService.SendMessageCallback; /** - * Action that sends a CEC command. If the message fails to be sent, it tries again for - * RETRANSMISSION_COUNT times. + * Action that retries RETRANSMISSION_COUNT times to send a CEC command when the first attempt to + * send the message failed. + * + * This action starts with a delay of SEND_COMMAND_RETRY_MS milliseconds. + * + * If this action can't be started it will be canceled and not deferred. + * See {@link HdmiCecLocalDevice#addAndStartAction}. */ -public class SendCecCommandAction extends HdmiCecFeatureAction { - private static final String TAG = "SendCecCommandAction"; - private static final int RETRANSMISSION_COUNT = 2; +public class ResendCecCommandAction extends HdmiCecFeatureAction { + private static final String TAG = "ResendCecCommandAction"; + private static final int RETRANSMISSION_COUNT = 1; private static final int STATE_WAIT_FOR_RESEND_COMMAND = 1; static final int SEND_COMMAND_RETRY_MS = 300; private final HdmiCecMessage mCommand; private int mRetransmissionCount = 0; + private final SendMessageCallback mResultCallback; private final SendMessageCallback mCallback = new SendMessageCallback(){ @Override public void onSendCompleted(int result) { @@ -41,20 +47,26 @@ public class SendCecCommandAction extends HdmiCecFeatureAction { mState = STATE_WAIT_FOR_RESEND_COMMAND; addTimer(mState, SEND_COMMAND_RETRY_MS); } else { + if (mResultCallback != null) { + mResultCallback.onSendCompleted(result); + } finish(); } } }; - SendCecCommandAction(HdmiCecLocalDevice source, HdmiCecMessage command) { + ResendCecCommandAction(HdmiCecLocalDevice source, HdmiCecMessage command, + SendMessageCallback callback) { super(source); mCommand = command; + mResultCallback = callback; + mState = STATE_WAIT_FOR_RESEND_COMMAND; + addTimer(mState, SEND_COMMAND_RETRY_MS); } @Override boolean start() { - Slog.d(TAG, "SendCecCommandAction started"); - sendCommand(mCommand, mCallback); + Slog.d(TAG, "ResendCecCommandAction started"); return true; } @@ -66,8 +78,8 @@ public class SendCecCommandAction extends HdmiCecFeatureAction { return; } if (mState == STATE_WAIT_FOR_RESEND_COMMAND) { - Slog.d(TAG, "sendCecCommand failed, retry"); - sendCommand(mCommand, mCallback); + Slog.d(TAG, "sendCommandWithoutRetries failed, retry"); + sendCommandWithoutRetries(mCommand, mCallback); } } diff --git a/services/tests/servicestests/src/com/android/server/hdmi/SendCecCommandActionPlaybackTest.java b/services/tests/servicestests/src/com/android/server/hdmi/ResendCecCommandActionPlaybackTest.java similarity index 74% rename from services/tests/servicestests/src/com/android/server/hdmi/SendCecCommandActionPlaybackTest.java rename to services/tests/servicestests/src/com/android/server/hdmi/ResendCecCommandActionPlaybackTest.java index 9022b18f99533..061e1f90fa58f 100644 --- a/services/tests/servicestests/src/com/android/server/hdmi/SendCecCommandActionPlaybackTest.java +++ b/services/tests/servicestests/src/com/android/server/hdmi/ResendCecCommandActionPlaybackTest.java @@ -17,11 +17,12 @@ package com.android.server.hdmi; import static com.android.server.SystemService.PHASE_SYSTEM_SERVICES_READY; -import static com.android.server.hdmi.SendCecCommandAction.SEND_COMMAND_RETRY_MS; +import static com.android.server.hdmi.ResendCecCommandAction.SEND_COMMAND_RETRY_MS; import static com.google.common.truth.Truth.assertThat; import android.content.Context; +import android.hardware.hdmi.HdmiControlManager; import android.hardware.hdmi.HdmiDeviceInfo; import android.hardware.tv.cec.V1_0.SendMessageResult; import android.os.Looper; @@ -38,11 +39,11 @@ import org.junit.runners.JUnit4; import java.util.Collections; -/** Tests for {@link SendCecCommandAction} */ +/** Tests for {@link ResendCecCommandAction} */ @SmallTest @Presubmit @RunWith(JUnit4.class) -public class SendCecCommandActionPlaybackTest { +public class ResendCecCommandActionPlaybackTest { private static final String TAG = "SendCecCommandActionPlaybackTest"; private HdmiControlService mHdmiControlService; private HdmiCecLocalDevice mPlaybackDevice; @@ -51,6 +52,7 @@ public class SendCecCommandActionPlaybackTest { private Looper mMyLooper; private TestLooper mTestLooper = new TestLooper(); private int mPhysicalAddress; + private boolean mIsPowerStandby; @Before public void setUp() throws Exception { @@ -63,7 +65,7 @@ public class SendCecCommandActionPlaybackTest { @Override boolean isPowerStandby() { - return false; + return mIsPowerStandby; } @Override @@ -71,6 +73,7 @@ public class SendCecCommandActionPlaybackTest { // do nothing } }; + mIsPowerStandby = false; mMyLooper = mTestLooper.getLooper(); mHdmiControlService.setIoLooper(mMyLooper); @@ -148,6 +151,35 @@ public class SendCecCommandActionPlaybackTest { assertThat(mNativeWrapper.getResultMessages()).doesNotContain(inactiveSourceMessage); } + @Test + public void sendCecCommand_inactiveSource_onStandby_powerControlModeNone_sendMessage() { + mPlaybackDevice.mService.getHdmiCecConfig().setStringValue( + HdmiControlManager.CEC_SETTING_NAME_POWER_CONTROL_MODE, + HdmiControlManager.POWER_CONTROL_MODE_NONE); + mPlaybackDevice.setActiveSource(mPlaybackDevice.getDeviceInfo().getLogicalAddress(), + mPhysicalAddress, "SendCecCommandActionPlaybackTest"); + mIsPowerStandby = true; + mPlaybackDevice.onStandby(false, HdmiControlService.STANDBY_SCREEN_OFF); + mTestLooper.dispatchAll(); + HdmiCecMessage inactiveSourceMessage = HdmiCecMessageBuilder.buildInactiveSource( + mPlaybackDevice.getDeviceInfo().getLogicalAddress(), mPhysicalAddress); + + assertThat(mNativeWrapper.getResultMessages()).contains(inactiveSourceMessage); + } + + @Test + public void sendCecCommand_inactiveSource_onStandby_initiatedByCec_sendMessage() { + mPlaybackDevice.setActiveSource(mPlaybackDevice.getDeviceInfo().getLogicalAddress(), + mPhysicalAddress, "SendCecCommandActionPlaybackTest"); + mIsPowerStandby = true; + mPlaybackDevice.onStandby(true, HdmiControlService.STANDBY_SCREEN_OFF); + mTestLooper.dispatchAll(); + HdmiCecMessage inactiveSourceMessage = HdmiCecMessageBuilder.buildInactiveSource( + mPlaybackDevice.getDeviceInfo().getLogicalAddress(), mPhysicalAddress); + + assertThat(mNativeWrapper.getResultMessages()).contains(inactiveSourceMessage); + } + @Test public void sendCecCommand_routingChange_sendMessageFails_resendMessage() { mNativeWrapper.setMessageSendResult(Constants.MESSAGE_ROUTING_CHANGE, @@ -293,4 +325,54 @@ public class SendCecCommandActionPlaybackTest { mTestLooper.dispatchAll(); assertThat(mNativeWrapper.getResultMessages()).doesNotContain(reportPhysicalAddress); } + + @Test + public void sendCecCommand_inactiveSource_sendMessageFails_afterStandby_noResendMessage() { + mNativeWrapper.setMessageSendResult(Constants.MESSAGE_INACTIVE_SOURCE, + SendMessageResult.BUSY); + mPlaybackDevice.setActiveSource(mPlaybackDevice.getDeviceInfo().getLogicalAddress(), + mPhysicalAddress, "SendCecCommandActionPlaybackTest"); + mIsPowerStandby = true; + mPlaybackDevice.onStandby(true, HdmiControlService.STANDBY_SCREEN_OFF); + mTestLooper.dispatchAll(); + HdmiCecMessage inactiveSourceMessage = HdmiCecMessageBuilder.buildInactiveSource( + mPlaybackDevice.getDeviceInfo().getLogicalAddress(), mPhysicalAddress); + assertThat(mNativeWrapper.getResultMessages()).contains(inactiveSourceMessage); + + mNativeWrapper.clearResultMessages(); + mHdmiControlService.onWakeUp(HdmiControlService.WAKE_UP_SCREEN_ON); + mTestLooper.dispatchAll(); + assertThat(mNativeWrapper.getResultMessages()).doesNotContain(inactiveSourceMessage); + + mTestLooper.moveTimeForward(SEND_COMMAND_RETRY_MS); + mTestLooper.dispatchAll(); + assertThat(mNativeWrapper.getResultMessages()).doesNotContain(inactiveSourceMessage); + } + + @Test + public void sendCecCommand_onStandby_removeAction_noResendMessage() { + mNativeWrapper.setMessageSendResult(Constants.MESSAGE_INACTIVE_SOURCE, + SendMessageResult.BUSY); + mTestLooper.dispatchAll(); + HdmiCecMessage inactiveSourceMessage = HdmiCecMessageBuilder.buildInactiveSource( + mPlaybackDevice.getDeviceInfo().getLogicalAddress(), mPhysicalAddress); + mHdmiControlService.sendCecCommand(inactiveSourceMessage); + mTestLooper.dispatchAll(); + assertThat(mNativeWrapper.getResultMessages()).contains(inactiveSourceMessage); + mHdmiControlService.onStandby(HdmiControlService.STANDBY_SCREEN_OFF); + + mNativeWrapper.clearResultMessages(); + mTestLooper.moveTimeForward(SEND_COMMAND_RETRY_MS); + mTestLooper.dispatchAll(); + assertThat(mNativeWrapper.getResultMessages()).doesNotContain(inactiveSourceMessage); + + mTestLooper.dispatchAll(); + mHdmiControlService.onWakeUp(HdmiControlService.WAKE_UP_SCREEN_ON); + mTestLooper.dispatchAll(); + assertThat(mNativeWrapper.getResultMessages()).doesNotContain(inactiveSourceMessage); + + mTestLooper.moveTimeForward(SEND_COMMAND_RETRY_MS); + mTestLooper.dispatchAll(); + assertThat(mNativeWrapper.getResultMessages()).doesNotContain(inactiveSourceMessage); + } } diff --git a/services/tests/servicestests/src/com/android/server/hdmi/SendCecCommandActionTvTest.java b/services/tests/servicestests/src/com/android/server/hdmi/ResendCecCommandActionTvTest.java similarity index 98% rename from services/tests/servicestests/src/com/android/server/hdmi/SendCecCommandActionTvTest.java rename to services/tests/servicestests/src/com/android/server/hdmi/ResendCecCommandActionTvTest.java index aae839810a0e0..b25ea2c5078c7 100644 --- a/services/tests/servicestests/src/com/android/server/hdmi/SendCecCommandActionTvTest.java +++ b/services/tests/servicestests/src/com/android/server/hdmi/ResendCecCommandActionTvTest.java @@ -17,7 +17,7 @@ package com.android.server.hdmi; import static com.android.server.SystemService.PHASE_SYSTEM_SERVICES_READY; -import static com.android.server.hdmi.SendCecCommandAction.SEND_COMMAND_RETRY_MS; +import static com.android.server.hdmi.ResendCecCommandAction.SEND_COMMAND_RETRY_MS; import static com.google.common.truth.Truth.assertThat; @@ -38,11 +38,11 @@ import org.junit.runners.JUnit4; import java.util.Collections; -/** Tests for {@link SendCecCommandAction} */ +/** Tests for {@link ResendCecCommandAction} */ @SmallTest @Presubmit @RunWith(JUnit4.class) -public class SendCecCommandActionTvTest { +public class ResendCecCommandActionTvTest { private static final String TAG = "SendCecCommandActionTvTest"; private HdmiControlService mHdmiControlService; private HdmiCecLocalDevice mTvDevice;