Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 1d67b02d authored by Yan Han's avatar Yan Han
Browse files

Fix ambiguous callback result for HdmiControlService#queryDisplayStatus 

queryDisplayStatus's callback result uses values from two sets of
constants: result, and power status.

Issue: These sets overlap, which can cause the caller to think a CEC
device is not connected even though it reported its power status.
(Example: RESULT_SOURCE_NOT_AVAILABLE == POWER_STATUS_TRANSIENT_TO_ON)

This CL abandons the use of result constants, instead lumping them all
under POWER_STATUS_UNKNOWN, for the following reasons:
1. Internal callers of queryDisplayStatus do not distinguish between
the various result constants in their callback logic.
2. External callers of the HdmiPlaybackClient#queryDisplayStatus API
not recognize the result constants. According to the documentation for
HdmiPlaybackClient#DisplayStatusCallback, the API should only return
power status constants, not result constants.

Bug: 208757839
Test: atest HdmiControlServiceTest

Change-Id: I66409b603e81dcb3ab31954d62885570abf647ca
parent 73e564fe

services/core/java/com/android/server/hdmi/HdmiCecLocalDeviceSource.java

+1 −1
Original line number Original line Diff line number Diff line
@@ -82,7 +82,7 @@ abstract class HdmiCecLocalDeviceSource extends HdmiCecLocalDevice { 
                callback);

                callback);

        if (action == null) {

        if (action == null) {

            Slog.w(TAG, "Cannot initiate queryDisplayStatus");

            Slog.w(TAG, "Cannot initiate queryDisplayStatus");

            invokeCallback(callback, HdmiControlManager.RESULT_EXCEPTION);

            invokeCallback(callback, HdmiControlManager.POWER_STATUS_UNKNOWN);

            return;

            return;

        }

        }

        addAndStartAction(action);

        addAndStartAction(action);

services/core/java/com/android/server/hdmi/HdmiControlService.java

