Re: [edk2] [PATCH 2/6] MdeModulePkg/ConSplitter: ReadKeyStrokeEx always return key state
Hi Ruiyu, One comment: ConSplitterTextInPrivateReadKeyStroke() needs filter out partial key from ConSplitterTextInExDequeueKey(). With the comment above handled, Reviewed-by: Star Zengto this patch series. Thanks, Star -Original Message- From: Ni, Ruiyu Sent: Monday, January 22, 2018 4:10 PM To: edk2-devel@lists.01.org Cc: Zeng, Star ; Kinney, Michael D Subject: [PATCH 2/6] MdeModulePkg/ConSplitter: ReadKeyStrokeEx always return key state Today's implementation only return key state when there is key. But when user doesn't press any key, the key state cannot be returned. The patch changes the ReadKeyStrokeEx() to always return the key state even there is no key pressed. Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Ruiyu Ni Cc: Star Zeng Cc: Michael D Kinney --- .../Universal/Console/ConSplitterDxe/ConSplitter.c | 164 ++--- .../Universal/Console/ConSplitterDxe/ConSplitter.h | 4 +- 2 files changed, 143 insertions(+), 25 deletions(-) diff --git a/MdeModulePkg/Universal/Console/ConSplitterDxe/ConSplitter.c b/MdeModulePkg/Universal/Console/ConSplitterDxe/ConSplitter.c index e70ff6114a..022fca7cbb 100644 --- a/MdeModulePkg/Universal/Console/ConSplitterDxe/ConSplitter.c +++ b/MdeModulePkg/Universal/Console/ConSplitterDxe/ConSplitter.c @@ -16,7 +16,7 @@ never removed. Such design ensures sytem function well during none console device situation. -Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved. +Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved. (C) Copyright 2016 Hewlett Packard Enterprise Development LP This program and the accompanying materials are licensed and made available under the terms and conditions of the BSD License @@ -67,6 +67,8 @@ GLOBAL_REMOVE_IF_UNREFERENCED TEXT_IN_SPLITTER_PRIVATE_DATA mConIn = { (LIST_ENTRY *) NULL, (LIST_ENTRY *) NULL }, + (EFI_KEY_DATA *) NULL, + 0, 0, FALSE, @@ -606,6 +608,7 @@ ConSplitterTextInConstructor ( ) { EFI_STATUS Status; + UINTN TextInExListCount; // // Allocate buffer for Simple Text Input device @@ -631,6 +634,19 @@ ConSplitterTextInConstructor ( ); ASSERT_EFI_ERROR (Status); + // + // Allocate buffer for KeyQueue + // + TextInExListCount = ConInPrivate->TextInExListCount; + Status = ConSplitterGrowBuffer ( + sizeof (EFI_KEY_DATA), + , + (VOID **) >KeyQueue + ); + if (EFI_ERROR (Status)) { +return EFI_OUT_OF_RESOURCES; + } + // // Allocate buffer for Simple Text Input Ex device // @@ -1968,6 +1984,17 @@ ConSplitterTextInExAddDevice ( return EFI_OUT_OF_RESOURCES; } } + +TextInExListCount = Private->TextInExListCount; +Status = ConSplitterGrowBuffer ( + sizeof (EFI_KEY_DATA), + , + (VOID **) >KeyQueue + ); +if (EFI_ERROR (Status)) { + return EFI_OUT_OF_RESOURCES; +} + Status = ConSplitterGrowBuffer ( sizeof (EFI_SIMPLE_TEXT_INPUT_EX_PROTOCOL *), >TextInExListCount, @@ -3445,11 +3472,46 @@ ConSplitterTextInReset ( if (!EFI_ERROR (ReturnStatus)) { ToggleStateSyncReInitialization (Private); +// +// Empty the key queue. +// +Private->CurrentNumberOfKeys = 0; } return ReturnStatus; } +/** + Dequeue the saved key from internal key queue. + + @param Private Protocol instance pointer. + @param KeyData A pointer to a buffer that is filled in with the + keystroke state data for the key that was + pressed. + @retval EFI_NOT_FOUNDQueue is empty. + @retval EFI_SUCCESS First key is dequeued and returned. +**/ +EFI_STATUS +ConSplitterTextInExDequeueKey ( + IN TEXT_IN_SPLITTER_PRIVATE_DATA *Private, + OUT EFI_KEY_DATA*KeyData + ) +{ + if (Private->CurrentNumberOfKeys == 0) { +return EFI_NOT_FOUND; + } + // + // Return the first saved key. + // + CopyMem (KeyData, >KeyQueue[0], sizeof (EFI_KEY_DATA)); + Private->CurrentNumberOfKeys--; + CopyMem ( +>KeyQueue[0], +>KeyQueue[1], +Private->CurrentNumberOfKeys * sizeof (EFI_KEY_DATA) +); + return EFI_SUCCESS; +} /** Reads the next keystroke from the input device. The WaitForKey Event can @@ -3473,7 +3535,13 @@ ConSplitterTextInPrivateReadKeyStroke ( { EFI_STATUSStatus; UINTN Index; - EFI_INPUT_KEY CurrentKey; + EFI_KEY_DATA KeyData; + + Status = ConSplitterTextInExDequeueKey (Private, ); + if (!EFI_ERROR (Status)) { +CopyMem (Key, , sizeof (EFI_INPUT_KEY)); +return Status; + } Key->UnicodeChar = 0; Key->ScanCode = SCAN_NULL; @@ -3486,15
[edk2] [PATCH 2/6] MdeModulePkg/ConSplitter: ReadKeyStrokeEx always return key state
Today's implementation only return key state when there is key. But when user doesn't press any key, the key state cannot be returned. The patch changes the ReadKeyStrokeEx() to always return the key state even there is no key pressed. Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Ruiyu NiCc: Star Zeng Cc: Michael D Kinney --- .../Universal/Console/ConSplitterDxe/ConSplitter.c | 164 ++--- .../Universal/Console/ConSplitterDxe/ConSplitter.h | 4 +- 2 files changed, 143 insertions(+), 25 deletions(-) diff --git a/MdeModulePkg/Universal/Console/ConSplitterDxe/ConSplitter.c b/MdeModulePkg/Universal/Console/ConSplitterDxe/ConSplitter.c index e70ff6114a..022fca7cbb 100644 --- a/MdeModulePkg/Universal/Console/ConSplitterDxe/ConSplitter.c +++ b/MdeModulePkg/Universal/Console/ConSplitterDxe/ConSplitter.c @@ -16,7 +16,7 @@ never removed. Such design ensures sytem function well during none console device situation. -Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved. +Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved. (C) Copyright 2016 Hewlett Packard Enterprise Development LP This program and the accompanying materials are licensed and made available under the terms and conditions of the BSD License @@ -67,6 +67,8 @@ GLOBAL_REMOVE_IF_UNREFERENCED TEXT_IN_SPLITTER_PRIVATE_DATA mConIn = { (LIST_ENTRY *) NULL, (LIST_ENTRY *) NULL }, + (EFI_KEY_DATA *) NULL, + 0, 0, FALSE, @@ -606,6 +608,7 @@ ConSplitterTextInConstructor ( ) { EFI_STATUS Status; + UINTN TextInExListCount; // // Allocate buffer for Simple Text Input device @@ -631,6 +634,19 @@ ConSplitterTextInConstructor ( ); ASSERT_EFI_ERROR (Status); + // + // Allocate buffer for KeyQueue + // + TextInExListCount = ConInPrivate->TextInExListCount; + Status = ConSplitterGrowBuffer ( + sizeof (EFI_KEY_DATA), + , + (VOID **) >KeyQueue + ); + if (EFI_ERROR (Status)) { +return EFI_OUT_OF_RESOURCES; + } + // // Allocate buffer for Simple Text Input Ex device // @@ -1968,6 +1984,17 @@ ConSplitterTextInExAddDevice ( return EFI_OUT_OF_RESOURCES; } } + +TextInExListCount = Private->TextInExListCount; +Status = ConSplitterGrowBuffer ( + sizeof (EFI_KEY_DATA), + , + (VOID **) >KeyQueue + ); +if (EFI_ERROR (Status)) { + return EFI_OUT_OF_RESOURCES; +} + Status = ConSplitterGrowBuffer ( sizeof (EFI_SIMPLE_TEXT_INPUT_EX_PROTOCOL *), >TextInExListCount, @@ -3445,11 +3472,46 @@ ConSplitterTextInReset ( if (!EFI_ERROR (ReturnStatus)) { ToggleStateSyncReInitialization (Private); +// +// Empty the key queue. +// +Private->CurrentNumberOfKeys = 0; } return ReturnStatus; } +/** + Dequeue the saved key from internal key queue. + + @param Private Protocol instance pointer. + @param KeyData A pointer to a buffer that is filled in with the + keystroke state data for the key that was + pressed. + @retval EFI_NOT_FOUNDQueue is empty. + @retval EFI_SUCCESS First key is dequeued and returned. +**/ +EFI_STATUS +ConSplitterTextInExDequeueKey ( + IN TEXT_IN_SPLITTER_PRIVATE_DATA *Private, + OUT EFI_KEY_DATA*KeyData + ) +{ + if (Private->CurrentNumberOfKeys == 0) { +return EFI_NOT_FOUND; + } + // + // Return the first saved key. + // + CopyMem (KeyData, >KeyQueue[0], sizeof (EFI_KEY_DATA)); + Private->CurrentNumberOfKeys--; + CopyMem ( +>KeyQueue[0], +>KeyQueue[1], +Private->CurrentNumberOfKeys * sizeof (EFI_KEY_DATA) +); + return EFI_SUCCESS; +} /** Reads the next keystroke from the input device. The WaitForKey Event can @@ -3473,7 +3535,13 @@ ConSplitterTextInPrivateReadKeyStroke ( { EFI_STATUSStatus; UINTN Index; - EFI_INPUT_KEY CurrentKey; + EFI_KEY_DATA KeyData; + + Status = ConSplitterTextInExDequeueKey (Private, ); + if (!EFI_ERROR (Status)) { +CopyMem (Key, , sizeof (EFI_INPUT_KEY)); +return Status; + } Key->UnicodeChar = 0; Key->ScanCode = SCAN_NULL; @@ -3486,15 +3554,15 @@ ConSplitterTextInPrivateReadKeyStroke ( for (Index = 0; Index < Private->CurrentNumberOfConsoles;) { Status = Private->TextInList[Index]->ReadKeyStroke ( Private->TextInList[Index], - + ); if (!EFI_ERROR (Status)) { // // If it is not partial keystorke, return the key. Otherwise, continue // to read key from THIS