USBSerial would not block on write if the USB inteface is not connected #444

ghent360 · 2019-02-18T03:19:37Z

No description provided.

fpistm · 2019-02-18T07:28:33Z

Hi @ghent360
Thanks for the PR.
I guess it solve issue with Marlin FW ?

ghent360 · 2019-02-18T07:30:08Z

Yep.

On Sun, Feb 17, 2019 at 11:28 PM Frederic Pillon ***@***.***> wrote: Hi @ghent360 <https://github.com/ghent360> Thanks for the PR. I guess it solve issue with Marlin FW ? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#444 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AHxY1mkc_6DER27wTfL8uOJk0mAKRa9hks5vOlYjgaJpZM4bABDN> .

ktand · 2019-02-18T08:00:07Z

Hi @ghent360 Will it handle the situation where the USB is first connected and then the cable is disconnected without locking up? If I remember I had to check lineState to prevent a lock up with the timer based CDC. Karl

…

On Mon, Feb 18, 2019 at 8:30 AM ghent360 ***@***.***> wrote: Yep. On Sun, Feb 17, 2019 at 11:28 PM Frederic Pillon ***@***.*** > wrote: > Hi @ghent360 <https://github.com/ghent360> > Thanks for the PR. > I guess it solve issue with Marlin FW ? > > — > You are receiving this because you were mentioned. > Reply to this email directly, view it on GitHub > < #444 (comment) >, > or mute the thread > < https://github.com/notifications/unsubscribe-auth/AHxY1mkc_6DER27wTfL8uOJk0mAKRa9hks5vOlYjgaJpZM4bABDN > > . > — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#444 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AQ8NJL-3c1vZnH6mA-3GhxNpcG-O2IJmks5vOlaDgaJpZM4bABDN> .

ghent360 · 2019-02-18T08:06:00Z

Let me write a little test to double check. On Mon, Feb 18, 2019 at 12:00 AM Karl Andersson <notifications@github.com> wrote:

…

Hi @ghent360 Will it handle the situation where the USB is first connected and then the cable is disconnected without locking up? If I remember I had to check lineState to prevent a lock up with the timer based CDC. Karl On Mon, Feb 18, 2019 at 8:30 AM ghent360 ***@***.***> wrote: > Yep. > > On Sun, Feb 17, 2019 at 11:28 PM Frederic Pillon < ***@***.*** > > > wrote: > > > Hi @ghent360 <https://github.com/ghent360> > > Thanks for the PR. > > I guess it solve issue with Marlin FW ? > > > > — > > You are receiving this because you were mentioned. > > Reply to this email directly, view it on GitHub > > < > #444 (comment) > >, > > or mute the thread > > < > https://github.com/notifications/unsubscribe-auth/AHxY1mkc_6DER27wTfL8uOJk0mAKRa9hks5vOlYjgaJpZM4bABDN > > > > . > > > > — > You are receiving this because you are subscribed to this thread. > Reply to this email directly, view it on GitHub > < #444 (comment) >, > or mute the thread > < https://github.com/notifications/unsubscribe-auth/AQ8NJL-3c1vZnH6mA-3GhxNpcG-O2IJmks5vOlaDgaJpZM4bABDN > > . > — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#444 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AHxY1uUbRE09I-wGH6TlFaQ557Yh1GE0ks5vOl2JgaJpZM4bABDN> .

ghent360 · 2019-02-18T08:15:36Z

