Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rework usb-hid-and-msc-support #351

Open
wants to merge 5 commits into
base: develop
Choose a base branch
from
Open

Rework usb-hid-and-msc-support #351

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
13294b4
dasharo-compatibility: usb-hid-and-msc: Skip keyboard and HID
m-iwanicki Jul 3, 2024
2ca1ee1
dasharo-compatibility: usb-hid-and-msc: Don't mount ISO if no pikvm
m-iwanicki Jul 3, 2024
f735b93
platform-configs: default & protectli: move ATTACHED_USB to default
m-iwanicki Jul 4, 2024
319a3e5
dasharo-compatibility: usb: Skip tests with no keyboard or storage
m-iwanicki Jul 4, 2024
2e687af
dasharo-compatibility: usb-hid-msc: fix tests on Windows & Linux
m-iwanicki Jul 4, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
73 changes: 50 additions & 23 deletions dasharo-compatibility/usb-hid-and-msc-support.robot
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ Resource ../keys.robot
# Required teardown keywords:
# Log Out And Close Connection - elementary teardown keyword for all tests.
Suite Setup Run Keyword
... Prepare Test Suite
... Prepare USB HID Test Suite
Suite Teardown Run Keyword
... Log Out And Close Connection

Expand All @@ -28,7 +28,7 @@ USB001.001 USB devices detected in FW
[Documentation] Check whether USB devices are detected in Tianocore
... (edk2).
Skip If not ${USB_DISKS_DETECTION_SUPPORT} USB001.001 not supported
Upload And Mount DTS Flash Iso
Skip If not ${HAS_USB_STORAGE} USB001.001 not supported
Power On
${boot_menu}= Enter Boot Menu Tianocore And Return Construction
Check That USB Devices Are Detected ${boot_menu}
Expand All @@ -44,8 +44,8 @@ USB001.002 USB devices detected by OS (Ubuntu)
Switch To Root User
Detect Or Install Package usbutils
${out}= Execute Command In Terminal lsusb -v | grep bInterfaceClass
Should Contain ${out} Human Interface Device
Should Contain ${out} Mass Storage
IF ${HAS_KEYBOARD} Should Contain ${out} Human Interface Device
IF ${HAS_USB_STORAGE} Should Contain ${out} Mass Storage
Exit From Root User

USB001.003 USB devices detected by OS (Windows)
Expand All @@ -55,28 +55,28 @@ USB001.003 USB devices detected by OS (Windows)
Skip If not ${TESTS_IN_WINDOWS_SUPPORT} USB001.003 not supported
Power On
Login To Windows
${out}= Execute Command In Terminal Get-PnpDevice -PresentOnly | Where-Object { $_.InstanceId -match '^USB' }
Should Contain ${out} HIDClass
Should Contain ${out} DiskDrive
${out}= Execute Command In Terminal
... Get-PnpDevice -PresentOnly | Where-Object { $_.InstanceId -match '^USB' }
IF ${HAS_KEYBOARD} Should Contain ${out} HIDClass
IF ${HAS_USB_STORAGE} Should Contain ${out} DiskDrive

USB002.001 USB keyboard detected in FW
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This still doesn't give reliable results for non-PiKVM platforms.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was/is addressed by @PLangowski in #206 and #310

[Documentation] Check whether the external USB keyboard is detected
... correctly by the firmware and all basic keys work
... according to their labels.
[Tags] minimal-regression
Skip If not ${HAS_KEYBOARD} USB002.001 not supported
Power On
Enter UEFI Shell
${out}= Execute UEFI Shell Command devices
Should Contain ${out} Usb Keyboard

USB002.002 USB keyboard in OS (Ubuntu)
[Documentation] Check whether the external USB keyboard is detected
... correctly by the Linux OS and all basic keys work
... according to their labels.
IF not ${USB_KEYBOARD_DETECTION_SUPPORT}
SKIP USB002.002 not supported
END
IF not ${TESTS_IN_UBUNTU_SUPPORT} SKIP USB002.002 not supported
... correctly by the Linux OS.
Skip If not ${USB_KEYBOARD_DETECTION_SUPPORT} USB002.002 not supported
Skip If "${DEVICE_USB_KEYBOARD}" == "${EMPTY}" USB002.002 not supported
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So we have:

  • USB_KEYBOARD_DETECTION_SUPPORT
  • DEVICE_USB_KEYBOARD
  • HAS_KEYBOARD

Isn't it too much for nearly the same meaning?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if those global variables have documentation anywhere.
From what I can understand *_SUPPORT variable means that it's possible to test this on this platform as a whole not that this particular platform has keyboard attached.
This test checks that ${DEVICE_USB_KEYBOARD} is in output of lsusb but if this variable is empty test succeeds everytime so doesn't really test anything.
HAS_KEYBOARD is combination of DEVICE_USB_KEYBOARD and PiKVM (I'm assuming if PiKVM is connected then it should always detect it as keyboard?)

This PR mostly fixes reported issues and one I encountered during tests so I didn't make bigger changes

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fair enough, thanks for explaining

Skip If not ${TESTS_IN_UBUNTU_SUPPORT} USB002.002 not supported
Power On
Boot System Or From Connected Disk ubuntu
Login To Linux
Expand All @@ -87,10 +87,9 @@ USB002.002 USB keyboard in OS (Ubuntu)
USB002.003 USB keyboard in OS (Windows)
[Documentation] Check whether the external USB keyboard is detected
... correctly by the Windows OS.
IF not ${USB_KEYBOARD_DETECTION_SUPPORT}
SKIP USB002.003 not supported
END
IF not ${TESTS_IN_WINDOWS_SUPPORT} SKIP USB002.003 not supported
Skip If not ${USB_KEYBOARD_DETECTION_SUPPORT} USB002.003 not supported
Skip If not ${HAS_KEYBOARD} USB002.003 not supported
Skip If not ${TESTS_IN_WINDOWS_SUPPORT} USB002.003 not supported
Power On
Login To Windows
${out}= Execute Command In Terminal Get-CimInstance win32_KEYBOARD
Expand All @@ -100,8 +99,9 @@ USB002.003 USB keyboard in OS (Windows)
USB003.001 Upload 1GB file on USB storage (Ubuntu)
[Documentation] Check whether the 1GB file can be transferred from the
... operating system to the USB storage.
IF not ${UPLOAD_ON_USB_SUPPORT} SKIP USB003.001 not supported
IF not ${TESTS_IN_UBUNTU_SUPPORT} SKIP USB003.001 not supported
Skip If not ${UPLOAD_ON_USB_SUPPORT} USB003.001 not supported
Skip If not ${HAS_USB_STORAGE} USB003.001 not supported
Skip If not ${TESTS_IN_UBUNTU_SUPPORT} USB003.001 not supported
Power On
Boot System Or From Connected Disk ubuntu
Login To Linux
Expand All @@ -116,16 +116,43 @@ USB003.001 Upload 1GB file on USB storage (Ubuntu)
USB003.002 Upload 1GB file on USB storage (Windows)
[Documentation] Check whether the 1GB file can be transferred from the
... operating system to the USB storage.
IF not ${UPLOAD_ON_USB_SUPPORT} SKIP USB003.002 not supported
IF not ${TESTS_IN_WINDOWS_SUPPORT} SKIP USB003.002 not supported
Skip If not ${UPLOAD_ON_USB_SUPPORT} USB003.002 not supported
Skip If not ${HAS_USB_STORAGE} USB003.002 not supported
Skip If not ${TESTS_IN_WINDOWS_SUPPORT} USB003.002 not supported
Power On
Login To Windows
Generate 1GB File In Windows
# Work only with one attached USB storage
${drive_letter}= Get Drive Letter Of USB
Execute Command In Terminal Copy-Item -Path C:\\Users\\user\\test_file.txt ${drive_letter}:
Execute Command In Terminal
... Copy-Item -Path C:\\Users\\user\\test_file.txt ${drive_letter}: 120
${hash1}= Get Hash Of File test_file.txt
${hash2}= Get Hash Of File ${drive_letter}:\\test_file.txt
Execute Command In Terminal Remove-Item -Path C:\\Users\\user\\test_file.txt
Execute Command In Terminal Remove-Item -Path ${drive_letter}:\\test_file.txt
Execute Command In Terminal
... Remove-Item -Path ${drive_letter}:\\test_file.txt 120
Should Be Equal ${hash1} ${hash2}


*** Keywords ***
Prepare USB HID Test Suite
[Documentation] Prepare this test suite
Prepare Test Suite
IF "${DEVICE_USB_KEYBOARD}" != "${EMPTY}" or "${DUT_CONNECTION_METHOD}" == "pikvm"
Set Suite Variable $HAS_KEYBOARD ${TRUE}
ELSE
Set Suite Variable $HAS_KEYBOARD ${FALSE}
END
${conf}= Get Current CONFIG ${CONFIG_LIST}
${has_storage}= Evaluate "USB_Storage" in """${conf}"""
IF "${DUT_CONNECTION_METHOD}" == "pikvm"
Upload And Mount DTS Flash Iso
${has_storage}= Set Variable ${TRUE}
END
IF "${ATTACHED_USB}" != "${EMPTY}" or ${has_storage}
Set Suite Variable $HAS_USB_STORAGE ${TRUE}
ELSE
Set Suite Variable $HAS_USB_STORAGE ${FALSE}
END
Skip If not ${HAS_KEYBOARD} and not ${HAS_USB_STORAGE}
... Platform doesn't have USB keyboard or USB storage attached
29 changes: 21 additions & 8 deletions keywords.robot
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1588,22 +1588,34 @@ Remove Entry From List

Generate 1GB File In Windows
[Documentation] Generates 1G file in Windows in .txt format.
Execute Command In Terminal
... if (Test-Path test_file.txt) { Remove-Item test_file.txt }
${out}= Execute Command In Terminal fsutil file createnew test_file.txt 1073741824
Should Contain ${out} is created

Get Drive Letter Of USB
[Documentation] Gets drive letter of attached USB, work with only one USB
... attached.
${drive_letter}= Execute Command In Terminal
... (Get-Volume | where drivetype -eq removable | where filesystemtype -eq FAT32).driveletter
${drive_letter}= Fetch From Left ${drive_letter} \r
[Documentation] Gets drive letter of attached USB, returns first letter
... on list.
${drive_letter_cmd}= Set Variable
... (Get-Volume | where DriveType -eq removable | where FileSystemType -eq FAT32).DriveLetter
${drive_count}= Execute Command In Terminal ${drive_letter_cmd}.Count
${drive_count}= Fetch From Right ${drive_count} \n
IF ${drive_count} > 0
${drive_letter}= Execute Command In Terminal
... ${drive_letter_cmd}\[0\]
ELSE
Fail Couldn't find any USB drive letter
END
${drive_letter}= Fetch From Right ${drive_letter} \n
RETURN ${drive_letter}

Get Hash Of File
[Documentation] Gets line with hash of file.
[Arguments] ${path_file}
${out}= Execute Command In Terminal Get-FileHash -Path ${path_file} | Format-List
${hash}= Get Lines Containing String ${out} Hash
${start_index}= Call Method ${out} rindex Hash
${hash}= Get Substring ${out} ${start_index}
${hash}= Fetch From Left ${hash} \r
RETURN ${hash}

Identify Path To USB
Expand All @@ -1619,9 +1631,10 @@ Identify Path To USB
Set Local Variable ${usb_disk} ${disk}
IF '${model_name}' == '${USB_MODEL}' BREAK
END
${out}= Execute Linux Command lsblk | grep ${usb_disk} | grep part | cat
${out}= Execute Linux Command
... lsblk --list --noheadings --output NAME,TYPE,PATH | grep ${usb_disk} | grep part
${split}= Split String ${out}
${path_to_usb}= Get From List ${split} 7
${path_to_usb}= Get From List ${split} 2
RETURN ${path_to_usb}

Get Intel ME Mode State
Expand Down
1 change: 1 addition & 0 deletions platform-configs/include/default.robot
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ ${DEVICE_AUDIO1_WIN}= ${EMPTY}
${WIFI_CARD_UBUNTU}= ${EMPTY}
${USB_MODEL}= ${EMPTY}
${USB_DEVICE}= ${EMPTY}
@{ATTACHED_USB}= ${EMPTY}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to include boot device names in the config for each board?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would say yes, all global variables should probably be in some default state instead of fixing errors that no keyword exists.
In this case this line from usb-detect.robot would always fail for all protectli except vp2410 and vp2420.

Do we even need this variable? in raptor-cs_talos2.robot config there is comment:
# Temporary parameter - we are not able to use the Hardware matrix
If this problem was fixed then we could just get rid of it as it was only used in usb-detect.robot


${FLASHROM_FLAGS}= ${EMPTY}

Expand Down
2 changes: 0 additions & 2 deletions platform-configs/protectli-vp2410.robot
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,6 @@ ${CPU_MIN_FREQUENCY}= 300
# eMMC driver support
${E_MMC_NAME}= 8GTF4R

@{ATTACHED_USB}= @{EMPTY}

${DMIDECODE_SERIAL_NUMBER}= N/A
${DMIDECODE_FIRMWARE_VERSION}= Dasharo (coreboot+UEFI) v
${DMIDECODE_PRODUCT_NAME}= VP2410
Expand Down
2 changes: 0 additions & 2 deletions platform-configs/protectli-vp2420.robot
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,6 @@ ${FLASHING_METHOD}= internal
# eMMC driver support
${E_MMC_NAME}= 8GTF4R

@{ATTACHED_USB}= ${EMPTY}

${DMIDECODE_SERIAL_NUMBER}= N/A
${DMIDECODE_FIRMWARE_VERSION}= Dasharo (coreboot+UEFI) v1.2.0
${DMIDECODE_PRODUCT_NAME}= VP2420
Expand Down
Loading