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

fixes default 100ms delay with HWCDC write() is CDC is not connected #9307

Merged
merged 6 commits into from
Feb 29, 2024

Conversation

SuGlider
Copy link
Collaborator

@SuGlider SuGlider commented Feb 28, 2024

Description of Change

Improves the functionality of HW CDC (HW Serial) when CDC is not connected to the Host side. At this point it solves the delay issue only when it is not plugged to the USB port. But even when plugged and the host enumerates it, it still delays when CDC is not connected, causing some side effect.

This PR fixes it and also adds 2 new APIs:

  • HWCDC::isConnected() will return true when HWCDC has a CDC connection to an application in the host side (like a serial terminal) and false otherwise.
  • HWCDC::isPlugged() will return true when USB is plugged to a USB host and it has enumarated the CDC class.

Tests scenarios

void setup() {
  Serial0.begin(115200);
  HWCDCSerial.begin();

  Serial0.println("\n ==> Connect to the CDC Serial Terminal\n");
  while(!HWCDCSerial) delay(100);

  Serial0.println("CDC is connected. It will write to the CDC and delay 1s.\nCheck the millisecond counter.");
  Serial0.println("After checking that it is steady in 1,000 milliseconds, unplug the USB cable. Check the time.");
  Serial0.println("Latter plug back the USB cable. Check the time. Do not open the CDC terminal.");
}

void loop() {
  Serial0.print("Time (ms): ");
  Serial0.println(millis());
  HWCDCSerial.print("Time (ms): ");
  HWCDCSerial.println(millis());

  // when USB is plugged and CDC terminal is not connected, it loop execution will take 1.1 seconds instead of 1.0 seconds.
  delay(1000);
}

Bad Output (from UART0):

ESP-ROM:esp32h2-20221101
Build:Nov  1 2022
rst:0x1 (POWERON),boot:0xc (SPI_FAST_FLASH_BOOT)
SPIWP:0xee
mode:DIO, clock div:1
load:0x4083cfd0,len:0xb68
load:0x4083efd0,len:0x2718
load:0x408460e8,len:0x590
entry 0x4083cfd0

 ==> Connect to the CDC Serial Terminal

CDC is connected. It will write to the CDC and delay 1s.
Check the millisecond counter.
After checking that it is steady in 1,000 milliseconds, unplug the USB cable. Check the time.
Latter plug back the USB cable. Check the time. Do not open the CDC terminal.
Time (ms): 12049             <<<<<======== USB plugged and CDC connected
Time (ms): 13049
Time (ms): 14054
Time (ms): 15054 <<<<<< average is + 1,000 ms
Time (ms): 16059
Time (ms): 17059
Time (ms): 18064             <<<<<======== USB NOT plugged and CDC NOT connected
Time (ms): 19064
Time (ms): 20069
Time (ms): 21069  <<<<<< average is + 1,000 ms
Time (ms): 22074
Time (ms): 23074
Time (ms): 24079
Time (ms): 25079             <<<<<======== USB plugged and CDC NOT connected :: PROBLEM
Time (ms): 26184
Time (ms): 27284
Time (ms): 28389
Time (ms): 29489
Time (ms): 30594 <<<<<< average is + 1,100 ms
Time (ms): 31694
Time (ms): 32799
Time (ms): 33899
Time (ms): 35004

Related links

#9043

Fixes delay when CDC is disconnected. At this time is only fixes it when USB cable is unplugged.
fixes delay when CDC is not connected. It was only considering when the USB cable was not plugged.
Adds 2 new methods:
- isPlugged() will return true when USB cable is plugged, false otherwise.
- isConnected() will return true when USB CDC is connected to a application in the USB Host side and communication is stablished.
Fixes the example to use the new added APIs for checking if USB cable is plugged and for checking if CDC is connected.
@SuGlider SuGlider marked this pull request as draft February 28, 2024 19:53
@SuGlider SuGlider self-assigned this Feb 28, 2024
Copy link
Contributor

github-actions bot commented Feb 28, 2024

Warnings
⚠️

Some issues found for the commit messages in this PR:

  • the commit message "feat(hwcdc): add 2 methods":
    • body's lines must not be longer than 100 characters
    • summary looks too short
  • the commit message "feat(hwcdc): adjusts APIs":
    • body's lines must not be longer than 100 characters
    • summary looks too short
  • the commit message "feat(hwcdc): fix delay":
    • summary looks too short
  • the commit message "feat(hwcdc): fix delay":
    • summary looks too short
  • the commit message "fixes API declaration":
    • summary looks empty
    • type/action looks empty
  • the commit message "fixes api declaration":
    • summary looks empty
    • type/action looks empty

Please fix these commit messages - here are some basic tips:

  • follow Conventional Commits style
  • correct format of commit message should be: <type/action>(<scope/component>): <summary>, for example fix(esp32): Fixed startup timeout issue
  • allowed types are: change,ci,docs,feat,fix,refactor,remove,revert,test
  • sufficiently descriptive message summary should be between 20 to 72 characters and start with upper case letter
  • avoid Jira references in commit messages (unavailable/irrelevant for our customers)

TIP: Install pre-commit hooks and run this check when committing (uses the Conventional Precommit Linter).

⚠️ Please consider squashing your 6 commits (simplifying branch history).

👋 Hello SuGlider, we appreciate your contribution to this project!


Click to see more instructions ...


This automated output is generated by the PR linter DangerJS, which checks if your Pull Request meets the project's requirements and helps you fix potential issues.

DangerJS is triggered with each push event to a Pull Request and modify the contents of this comment.

Please consider the following:
- Danger mainly focuses on the PR structure and formatting and can't understand the meaning behind your code or changes.
- Danger is not a substitute for human code reviews; it's still important to request a code review from your colleagues.
- Resolve all warnings (⚠️ ) before requesting a review from human reviewers - they will appreciate it.
- To manually retry these Danger checks, please navigate to the Actions tab and re-run last Danger workflow.

Review and merge process you can expect ...


We do welcome contributions in the form of bug reports, feature requests and pull requests.

1. An internal issue has been created for the PR, we assign it to the relevant engineer.
2. They review the PR and either approve it or ask you for changes or clarifications.
3. Once the GitHub PR is approved we do the final review, collect approvals from core owners and make sure all the automated tests are passing.
- At this point we may do some adjustments to the proposed change, or extend it by adding tests or documentation.
4. If the change is approved and passes the tests it is merged into the default branch.

Generated by 🚫 dangerJS against e72d945

Copy link
Collaborator

@lucasssvaz lucasssvaz left a comment

Choose a reason for hiding this comment

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

LGTM

@me-no-dev me-no-dev merged commit 2fdd901 into master Feb 29, 2024
39 checks passed
@me-no-dev me-no-dev deleted the hwcdc_delay branch February 29, 2024 06:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

3 participants