Skip to content

USB basic and hid greentea test cleanup and enable to the bare metal profile #12916

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

Merged
merged 2 commits into from
May 12, 2020
Merged

USB basic and hid greentea test cleanup and enable to the bare metal profile #12916

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f7f0681
USB hid and basic test
rajkan01 May 4, 2020
61c70e0
Cleanup
rajkan01 May 11, 2020
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
3 changes: 2 additions & 1 deletion TESTS/configs/baremetal.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,8 @@
"kv-global-api",
"sd",
"qspif",
"cryptocell310"
"cryptocell310",
"drivers-usb"
],
"target_overrides": {
"*": {
Expand Down
14 changes: 4 additions & 10 deletions TESTS/host_tests/pyusb_basic.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,16 +58,10 @@ def get_interface(dev, interface, alternate=0):

VENDOR_TEST_CTRL_IN = 1
VENDOR_TEST_CTRL_OUT = 2
VENDOR_TEST_CTRL_NONE = 3
VENDOR_TEST_CTRL_IN_DELAY = 4
VENDOR_TEST_CTRL_OUT_DELAY = 5
VENDOR_TEST_CTRL_NONE_DELAY = 6
VENDOR_TEST_CTRL_IN_STATUS_DELAY = 7
VENDOR_TEST_CTRL_OUT_STATUS_DELAY = 8
VENDOR_TEST_CTRL_IN_SIZES = 9
VENDOR_TEST_CTRL_OUT_SIZES = 10
VENDOR_TEST_RW_RESTART = 11
VENDOR_TEST_ABORT_BUFF_CHECK = 12
VENDOR_TEST_CTRL_IN_SIZES = 3
VENDOR_TEST_CTRL_OUT_SIZES = 4
VENDOR_TEST_RW_RESTART = 5
VENDOR_TEST_ABORT_BUFF_CHECK = 6
VENDOR_TEST_UNSUPPORTED_REQUEST = 32

REQUEST_GET_STATUS = 0
Expand Down
13 changes: 5 additions & 8 deletions TESTS/usb_device/basic/USBEndpointTester.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,6 @@

#if USB_DEVICE_TESTS

#if defined(MBED_CONF_RTOS_PRESENT)

#include "stdint.h"
#include "stdlib.h"
#include "USBEndpointTester.h"
Expand All @@ -39,10 +37,10 @@

#define VENDOR_TEST_CTRL_IN 1
#define VENDOR_TEST_CTRL_OUT 2
#define VENDOR_TEST_CTRL_IN_SIZES 9
#define VENDOR_TEST_CTRL_OUT_SIZES 10
#define VENDOR_TEST_RW_RESTART 11
#define VENDOR_TEST_ABORT_BUFF_CHECK 12
#define VENDOR_TEST_CTRL_IN_SIZES 3
#define VENDOR_TEST_CTRL_OUT_SIZES 4
#define VENDOR_TEST_RW_RESTART 5
#define VENDOR_TEST_ABORT_BUFF_CHECK 6

#define CTRL_BUF_SIZE (2048)

Expand Down Expand Up @@ -173,7 +171,6 @@ USBEndpointTester::USBEndpointTester(USBPhy *phy, uint16_t vendor_id, uint16_t p
MBED_ASSERT(_endpoint_buffs[i] != NULL);
}
MBED_ASSERT(resolver.valid());
queue = mbed::mbed_highprio_event_queue();
Copy link
Contributor

Choose a reason for hiding this comment

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

by removing the queue here, the test does not change - it has not been used at all?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes. This queue initially added by Russ to create a utility function but the scope of this work became obsolete. This queue is not used any of the test cases on full profile.

configuration_desc(0);
ctrl_buf = new uint8_t[CTRL_BUF_SIZE];
init();
Expand Down Expand Up @@ -864,5 +861,5 @@ void USBEndpointTester::start_ep_in_abort_test()
write_start(_endpoints[EP_BULK_IN], _endpoint_buffs[EP_BULK_IN], (*_endpoint_configs)[EP_BULK_IN].max_packet);
write_start(_endpoints[EP_INT_IN], _endpoint_buffs[EP_INT_IN], (*_endpoint_configs)[EP_INT_IN].max_packet);
}
#endif

#endif //USB_DEVICE_TESTS
1 change: 0 additions & 1 deletion TESTS/usb_device/basic/USBEndpointTester.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,6 @@ class USBEndpointTester: public USBDevice, private mbed::NonCopyable<USBEndpoint
};

protected:
events::EventQueue *queue;
rtos::EventFlags flags;
uint8_t *ctrl_buf;

Expand Down
30 changes: 4 additions & 26 deletions TESTS/usb_device/basic/USBTester.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,6 @@

#if USB_DEVICE_TESTS

#if defined(MBED_CONF_RTOS_PRESENT)

#include "stdint.h"
#include "USBTester.h"
#include "mbed_shared_queues.h"
Expand All @@ -29,14 +27,8 @@

#define VENDOR_TEST_CTRL_IN 1
#define VENDOR_TEST_CTRL_OUT 2
#define VENDOR_TEST_CTRL_NONE 3
#define VENDOR_TEST_CTRL_IN_DELAY 4
#define VENDOR_TEST_CTRL_OUT_DELAY 5
#define VENDOR_TEST_CTRL_NONE_DELAY 6
Copy link
Contributor

Choose a reason for hiding this comment

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

why is this delay removed? Shall this be a series of commits rather than "enable usb basic" - does not provide a reason for some changes I am commenting now on

Copy link
Contributor

Choose a reason for hiding this comment

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

