[edk2-devel] [PATCH] uefi-sct/SctPkg:Enhance BBTestReadKeyStrokeExFunctionAutoTestCheckpoint1()

Gao Jie posted 1 patch 2 years, 4 months ago
Failed in applying to current master (apply log)
.../BlackBoxTest/SimpleTextInputExBBTestFunction.c    | 11 ++++++++++-
.../BlackBoxTest/SimpleTextInputExBBTestFunction.c    | 11 ++++++++++-
2 files changed, 20 insertions(+), 2 deletions(-)
[edk2-devel] [PATCH] uefi-sct/SctPkg:Enhance BBTestReadKeyStrokeExFunctionAutoTestCheckpoint1()
Posted by Gao Jie 2 years, 4 months ago
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2386

Enhance BBTestReadKeyStrokeExFunctionAutoTestCheckpoint1()
to handle ReadKeyStrokeEx implementation which returns EFI_NOT_READY
but without touching KeyToggleState.

Signed-off-by: Barton Gao <gaojie@byosoft.com.cn>
---
 .../BlackBoxTest/SimpleTextInputExBBTestFunction.c    | 11 ++++++++++-
 .../BlackBoxTest/SimpleTextInputExBBTestFunction.c    | 11 ++++++++++-
 2 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/SimpleTextInputEx/BlackBoxTest/SimpleTextInputExBBTestFunction.c b/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/SimpleTextInputEx/BlackBoxTest/SimpleTextInputExBBTestFunction.c