So I tried this: void setup() { // put your setup code here, to run once: SerialUSB.begin(115200); Serial1.begin(115200); } void loop() { for(int idx = 0; true; idx++) { SerialUSB.println(idx); Serial1.println(idx); } } The Serial1 continues to print when the USB is disconnected. The USB resumes after it gets re-connected. https://photos.app.goo.gl/hiP9ZAKGDzBkLmBMA On Mon, Feb 18, 2019 at 12:05 AM Venelin Efremov <veffremov.ve@gmail.com> wrote:

…

Let me write a little test to double check. On Mon, Feb 18, 2019 at 12:00 AM Karl Andersson ***@***.***> wrote: > Hi @ghent360 > > Will it handle the situation where the USB is first connected and then the > cable is disconnected without locking up? If I remember I had to check > lineState to prevent a lock up with the timer based CDC. > > Karl > > > > > > On Mon, Feb 18, 2019 at 8:30 AM ghent360 ***@***.***> > wrote: > > > Yep. > > > > On Sun, Feb 17, 2019 at 11:28 PM Frederic Pillon < > ***@***.*** > > > > > wrote: > > > > > Hi @ghent360 <https://github.com/ghent360> > > > Thanks for the PR. > > > I guess it solve issue with Marlin FW ? > > > > > > — > > > You are receiving this because you were mentioned. > > > Reply to this email directly, view it on GitHub > > > < > > > #444 (comment) > > >, > > > or mute the thread > > > < > > > https://github.com/notifications/unsubscribe-auth/AHxY1mkc_6DER27wTfL8uOJk0mAKRa9hks5vOlYjgaJpZM4bABDN > > > > > > . > > > > > > > — > > You are receiving this because you are subscribed to this thread. > > Reply to this email directly, view it on GitHub > > < > #444 (comment) > >, > > or mute the thread > > < > https://github.com/notifications/unsubscribe-auth/AQ8NJL-3c1vZnH6mA-3GhxNpcG-O2IJmks5vOlaDgaJpZM4bABDN > > > > . > > > > — > You are receiving this because you were mentioned. > Reply to this email directly, view it on GitHub > <#444 (comment)>, > or mute the thread > <https://github.com/notifications/unsubscribe-auth/AHxY1uUbRE09I-wGH6TlFaQ557Yh1GE0ks5vOl2JgaJpZM4bABDN> > . >

ghent360 · 2019-02-18T08:22:42Z

I see where the problem is: this only handles the case where the cable is yanked, but when connected there is something that reads the data on the PC. If I close the terminal on the PC the HardwareSerial still stops printing. Karl, what is it that you had to check? ~V On Mon, Feb 18, 2019 at 12:15 AM Venelin Efremov <veffremov.ve@gmail.com> wrote:

…

So I tried this: void setup() { // put your setup code here, to run once: SerialUSB.begin(115200); Serial1.begin(115200); } void loop() { for(int idx = 0; true; idx++) { SerialUSB.println(idx); Serial1.println(idx); } } The Serial1 continues to print when the USB is disconnected. The USB resumes after it gets re-connected. https://photos.app.goo.gl/hiP9ZAKGDzBkLmBMA On Mon, Feb 18, 2019 at 12:05 AM Venelin Efremov ***@***.***> wrote: > Let me write a little test to double check. > > On Mon, Feb 18, 2019 at 12:00 AM Karl Andersson ***@***.***> > wrote: > >> Hi @ghent360 >> >> Will it handle the situation where the USB is first connected and then >> the >> cable is disconnected without locking up? If I remember I had to check >> lineState to prevent a lock up with the timer based CDC. >> >> Karl >> >> >> >> >> >> On Mon, Feb 18, 2019 at 8:30 AM ghent360 ***@***.***> >> wrote: >> >> > Yep. >> > >> > On Sun, Feb 17, 2019 at 11:28 PM Frederic Pillon < >> ***@***.*** >> > > >> > wrote: >> > >> > > Hi @ghent360 <https://github.com/ghent360> >> > > Thanks for the PR. >> > > I guess it solve issue with Marlin FW ? >> > > >> > > — >> > > You are receiving this because you were mentioned. >> > > Reply to this email directly, view it on GitHub >> > > < >> > >> #444 (comment) >> > >, >> > > or mute the thread >> > > < >> > >> https://github.com/notifications/unsubscribe-auth/AHxY1mkc_6DER27wTfL8uOJk0mAKRa9hks5vOlYjgaJpZM4bABDN >> > > >> > > . >> > > >> > >> > — >> > You are receiving this because you are subscribed to this thread. >> > Reply to this email directly, view it on GitHub >> > < >> #444 (comment) >> >, >> > or mute the thread >> > < >> https://github.com/notifications/unsubscribe-auth/AQ8NJL-3c1vZnH6mA-3GhxNpcG-O2IJmks5vOlaDgaJpZM4bABDN >> > >> > . >> > >> >> — >> You are receiving this because you were mentioned. >> Reply to this email directly, view it on GitHub >> <#444 (comment)>, >> or mute the thread >> <https://github.com/notifications/unsubscribe-auth/AHxY1uUbRE09I-wGH6TlFaQ557Yh1GE0ks5vOl2JgaJpZM4bABDN> >> . >> >

uzi18 · 2019-02-18T08:29:54Z

Is there a way to check if usb/cdc is connected?

ghent360 · 2019-02-18T09:08:33Z

Yeah, this is a complex issue. I added checking the DTR line value, which gets set when the terminal is open and reset when the terminal is closed.

That makes it work almost all the time. Now the edge case is when the terminal is open, but the program on the PC does not actually read the serial data, then it locks again.

In my Ubuntu I use minicom for serial port communication, when I try to exit, there is a confirmation dialog "Are you sure you want to exit", if I stay there long enough the STM32 lock again. If I exit quickly there is no issue, the DTR line is reset and the code stops sending data, but if I linger on the exit dialog, apparently minicom stops reading data from the virtual terminal, but the terminal port is not closed.

What a bummer :-(

fpistm

~~I would propose also to add a new USBSerial method:~~
Forgot my comment about isConnected().

cores/arduino/stm32/usb/cdc/usbd_cdc_if.c

cores/arduino/stm32/usb/cdc/usbd_cdc_if.h

cores/arduino/stm32/usb/cdc/usbd_cdc_if.c

ghent360 · 2019-02-18T11:50:24Z

This last variant works even if I linger in the minicom menus.
I added a simple timeout if our transmit packet drags for too long I would assume the host is not active.

ghent360 · 2019-02-18T11:54:23Z

If people can test this change with different boards and host environments. Look for this #444 (comment) for an example test sketch.
You would need a board with physical serial port as well as USB connector.

fpistm

Could you also made some commit clean up by squashing some of them.
At this time, this PR should only have 4 commits:

Do not block writes...
Return the correct bytes written
Check DTR value
Check if the last packet transfer ...

You can refers to contributing.md about PR wow and commit message tips.

Thanks in advance

cores/arduino/USBSerial.cpp

ghent360 · 2019-02-19T00:38:03Z

I'm going on vacation tomorrow. I'll continue working on this PR when I come back at the end of the week.
Thank you all for the feedback.

ktand · 2019-02-21T21:36:41Z

@ghent360

I've tried the changes in this PR and this resolves the issue I had with Marlin where the firmware would hang until the serial port is opened on the host side.

On a side note, do you think there is any way that we could have a "serial disconnected" event that we could use within marlin to reset the board if the CDC port is closed/disconnected?

Thanks!

ghent360 · 2019-02-22T14:37:00Z

I was thinking about that for a bit, but it is not elegant approach, because when the board is restarted the usb device is disconnected/reconnected and it can result in a circular board reset behavior: Board starts; USB is connected; Port is open; Board is reset; Go to 1

…

On Thu, Feb 21, 2019 at 4:36 PM Karl Andersson ***@***.***> wrote: @ghent360 <https://github.com/ghent360> I've tried the changes in this PR and this resolves the issue I had with Marlin where the firmware would hang until the serial port is opened on the host side. On a side note, do you think there is any way that we could have a "serial disconnected" event that we could use within marlin to reset the board if the CDC port is closed/disconnected? Thanks! — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#444 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AHxY1gxq8jOj3ruH3mfJ0QAASbVdv8gjks5vPxFsgaJpZM4bABDN> .

roc2 · 2019-03-01T04:03:21Z

why the serial assistant on PC can't receive data with follow sketch:

#include <Arduino.h>
#include "USBSerial.h"

void setup()
{
    SerialUSB.begin();
    pinMode(LED_BUILTIN, OUTPUT);
}

void loop()
{
//    if (SerialUSB.available() > 0)
    {
       SerialUSB.println("testing read and wirte");
    }
    digitalToggle(LED_BUILTIN);
    delay(300);
}

Only when I add this line "if (SerialUSB.available() > 0)", then send some data with serial assistant on PC, the PC can receive data continusly.

fpistm · 2019-03-01T05:37:55Z

@roc2 this is not clear. Which board? Core version (1.5.0, master,...)? With this PR applied or not?....

roc2 · 2019-03-01T05:43:12Z

@fpistm My board is Black F407VE, core version 1.5.0.

fpistm

Some questions/remarks else code LGTM.
Currently, some tests on going.

cores/arduino/USBSerial.cpp

cores/arduino/stm32/usb/cdc/usbd_cdc_if.c

Old review already adress

fpistm · 2019-03-07T13:19:57Z

@ghent360 I've made several test on several boards with different STM32 families and host OS.
It's ok.
Still my comments to discuss.

cores/arduino/stm32/usb/cdc/usbd_cdc_if.c

fpistm

This does not build.

cores/arduino/stm32/usb/cdc/usbd_cdc_if.c

fpistm

LGTM. Will do some other tests

fpistm · 2019-03-09T06:45:54Z

Thanks @ghent360

fpistm · 2019-03-11T16:51:34Z

Using your sketch #444 (comment)
Works fine under Linux.
Under Windows, Unable to recognized the device after unplug/plug. Enum failed.
Anyway this is better than before :)

cores/arduino/USBSerial.cpp

fpistm · 2019-03-12T10:13:53Z

Under Windows, Unable to recognized the device after unplug/plug. Enum failed.

Update:
Doing further investigations, if I didn't open a terminal on the COM port. Enumeration is correct each time.

ghent360 · 2019-03-12T14:53:46Z

Does my change affect the enumeration?

…

On Tue, Mar 12, 2019 at 3:13 AM Frederic Pillon ***@***.***> wrote: Under Windows, Unable to recognized the device after unplug/plug. Enum failed. Update: Doing further investigations, if I didn't open a terminal on the COM port. Enumeration is correct each time. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#444 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AHxY1ply-75_yKZctstBm8A5Ur1DR4apks5vV33igaJpZM4bABDN> .

fpistm · 2019-03-12T15:51:42Z

Does my change affect the enumeration?

No @ghent360 , this is better with this PR.
Seems more like a driver issue but only after the COM has been used by Windows host (with a term).

Trying Teraterm, if I well disconnect, then it's ok. This seems very linked to the driver and how Windows handle the COM...

USBSerial would not block on write if the USB inteface is not connected

fpistm self-requested a review February 18, 2019 07:28

fpistm added Bug 🐛 Fix labels Feb 18, 2019

fpistm added this to In progress in STM32 core based on ST HAL via automation Feb 18, 2019

fpistm added this to the 1.5.1/1.6.0 milestone Feb 18, 2019

fpistm requested changes Feb 18, 2019

View reviewed changes

STM32 core based on ST HAL automation moved this from In progress to Needs review Feb 18, 2019

fpistm previously requested changes Feb 18, 2019

View reviewed changes

cores/arduino/USBSerial.cpp Outdated Show resolved Hide resolved

ghent360 force-pushed the cdc branch from 3eb08f8 to b949b94 Compare February 27, 2019 00:49

ghent360 added 3 commits February 26, 2019 23:21

Do not block writes to USBSerial if CDC interface is not connected

4d51503

Return the correct bytes written

60e4d4a

Check the DTR value when deciding to drop data

5e0da73

ghent360 force-pushed the cdc branch from b949b94 to 8bb04c6 Compare February 27, 2019 07:22

fpistm self-requested a review February 27, 2019 09:33

fpistm reviewed Mar 7, 2019

View reviewed changes

cores/arduino/USBSerial.cpp Outdated Show resolved Hide resolved

cores/arduino/USBSerial.cpp Outdated Show resolved Hide resolved

cores/arduino/stm32/usb/cdc/usbd_cdc_if.c Show resolved Hide resolved

fpistm added the Waiting feedback label Mar 7, 2019

ghent360 force-pushed the cdc branch 2 times, most recently from 8318da3 to 8bf279d Compare March 8, 2019 04:29

fpistm reviewed Mar 8, 2019

View reviewed changes

cores/arduino/stm32/usb/cdc/usbd_cdc_if.c Outdated Show resolved Hide resolved

ghent360 force-pushed the cdc branch from 8bf279d to 6656eb6 Compare March 8, 2019 08:00

fpistm requested changes Mar 8, 2019

View reviewed changes

cores/arduino/stm32/usb/cdc/usbd_cdc_if.c Outdated Show resolved Hide resolved

fpistm reviewed Mar 8, 2019

View reviewed changes

cores/arduino/stm32/usb/cdc/usbd_cdc_if.c Show resolved Hide resolved

ghent360 force-pushed the cdc branch from 6656eb6 to 82d932f Compare March 9, 2019 05:51

Check if the last packet we tried to transfer is taking too long

9373462

ghent360 force-pushed the cdc branch from 82d932f to 9373462 Compare March 9, 2019 06:04

fpistm approved these changes Mar 9, 2019

View reviewed changes

STM32 core based on ST HAL automation moved this from Needs review to Reviewer approved Mar 9, 2019

fpistm removed the Waiting feedback label Mar 9, 2019

fpistm reviewed Mar 12, 2019

View reviewed changes

cores/arduino/USBSerial.cpp Outdated Show resolved Hide resolved

fpistm mentioned this pull request Mar 12, 2019

USB features #222

Closed

2 tasks

Reset transmit timeout on terminal re-connect.

eea20ce

fpistm removed the Review on going label Mar 14, 2019

fpistm merged commit 4e861b4 into stm32duino:master Mar 14, 2019

STM32 core based on ST HAL automation moved this from Reviewer approved to Done Mar 14, 2019

fpistm mentioned this pull request Mar 24, 2019

USBSerial loosing characters on Linux #478

Closed

benwaffle pushed a commit to benwaffle/Arduino_Core_STM32 that referenced this pull request Apr 10, 2019

Merge pull request stm32duino#444 from ghent360/cdc

3fefc13

USBSerial would not block on write if the USB inteface is not connected

fpistm mentioned this pull request May 20, 2019

When the USB is not connected and the COM port is not open, system crashes. #527

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

USBSerial would not block on write if the USB inteface is not connected #444

USBSerial would not block on write if the USB inteface is not connected #444

ghent360 commented Feb 18, 2019 •

edited by fpistm

fpistm commented Feb 18, 2019

ghent360 commented Feb 18, 2019 via email

ktand commented Feb 18, 2019 via email

ghent360 commented Feb 18, 2019 via email

ghent360 commented Feb 18, 2019 via email

ghent360 commented Feb 18, 2019 via email

uzi18 commented Feb 18, 2019

ghent360 commented Feb 18, 2019 •

edited

fpistm left a comment •

edited

ghent360 commented Feb 18, 2019

ghent360 commented Feb 18, 2019 •

edited by fpistm

fpistm left a comment

ghent360 commented Feb 19, 2019

ktand commented Feb 21, 2019

ghent360 commented Feb 22, 2019 via email

roc2 commented Mar 1, 2019 •

edited

fpistm commented Mar 1, 2019

roc2 commented Mar 1, 2019

fpistm left a comment

fpistm commented Mar 7, 2019

fpistm left a comment

fpistm left a comment

fpistm commented Mar 9, 2019

fpistm commented Mar 11, 2019

fpistm commented Mar 12, 2019 •

edited

ghent360 commented Mar 12, 2019 via email

fpistm commented Mar 12, 2019 •

edited

USBSerial would not block on write if the USB inteface is not connected #444

USBSerial would not block on write if the USB inteface is not connected #444

Conversation

ghent360 commented Feb 18, 2019 • edited by fpistm

fpistm commented Feb 18, 2019

ghent360 commented Feb 18, 2019 via email

ktand commented Feb 18, 2019 via email

ghent360 commented Feb 18, 2019 via email

ghent360 commented Feb 18, 2019 via email

ghent360 commented Feb 18, 2019 via email

uzi18 commented Feb 18, 2019

ghent360 commented Feb 18, 2019 • edited

fpistm left a comment • edited

Choose a reason for hiding this comment

ghent360 commented Feb 18, 2019

ghent360 commented Feb 18, 2019 • edited by fpistm

fpistm left a comment

Choose a reason for hiding this comment

ghent360 commented Feb 19, 2019

ktand commented Feb 21, 2019

ghent360 commented Feb 22, 2019 via email

roc2 commented Mar 1, 2019 • edited

fpistm commented Mar 1, 2019

roc2 commented Mar 1, 2019

fpistm left a comment

Choose a reason for hiding this comment

fpistm commented Mar 7, 2019

fpistm left a comment

Choose a reason for hiding this comment

fpistm left a comment

Choose a reason for hiding this comment

fpistm commented Mar 9, 2019

fpistm commented Mar 11, 2019

fpistm commented Mar 12, 2019 • edited

ghent360 commented Mar 12, 2019 via email

fpistm commented Mar 12, 2019 • edited

ghent360 commented Feb 18, 2019 •

edited by fpistm

ghent360 commented Feb 18, 2019 •

edited

fpistm left a comment •

edited

ghent360 commented Feb 18, 2019 •

edited by fpistm

roc2 commented Mar 1, 2019 •

edited

fpistm commented Mar 12, 2019 •

edited

fpistm commented Mar 12, 2019 •

edited