Rework usb-hid-and-msc-support #351
Conversation
m-iwanicki
force-pushed
the
rework-usb-hid
branch
from
f65f3e7 to
f5bc9de
Compare
| @@ -63,6 +63,7 @@ ${DEVICE_AUDIO1_WIN}= ${EMPTY} | |||
| ${WIFI_CARD_UBUNTU}= ${EMPTY} | |||
| ${USB_MODEL}= ${EMPTY} | |||
| ${USB_DEVICE}= ${EMPTY} | |||
| @{ATTACHED_USB}= ${EMPTY} | |||
There was a problem hiding this comment.
We need to include boot device names in the config for each board?
There was a problem hiding this comment.
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
| 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 |
There was a problem hiding this comment.
So we have:
- USB_KEYBOARD_DETECTION_SUPPORT
- DEVICE_USB_KEYBOARD
- HAS_KEYBOARD
Isn't it too much for nearly the same meaning?
There was a problem hiding this comment.
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
There was a problem hiding this comment.
fair enough, thanks for explaining
m-iwanicki
force-pushed
the
rework-usb-hid
branch
from
5a4dd95 to
40d174a
Compare
| ${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 |
There was a problem hiding this comment.
This still doesn't give reliable results for non-PiKVM platforms.
There was a problem hiding this comment.
This was/is addressed by @PLangowski in #206 and #310
tests if no keyboard or pikvm connected Signed-off-by: Michał Iwanicki <[email protected]>
Signed-off-by: Michał Iwanicki <[email protected]>
Signed-off-by: Michał Iwanicki <[email protected]>
Signed-off-by: Michał Iwanicki <[email protected]>
Signed-off-by: Michał Iwanicki <[email protected]>
macpijan
force-pushed
the
rework-usb-hid
branch
from
40d174a to
2e687af
Compare
No description provided.