Removed the USB endpoint test class unused variable and partially (The greentea test host side implementation is missed on pyusb_basic.py ) implemented USB request control transfer of VENDOR_TEST_CTRL_NONE_DELAY so it can run with the bare metal profile.

This would explain some changes, should be in the commit msg as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@0xc0170 Done. Please check

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think it is worth deleting VENDOR_TEST_CTRL_NONE_DELAY (here and in the .py file) because it breaks the defines and it is not the only one not implemented. For example I don't see any code calling VENDOR_TEST_CTRL_IN_STATUS_DELAY or VENDOR_TEST_CTRL_OUT_STATUS_DELAY

Copy link
Contributor Author

Choose a reason for hiding this comment

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

After discussion with @evedon have decided to remove all unused USB control transfer request type and also partially implemented test.

#define VENDOR_TEST_CTRL_IN_STATUS_DELAY 7
#define VENDOR_TEST_CTRL_OUT_STATUS_DELAY 8
#define VENDOR_TEST_CTRL_IN_SIZES 9
#define VENDOR_TEST_CTRL_OUT_SIZES 10
#define VENDOR_TEST_CTRL_IN_SIZES 3
#define VENDOR_TEST_CTRL_OUT_SIZES 4

#define MAX_EP_SIZE 64
#define MIN_EP_SIZE 8
Expand All @@ -45,7 +37,6 @@

#define EVENT_READY (1 << 0)


USBTester::USBTester(USBPhy *phy, uint16_t vendor_id, uint16_t product_id, uint16_t product_release):
USBDevice(phy, vendor_id, product_id, product_release), interface_0_alt_set(NONE),
interface_1_alt_set(NONE), configuration_set(NONE), reset_count(0),
Expand All @@ -60,7 +51,6 @@ USBTester::USBTester(USBPhy *phy, uint16_t vendor_id, uint16_t product_id, uint1
int_in = resolver.endpoint_in(USB_EP_TYPE_INT, 64);
int_out = resolver.endpoint_out(USB_EP_TYPE_INT, 64);
MBED_ASSERT(resolver.valid());
queue = mbed::mbed_highprio_event_queue();
configuration_desc(0);
ctrl_buf = new uint8_t[CTRL_BUF_SIZE];
init();
Expand Down Expand Up @@ -136,7 +126,6 @@ void USBTester::callback_request(const setup_packet_t *setup)
RequestResult result = PassThrough;
uint8_t *data = NULL;
uint32_t size = 0;
uint32_t delay = 0;

/* Process vendor-specific requests */
if (setup->bmRequestType.Type == VENDOR_TYPE) {
Expand All @@ -151,13 +140,6 @@ void USBTester::callback_request(const setup_packet_t *setup)
data = ctrl_buf;
size = setup->wValue < 8 ? setup->wValue : 8;
break;
case VENDOR_TEST_CTRL_NONE:
result = Success;
break;
case VENDOR_TEST_CTRL_NONE_DELAY:
result = Success;
delay = 2000;
break;
case VENDOR_TEST_CTRL_IN_SIZES:
result = Send;
data = ctrl_buf;
Expand All @@ -174,11 +156,7 @@ void USBTester::callback_request(const setup_packet_t *setup)
}
}

if (delay) {
queue->call_in(delay, static_cast<USBDevice *>(this), &USBTester::complete_request, Success, data, size);
} else {
complete_request(result, data, size);
}
complete_request(result, data, size);
}

void USBTester::callback_request_xfer_done(const setup_packet_t *setup, bool aborted)
Expand Down Expand Up @@ -706,5 +684,5 @@ void USBTester::epbulk_out_callback()
read_finish(bulk_out);
read_start(bulk_out, bulk_buf, sizeof(bulk_buf));
}
#endif

#endif //USB_DEVICE_TESTS
1 change: 0 additions & 1 deletion TESTS/usb_device/basic/USBTester.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,6 @@ class USBTester: public USBDevice, private mbed::NonCopyable<USBTester> {
uint8_t int_in;
uint8_t int_out;
uint8_t int_buf[64];
events::EventQueue *queue;
rtos::EventFlags flags;
volatile uint32_t reset_count;
volatile uint32_t suspend_count;
Expand Down
5 changes: 0 additions & 5 deletions TESTS/usb_device/basic/main.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,6 @@
#error [NOT_SUPPORTED] usb device tests not enabled
#else

#if !defined(MBED_CONF_RTOS_PRESENT)
#error [NOT_SUPPORTED] USB stack and test cases require RTOS to run.
#else

#include <stdio.h>
#include <string.h>
#include "mbed.h"
Expand Down Expand Up @@ -669,5 +665,4 @@ int main()
}

#endif // !defined(DEVICE_USBDEVICE) || !DEVICE_USBDEVICE
#endif // !defined(MBED_CONF_RTOS_PRESENT)
#endif // !defined(USB_DEVICE_TESTS)
5 changes: 0 additions & 5 deletions TESTS/usb_device/hid/main.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,6 @@
#error [NOT_SUPPORTED] usb device tests not enabled
#else

#if !defined(MBED_CONF_RTOS_PRESENT)
#error [NOT_SUPPORTED] USB stack and test cases require RTOS to run.
#else

#if !defined(DEVICE_USBDEVICE) || !DEVICE_USBDEVICE
#error [NOT_SUPPORTED] USB Device not supported for this target
#else
Expand Down Expand Up @@ -393,5 +389,4 @@ int main()
}

#endif // !defined(DEVICE_USBDEVICE) || !DEVICE_USBDEVICE
#endif // !defined(MBED_CONF_RTOS_PRESENT)
#endif // !defined(USB_DEVICE_TESTS)