index 0398bc26..c1669959 100644
--- a/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/SimpleTextInputEx/BlackBoxTest/SimpleTextInputExBBTestFunction.c
+++ b/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/SimpleTextInputEx/BlackBoxTest/SimpleTextInputExBBTestFunction.c
@@ -1160,6 +1160,12 @@ BBTestReadKeyStrokeExFunctionAutoTestCheckpoint1 (
   ValidState[5] = EFI_TOGGLE_STATE_VALID | EFI_KEY_STATE_EXPOSED | EFI_NUM_LOCK_ACTIVE | EFI_CAPS_LOCK_ACTIVE;
   ValidState[6] = EFI_TOGGLE_STATE_VALID | EFI_KEY_STATE_EXPOSED | EFI_SCROLL_LOCK_ACTIVE |EFI_NUM_LOCK_ACTIVE | EFI_CAPS_LOCK_ACTIVE;
 
+  //
+  // Set all bits to one (invalid values) for both KeyShiftState and KeyToggleState
+  //
+  Key.KeyState.KeyShiftState = 0xFFFFFFFF;
+  Key.KeyState.KeyToggleState = 0xFF;
+
   //
   //Read next keystroke from the input device
   //
@@ -1171,7 +1177,10 @@ BBTestReadKeyStrokeExFunctionAutoTestCheckpoint1 (
     return Status;
   }
 
-  if ((Key.KeyState.KeyToggleState & EFI_TOGGLE_STATE_VALID) == 0) {
+  if (((Key.KeyState.KeyToggleState & EFI_TOGGLE_STATE_VALID) == 0) || (Key.KeyState.KeyShiftState == 0xFFFFFFFF) || (Key.KeyState.KeyToggleState == 0xFF)) {
+    //
+    // Log the error here and return key states are not supported when high order bit of KeyToggleState is 0 or KeyState not touched
+    //
     return EFI_UNSUPPORTED;
   }
 
diff --git a/uefi-sct/SctPkg/TestCase/UEFI/IHV/Protocol/SimpleTextInputEx/BlackBoxTest/SimpleTextInputExBBTestFunction.c b/uefi-sct/SctPkg/TestCase/UEFI/IHV/Protocol/SimpleTextInputEx/BlackBoxTest/SimpleTextInputExBBTestFunction.c
index adbf3dcf..e20ed345 100644
--- a/uefi-sct/SctPkg/TestCase/UEFI/IHV/Protocol/SimpleTextInputEx/BlackBoxTest/SimpleTextInputExBBTestFunction.c
+++ b/uefi-sct/SctPkg/TestCase/UEFI/IHV/Protocol/SimpleTextInputEx/BlackBoxTest/SimpleTextInputExBBTestFunction.c
@@ -1160,6 +1160,12 @@ BBTestReadKeyStrokeExFunctionAutoTestCheckpoint1 (
   ValidState[5] = EFI_TOGGLE_STATE_VALID | EFI_KEY_STATE_EXPOSED | EFI_NUM_LOCK_ACTIVE | EFI_CAPS_LOCK_ACTIVE;
   ValidState[6] = EFI_TOGGLE_STATE_VALID | EFI_KEY_STATE_EXPOSED | EFI_SCROLL_LOCK_ACTIVE |EFI_NUM_LOCK_ACTIVE | EFI_CAPS_LOCK_ACTIVE;
 
+  //
+  // Set all bits to one (invalid values) for both KeyShiftState and KeyToggleState
+  //
+  Key.KeyState.KeyShiftState = 0xFFFFFFFF;
+  Key.KeyState.KeyToggleState = 0xFF;
+
   //
   //Read next keystroke from the input device
   //
@@ -1171,7 +1177,10 @@ BBTestReadKeyStrokeExFunctionAutoTestCheckpoint1 (
     return Status;
   }
 
-  if ((Key.KeyState.KeyToggleState & EFI_TOGGLE_STATE_VALID) == 0) {
+  if (((Key.KeyState.KeyToggleState & EFI_TOGGLE_STATE_VALID) == 0) || (Key.KeyState.KeyShiftState == 0xFFFFFFFF) || (Key.KeyState.KeyToggleState == 0xFF)) {
+    //
+    // Log the error here and return key states are not supported when high order bit of KeyToggleState is 0 or KeyState not touched
+    //
     return EFI_UNSUPPORTED;
   }
 
-- 
2.31.0.windows.1




-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#84011): https://edk2.groups.io/g/devel/message/84011
Mute This Topic: https://groups.io/mt/87274606/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-


Re: [edk2-devel] [PATCH] uefi-sct/SctPkg:Enhance BBTestReadKeyStrokeExFunctionAutoTestCheckpoint1()
Posted by Sunny Wang 2 years, 1 month ago
Sorry for the late review.

The change looks fine. However, I'm confused about the comment " Log the error here and return key states are not supported when high order bit of KeyToggleState is 0 or KeyState not touched". It looks like returning EFI_UNSUPPORTED won't log any error/failure in the result log. Did I overlook anything here?

Furthermore, it looks like UEFI spec implies that the ReadKeyStrokeEx should clear KeyState (set the value to zero) if there is no key data. There is also a code change for doing this in edk2. https://edk2.groups.io/g/devel/message/20311
Therefore, if I didn't overlook anything here, we should separately deal with the "(Key.KeyState.KeyShiftState == 0xFFFFFFFF) || (Key.KeyState.KeyToggleState == 0xFF)" as a failure rather than directly returning EFI_UNSUPPORTED.

Barton and Edhaya, what do you guys think?

Best Regards,
Sunny
-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Gao Jie via groups.io
Sent: 24 November 2021 03:16
To: devel@edk2.groups.io
Cc: G Edhaya Chandran <Edhaya.Chandran@arm.com>; Carolyn.Gjertsen@amd.com; Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@arm.com>; eric.jin@intel.com; arvinx.chen@intel.com; Supreeth.Venkatesh@amd.com
Subject: [edk2-devel] [PATCH] uefi-sct/SctPkg:Enhance BBTestReadKeyStrokeExFunctionAutoTestCheckpoint1()

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2386

Enhance BBTestReadKeyStrokeExFunctionAutoTestCheckpoint1()
to handle ReadKeyStrokeEx implementation which returns EFI_NOT_READY
but without touching KeyToggleState.

Signed-off-by: Barton Gao <gaojie@byosoft.com.cn>
---
 .../BlackBoxTest/SimpleTextInputExBBTestFunction.c    | 11 ++++++++++-
 .../BlackBoxTest/SimpleTextInputExBBTestFunction.c    | 11 ++++++++++-
 2 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/SimpleTextInputEx/BlackBoxTest/SimpleTextInputExBBTestFunction.c b/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/SimpleTextInputEx/BlackBoxTest/SimpleTextInputExBBTestFunction.c
index 0398bc26..c1669959 100644
--- a/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/SimpleTextInputEx/BlackBoxTest/SimpleTextInputExBBTestFunction.c
+++ b/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/SimpleTextInputEx/BlackBoxTest/SimpleTextInputExBBTestFunction.c
@@ -1160,6 +1160,12 @@ BBTestReadKeyStrokeExFunctionAutoTestCheckpoint1 (
   ValidState[5] = EFI_TOGGLE_STATE_VALID | EFI_KEY_STATE_EXPOSED | EFI_NUM_LOCK_ACTIVE | EFI_CAPS_LOCK_ACTIVE;
   ValidState[6] = EFI_TOGGLE_STATE_VALID | EFI_KEY_STATE_EXPOSED | EFI_SCROLL_LOCK_ACTIVE |EFI_NUM_LOCK_ACTIVE | EFI_CAPS_LOCK_ACTIVE;

+  //
+  // Set all bits to one (invalid values) for both KeyShiftState and KeyToggleState
+  //
+  Key.KeyState.KeyShiftState = 0xFFFFFFFF;
+  Key.KeyState.KeyToggleState = 0xFF;
+
   //
   //Read next keystroke from the input device
   //
@@ -1171,7 +1177,10 @@ BBTestReadKeyStrokeExFunctionAutoTestCheckpoint1 (
     return Status;
   }

-  if ((Key.KeyState.KeyToggleState & EFI_TOGGLE_STATE_VALID) == 0) {
+  if (((Key.KeyState.KeyToggleState & EFI_TOGGLE_STATE_VALID) == 0) || (Key.KeyState.KeyShiftState == 0xFFFFFFFF) || (Key.KeyState.KeyToggleState == 0xFF)) {
+    //
+    // Log the error here and return key states are not supported when high order bit of KeyToggleState is 0 or KeyState not touched
+    //
     return EFI_UNSUPPORTED;
   }

diff --git a/uefi-sct/SctPkg/TestCase/UEFI/IHV/Protocol/SimpleTextInputEx/BlackBoxTest/SimpleTextInputExBBTestFunction.c b/uefi-sct/SctPkg/TestCase/UEFI/IHV/Protocol/SimpleTextInputEx/BlackBoxTest/SimpleTextInputExBBTestFunction.c
index adbf3dcf..e20ed345 100644
--- a/uefi-sct/SctPkg/TestCase/UEFI/IHV/Protocol/SimpleTextInputEx/BlackBoxTest/SimpleTextInputExBBTestFunction.c
+++ b/uefi-sct/SctPkg/TestCase/UEFI/IHV/Protocol/SimpleTextInputEx/BlackBoxTest/SimpleTextInputExBBTestFunction.c
@@ -1160,6 +1160,12 @@ BBTestReadKeyStrokeExFunctionAutoTestCheckpoint1 (
   ValidState[5] = EFI_TOGGLE_STATE_VALID | EFI_KEY_STATE_EXPOSED | EFI_NUM_LOCK_ACTIVE | EFI_CAPS_LOCK_ACTIVE;
   ValidState[6] = EFI_TOGGLE_STATE_VALID | EFI_KEY_STATE_EXPOSED | EFI_SCROLL_LOCK_ACTIVE |EFI_NUM_LOCK_ACTIVE | EFI_CAPS_LOCK_ACTIVE;

+  //
+  // Set all bits to one (invalid values) for both KeyShiftState and KeyToggleState
+  //
+  Key.KeyState.KeyShiftState = 0xFFFFFFFF;
+  Key.KeyState.KeyToggleState = 0xFF;
+
   //
   //Read next keystroke from the input device
   //
@@ -1171,7 +1177,10 @@ BBTestReadKeyStrokeExFunctionAutoTestCheckpoint1 (
     return Status;
   }

-  if ((Key.KeyState.KeyToggleState & EFI_TOGGLE_STATE_VALID) == 0) {
+  if (((Key.KeyState.KeyToggleState & EFI_TOGGLE_STATE_VALID) == 0) || (Key.KeyState.KeyShiftState == 0xFFFFFFFF) || (Key.KeyState.KeyToggleState == 0xFF)) {
+    //
+    // Log the error here and return key states are not supported when high order bit of KeyToggleState is 0 or KeyState not touched
+    //
     return EFI_UNSUPPORTED;
   }

--
2.31.0.windows.1







IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#87268): https://edk2.groups.io/g/devel/message/87268
Mute This Topic: https://groups.io/mt/87274606/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH] uefi-sct/SctPkg:Enhance BBTestReadKeyStrokeExFunctionAutoTestCheckpoint1()
Posted by G Edhaya Chandran 2 years, 3 months ago
Reviewed-by: G Edhaya Chandran <edhaya.chandran@arm.com>


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#85660): https://edk2.groups.io/g/devel/message/85660
Mute This Topic: https://groups.io/mt/87274606/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-