+11 −8
Original line number Original line Diff line number Diff line
@@ -2724,6 +2724,15 @@ public class HdmiControlService extends SystemService { 
        return mIsCecAvailable;

        return mIsCecAvailable;

    }

    }





    /**

     * Queries the display status of the TV and calls {@code callback} upon completion.

     *

     * If this is a non-source device, or if the query fails for any reason, the callback will

     * be called with {@link HdmiControlManager.POWER_STATUS_UNKNOWN}.

     *

     * If the query succeeds, the callback will be called with one of the other power status

     * constants.

     */

    @ServiceThreadOnly

    @ServiceThreadOnly

    protected void queryDisplayStatus(final IHdmiControlCallback callback) {

    protected void queryDisplayStatus(final IHdmiControlCallback callback) {

        assertRunOnServiceThread();

        assertRunOnServiceThread();
@@ -2741,7 +2750,7 @@ public class HdmiControlService extends SystemService { 




        if (source == null) {

        if (source == null) {

            Slog.w(TAG, "Local source device not available");

            Slog.w(TAG, "Local source device not available");

            invokeCallback(callback, HdmiControlManager.RESULT_SOURCE_NOT_AVAILABLE);

            invokeCallback(callback, HdmiControlManager.POWER_STATUS_UNKNOWN);

            return;

            return;

        }

        }

        source.queryDisplayStatus(callback);

        source.queryDisplayStatus(callback);
@@ -3110,13 +3119,7 @@ public class HdmiControlService extends SystemService { 
        if (isEnabled == HdmiControlManager.HDMI_CEC_CONTROL_ENABLED) {

        if (isEnabled == HdmiControlManager.HDMI_CEC_CONTROL_ENABLED) {

            queryDisplayStatus(new IHdmiControlCallback.Stub() {

            queryDisplayStatus(new IHdmiControlCallback.Stub() {

                public void onComplete(int status) {

                public void onComplete(int status) {

                    if (status == HdmiControlManager.POWER_STATUS_UNKNOWN

                    mIsCecAvailable = status != HdmiControlManager.POWER_STATUS_UNKNOWN;

                            || status == HdmiControlManager.RESULT_EXCEPTION

                            || status == HdmiControlManager.RESULT_SOURCE_NOT_AVAILABLE) {

                        mIsCecAvailable = false;

                    } else {

                        mIsCecAvailable = true;

                    }

                    if (!listeners.isEmpty()) {

                    if (!listeners.isEmpty()) {

                        invokeHdmiControlStatusChangeListenerLocked(listeners,

                        invokeHdmiControlStatusChangeListenerLocked(listeners,

                                isEnabled, mIsCecAvailable);

                                isEnabled, mIsCecAvailable);

services/tests/servicestests/src/com/android/server/hdmi/HdmiControlServiceTest.java

+46 −0
Original line number Original line Diff line number Diff line
@@ -668,6 +668,52 @@ public class HdmiControlServiceTest { 
        assertThat(hdmiControlStatusCallback.mCecAvailable).isTrue();

        assertThat(hdmiControlStatusCallback.mCecAvailable).isTrue();

    }

    }





    @Test

    public void initCec_statusListener_CecEnabled_CecAvailable_TvTransientToOn() {

        HdmiControlStatusCallback hdmiControlStatusCallback = new HdmiControlStatusCallback();

        mHdmiControlServiceSpy.setControlEnabled(HdmiControlManager.HDMI_CEC_CONTROL_DISABLED);

        mTestLooper.dispatchAll();



        mHdmiControlServiceSpy.addHdmiControlStatusChangeListener(hdmiControlStatusCallback);

        mHdmiControlServiceSpy.setControlEnabled(HdmiControlManager.HDMI_CEC_CONTROL_ENABLED);

        mHdmiControlServiceSpy.allocateLogicalAddress(mLocalDevices, INITIATED_BY_ENABLE_CEC);

        mTestLooper.dispatchAll();



        HdmiCecMessage reportPowerStatus =

                HdmiCecMessageBuilder.buildReportPowerStatus(

                        Constants.ADDR_TV,

                        mHdmiControlServiceSpy.playback().getDeviceInfo().getLogicalAddress(),

                        HdmiControlManager.POWER_STATUS_TRANSIENT_TO_ON);

        mNativeWrapper.onCecMessage(reportPowerStatus);

        mTestLooper.dispatchAll();



        assertThat(hdmiControlStatusCallback.mCecEnabled).isTrue();

        assertThat(hdmiControlStatusCallback.mCecAvailable).isTrue();

    }



    @Test

    public void initCec_statusListener_CecEnabled_CecAvailable_TvTransientToStandby() {

        HdmiControlStatusCallback hdmiControlStatusCallback = new HdmiControlStatusCallback();

        mHdmiControlServiceSpy.setControlEnabled(HdmiControlManager.HDMI_CEC_CONTROL_DISABLED);

        mTestLooper.dispatchAll();



        mHdmiControlServiceSpy.addHdmiControlStatusChangeListener(hdmiControlStatusCallback);

        mHdmiControlServiceSpy.setControlEnabled(HdmiControlManager.HDMI_CEC_CONTROL_ENABLED);

        mHdmiControlServiceSpy.allocateLogicalAddress(mLocalDevices, INITIATED_BY_ENABLE_CEC);

        mTestLooper.dispatchAll();



        HdmiCecMessage reportPowerStatus =

                HdmiCecMessageBuilder.buildReportPowerStatus(

                        Constants.ADDR_TV,

                        mHdmiControlServiceSpy.playback().getDeviceInfo().getLogicalAddress(),

                        HdmiControlManager.POWER_STATUS_TRANSIENT_TO_STANDBY);

        mNativeWrapper.onCecMessage(reportPowerStatus);

        mTestLooper.dispatchAll();



        assertThat(hdmiControlStatusCallback.mCecEnabled).isTrue();

        assertThat(hdmiControlStatusCallback.mCecAvailable).isTrue();

    }



    @Test

    @Test

    public void handleCecCommand_errorParameter_returnsAbortInvalidOperand() {

    public void handleCecCommand_errorParameter_returnsAbortInvalidOperand() {

        // Validity ERROR_PARAMETER. Taken from HdmiCecMessageValidatorTest#isValid_menuStatus

        // Validity ERROR_PARAMETER. Taken from HdmiCecMessageValidatorTest#isValid_menuStatus