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

[hal] Fixes USB send failed #1871

Merged
merged 5 commits into from Aug 5, 2019

Conversation

@YutingYou
Copy link
Contributor

commented Aug 2, 2019

Problem

We Introduced USB state in #1846 , the USB sending API didn't update its available condition.

Solution

Change The available state of USB serial from HAL_USB_STATE_POWERED to HAL_USB_STATE_DEFAULT

Steps to Test

Make sure the USB can send data.

Example App

void setup() {
    Serial.begin();
}

void loop() {
    Serial.print("hello");
    delay(1000);
}

References


Completeness

  • [bugfix] [Gen3] Resolved a HardFault after USB cable is unplugged under certain conditions
  • [enhancement] [Gen3] USB state tracking enhancements

@YutingYou YutingYou requested a review from avtolstoy Aug 2, 2019

@avtolstoy

This comment has been minimized.

Copy link
Member

commented Aug 2, 2019

@YutingYou Please review 4e9e244

@avtolstoy

This comment has been minimized.

Copy link
Member

commented Aug 2, 2019

Boot:

0000000006 [hal.usbcdc] TRACE: USB state NONE -> POWERED
0000000009 [hal.usbcdc] TRACE: APP_USBD_EVT_DRV_SUSPEND
0000000010 [hal.usbcdc] TRACE: USB state POWERED -> SUSPENDED
0000000153 [hal.usbcdc] TRACE: APP_USBD_EVT_DRV_RESUME
0000000153 [hal.usbcdc] TRACE: USB state SUSPENDED -> POWERED
0000000154 [hal.usbcdc] TRACE: USB state POWERED -> DEFAULT
0000000313 [hal.usbcdc] TRACE: USB state DEFAULT -> ADDRESSED
0000000336 [hal.usbcdc] TRACE: USB state ADDRESSED -> CONFIGURED

USB cable disconnected:

0000102423 [hal.usbcdc] TRACE: APP_USBD_EVT_DRV_SUSPEND
0000102424 [hal.usbcdc] TRACE: USB state CONFIGURED -> SUSPENDED
0000102427 [hal.usbcdc] TRACE: USB state SUSPENDED -> DETACHED

USB cable reattached:

0000122746 [hal.usbcdc] TRACE: USB state DETACHED -> POWERED
0000122747 [hal.usbcdc] TRACE: USB state POWERED -> DEFAULT
0000122748 [hal.usbcdc] TRACE: APP_USBD_EVT_DRV_RESUME
0000122753 [hal.usbcdc] TRACE: APP_USBD_EVT_DRV_SUSPEND
0000122764 [hal.usbcdc] TRACE: USB state DEFAULT -> SUSPENDED
0000123037 [hal.usbcdc] TRACE: APP_USBD_EVT_DRV_RESUME
0000123038 [hal.usbcdc] TRACE: USB state SUSPENDED -> DEFAULT
0000123197 [hal.usbcdc] TRACE: USB state DEFAULT -> ADDRESSED
0000123220 [hal.usbcdc] TRACE: USB state ADDRESSED -> CONFIGURED
hal/src/nRF52840/usb_hal_cdc.c Show resolved Hide resolved
hal/src/nRF52840/usb_hal_cdc.c Outdated Show resolved Hide resolved
hal/src/nRF52840/usb_hal_cdc.c Outdated Show resolved Hide resolved

@avtolstoy avtolstoy force-pushed the fix/gen3-usb-send-failed branch from c3c2e04 to dc870ea Aug 2, 2019

@avtolstoy avtolstoy added this to the 1.3.1-rc.1 milestone Aug 2, 2019

@avtolstoy avtolstoy force-pushed the fix/gen3-usb-send-failed branch from dc870ea to 629f892 Aug 2, 2019

@avtolstoy

This comment has been minimized.

Copy link
Member

commented Aug 2, 2019

@YutingYou Please re-review the changes I've made and when you have the time verify:

  1. State changes are sane when the device is powered with USB cable in
  2. State changes are sane when the device is powered with no USB cable in (powered through VIN)
  3. State changes are sane when USB cable is unplugged
  4. State changes are sane when USB cable is plugged back
  5. State changes are sane when Serial.end() is called and cable is unplugged and re-plugged (nothing much should happen until Serial.begin() is called)
  6. State changes are sane once Serial.begin() is called after ^^
  7. State changes are sane once Serial.end() is called, cable is unplugged, Serial.begin() is called, cable is plugged back

@avtolstoy avtolstoy force-pushed the fix/gen3-usb-send-failed branch from 629f892 to 2e5ae5e Aug 2, 2019

@YutingYou

This comment has been minimized.

Copy link
Contributor Author

commented Aug 3, 2019

The only small issue I found is when Powered by VIN, USB state stops at SUSPENDED instead of stopping at DETACHED after unplugging the USB cable.

Using the below app, just a test app, don't mind thread-safe issue for Serial1, enable log in the interrupt.

SYSTEM_MODE(MANUAL);

Serial1LogHandler logHandler(115200, LOG_LEVEL_ALL, {{"app", LOG_LEVEL_ALL}});

void setup() {
}

void loop() {
    if (Serial1.available()) {
        switch(Serial1.read()) {
            case '1': Serial.begin(); break;
            case '2': Serial.end(); break;
            case '3': Serial.print("hello"); break;
            default: break;
        }
    }
}

@avtolstoy avtolstoy force-pushed the fix/gen3-usb-send-failed branch from 2e5ae5e to 70b84fa Aug 5, 2019

@avtolstoy avtolstoy force-pushed the fix/gen3-usb-send-failed branch from 70b84fa to 2155fb5 Aug 5, 2019

@avtolstoy

This comment has been minimized.

Copy link
Member

commented Aug 5, 2019

@YutingYou I've added some minor workarounds, however even before them I was not able to reproduce it:

0000000086 [hal.usbcdc] TRACE: USB state NONE -> DETACHED
0000000091 [hal.usbcdc] TRACE: USB state DETACHED -> POWERED
0000000098 [hal.usbcdc] TRACE: APP_USBD_EVT_DRV_SUSPEND
0000000098 [hal.usbcdc] TRACE: USB state POWERED -> SUSPENDED
0000000317 [hal.usbcdc] TRACE: APP_USBD_EVT_DRV_RESUME
0000000317 [hal.usbcdc] TRACE: USB state SUSPENDED -> POWERED
0000000318 [hal.usbcdc] TRACE: USB state POWERED -> DEFAULT
0000000476 [hal.usbcdc] TRACE: USB state DEFAULT -> ADDRESSED
0000000500 [hal.usbcdc] TRACE: USB state ADDRESSED -> CONFIGURED

Cable unplugged

0000001407 [hal.usbcdc] TRACE: APP_USBD_EVT_DRV_SUSPEND
0000001408 [hal.usbcdc] TRACE: USB state CONFIGURED -> SUSPENDED
0000001410 [hal.usbcdc] TRACE: USB state SUSPENDED -> DETACHED
@YutingYou

This comment has been minimized.

Copy link
Contributor Author

commented Aug 5, 2019

The only small issue I found is when Powered by VIN, USB state stops at SUSPENDED instead of stopping at DETACHED after unplugging the USB cable.

Even though sometimes USB state stops at SUSPENDED, when re-plugging USB cable, it will work properly again. So that's fine.

@avtolstoy avtolstoy merged commit f8018cb into develop Aug 5, 2019

2 checks passed

Codacy/PR Quality Review Up to standards. A positive pull request.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@avtolstoy avtolstoy deleted the fix/gen3-usb-send-failed branch Aug 5, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.