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.

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

feature(cdc_teardown): Added iteration test #106

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

roma-jam
Copy link
Collaborator

@roma-jam roma-jam commented Dec 20, 2024

Description

Related

Testing

Added test_app teardown_device for CDC Device Class. The logic of the test:

  1. Installs the tinyUSB driver via esp-tinyusb wrapper with 1xCDC class device
  2. Awaits device configuration be the Host
  3. Awaits the terminal connection
  4. Expects the command sequence from the Host
  5. Replies with the response sequence to the Host
  6. Awaits terminal disconnection
  7. Teardowns the tinyUSB driver via esp-tinyusb wrapper
  8. Repeats the steps from the step.1 4 times
  9. Verify amount of attempts and memory leakage

command sequence[] = ep_size * 0xAA
response sequence[] = ep_size * 0x55

Hint: Values 0xAA and 0x55 were selected to verify the buffer memory integrity, as the 0xAA and 0x55 are the inversion of each other and the data bits in the same position changes from 1 to 0 in every transaction.

ep_size = 64 (S2/S3) and 512 (P4)

Checklist

Before submitting a Pull Request, please ensure the following:

  • 🚨 This PR does not introduce breaking changes.
  • All CI checks (GH Actions) pass.
  • Documentation is updated as needed.
  • Tests are updated or added as necessary.
  • Code is well-commented, especially in complex areas.
  • Git history is clean — commits are squashed to the minimum necessary.

@roma-jam roma-jam self-assigned this Dec 20, 2024
@roma-jam roma-jam force-pushed the feature/esp_tinyusb_stack_teardown branch 2 times, most recently from a1e0772 to d302232 Compare December 20, 2024 12:28
@roma-jam roma-jam changed the title Feature/cdc acm teardown test feature(cdc_teardown): Added iteration test Dec 20, 2024
@roma-jam roma-jam force-pushed the feature/esp_tinyusb_stack_teardown branch 3 times, most recently from 5922503 to be83b3f Compare December 22, 2024 23:22
Comment on lines +189 to +191
for (int i = 0; i < TEARDOWN_CMD_RPL_SIZE; i++) {
TEST_ASSERT_EQUAL(TEARDOWN_CMD_KEY, rx_buf[i]);
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
for (int i = 0; i < TEARDOWN_CMD_RPL_SIZE; i++) {
TEST_ASSERT_EQUAL(TEARDOWN_CMD_KEY, rx_buf[i]);
}
TEST_ASSERT_EACH_EQUAL_UINT8(TEARDOWN_CMD_KEY, rx_buf, TEARDOWN_CMD_RPL_SIZE);

You can use EACH_EQUAL macro to compare an array to a single value

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks for the idea!

}
// Remove primitives
vSemaphoreDelete(wait_mount);
vSemaphoreDelete(wait_command);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
vSemaphoreDelete(wait_command);
vSemaphoreDelete(wait_command);
vSemaphoreDelete(wait_terminal);

Delete also the wait_terminal semaphore.

@pytest.mark.esp32s3
@pytest.mark.esp32p4
@pytest.mark.usb_device
def test_usb_teardown_device(dut) -> None:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
def test_usb_teardown_device(dut) -> None:
def test_usb_teardown_device(dut: IdfDut) -> None:

I don't know if you are using some linting extension in VSCode, (I am using Pylint), It will give you some information and corrections about coding style.

This suggestion also comes from Pylint.


def teardown_device(key_len, amount):
TUSB_VID = 0x303A # Espressif TinyUSB VID
TUSB_PID = 0x4002 # Espressif TinyUSB VID
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
TUSB_PID = 0x4002 # Espressif TinyUSB VID
TUSB_PID = 0x4002 # Espressif TinyUSB PID

TUSB_VID = 0x303A # Espressif TinyUSB VID
TUSB_PID = 0x4002 # Espressif TinyUSB VID

# Command to send and expected response
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
# Command to send and expected response
# Command to send an expected response

Or Command to send and expect response

@roma-jam
Copy link
Collaborator Author

roma-jam commented Jan 2, 2025

@peter-marcisovsky Thanks for the review, I will apply all the notes.
But I don't think that this test is worth adding, as it is quite unstable yet and I don't know the reasons why so.
Also, I didn't solve the problem with your host, when the test just stops after first iteration.
So, lets keep this changes as a draft for a moment and return (*maybe) to them later.

@roma-jam roma-jam force-pushed the feature/esp_tinyusb_stack_teardown branch from be83b3f to a444bcb Compare January 2, 2025 11:19
Base automatically changed from feature/esp_tinyusb_stack_teardown to master January 2, 2025 15:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants