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

while (!Serial); hangs on Swan R5 with CDC (generic 'Serial' supercede U(S)ART) #1672

Closed
zfields opened this issue Mar 8, 2022 · 22 comments
Closed
Labels
invalid This doesn't seem right

Comments

@zfields
Copy link
Contributor

zfields commented Mar 8, 2022

Describe the bug
Firmware hangs at while (!Serial);.

Here is the implementation:

USBSerial::operator bool()
{
  delay(10);
  return dtrState;
}

I took a cursory look, and I don't see anywhere dtrState is getting set.

Here is the Arduino Board Configuration
image

The bug only appears with CDC (generic 'Serial' supercede U(S)ART).

The bug does not reproduce with:

  • CDC (no generic 'Serial')
  • HID (keyboard and mouse)

To Reproduce
Complete source code which can be used to reproduce the issue. Please try to be as generic as possible (no extra code, extra hardware,...)

Steps to reproduce the behavior:

  1. Flash this program to the device

    void setup()
    {
        Serial.begin(115200);
        while (!Serial);
        pinMode(LED_BUILTIN, OUTPUT);
    }
    
    void loop()
    {
        Serial.println("Looping!");
        static uint32_t state = LOW;
        state = !state;
        digitalWrite(LED_BUILTIN, state);
        delay(750);
    }
  2. The device will not blink or print "Looping!".

  3. Comment out

       while (!Serial);
  4. Reflash device

  5. The device will work as expected.

Expected behavior
I would expect Serial == true (a.k.a. to be "ready"), if it can send bytes over USB.

Screenshots
If applicable, add screenshots to help explain your problem.

Desktop (please complete the following information):

  • OS: Linux
  • Arduino IDE version: 1.8.19
  • STM32 core version: latest

Board (please complete the following information):

  • Name: Swan R5
  • Hardware Revision:

Additional context

VSCode Version: 1.65.1
Commit: 8908a9ca0f221f36507231afb39d2d8d1e182702
Date: 2022-03-08T02:09:03.269Z
Electron: 13.5.2
Chromium: 91.0.4472.164
Node.js: 14.16.0
V8: 9.1.269.39-electron.0
OS: Linux x64 5.16.11-76051611-generic

@fpistm
Copy link
Member

fpistm commented Mar 9, 2022

Hi @zfields
Could you try to disable dtr before the while:
Serial.dtr(false);

See #1382

@fpistm fpistm added this to To do in STM32 core based on ST HAL via automation Mar 9, 2022
@fpistm fpistm added the waiting feedback Further information is required label Mar 9, 2022
@zfields
Copy link
Contributor Author

zfields commented Mar 9, 2022

I can confirm setting Serial.dtr() to either true or false before while (!Serial); will allow it to behave as expected.

@zfields
Copy link
Contributor Author

zfields commented Mar 9, 2022

void USBSerial::dtr(bool enable)
{
  CDC_enableDTR(enable);
}
void CDC_enableDTR(bool enable)
{
  CDC_DTR_enabled = enable;
  dtrState = true;
}

Would it be possible to use a #define to set dtrState = true if CDC (generic 'Serial' supercede U(S)ART is selected as a board option?

I'm curious how it is working with CDC (no generic 'Serial') and HID (keyboard and mouse), without having to manually set the DTR state.

@fpistm
Copy link
Member

fpistm commented Mar 9, 2022

The Serial.dtr(bool) does not set the dtr value only enable or not the dtr feature as not all the terminal use it.

About:

I'm curious how it is working with CDC (no generic 'Serial') and HID (keyboard and mouse), without having to manually set the DTR state.
and:
The bug does not reproduce with:

* CDC (no generic 'Serial')

* HID (keyboard and mouse)

This is normal it does not occurred as Serial is mapped on a Hardware Serial instance in this case (Serialx) not the SerialUSB.

``

So:

Would it be possible to use a #define to set dtrState = true if CDC (generic 'Serial' supercede U(S)ART is selected as a board option?

No this is not the goal, dtr state is a volatile that can change if the dtr feature is available (default case). If you disable it then it will be always true.

@zfields
Copy link
Contributor Author

zfields commented Mar 9, 2022

I don't understand what you mean by the following statement...

The Serial.dtr(bool) does not set the dtr value only enable or not the dtr feature as not all the terminal use it.

How is this the case?

void USBSerial::dtr(bool enable)
{
  CDC_enableDTR(enable);
}
void CDC_enableDTR(bool enable)
{
  CDC_DTR_enabled = enable;
  dtrState = true;
}

Based on the code above, it appears Serial.dtr(bool) ALWAYS sets the DTR value (dtrState) to true.

@fpistm
Copy link
Member

fpistm commented Mar 9, 2022

I've tested your sketch with my Swan board and have no issue.

@fpistm
Copy link
Member

fpistm commented Mar 9, 2022

Based on the code above, it appears Serial.dtr(bool) ALWAYS sets the DTR value (dtrState) to true.

Yes it set it to true when you force to be enable but after that it is updated:

dtrState = (CDC_DTR_enabled) ? (((USBD_SetupReqTypedef *)pbuf)->wValue & CLS_DTR) : true;

@fpistm
Copy link
Member

fpistm commented Mar 9, 2022

My guess is that the terminal you used under Linux does not manage the DTR signal and so the while() is block.
Setting the DTR to True or false simply force the dtrstate value to be true, that's why it works.

@zfields
Copy link
Contributor Author

zfields commented Mar 9, 2022

I'm using the terminal supplied the the Arduino extension in VSCode.

@zfields
Copy link
Contributor Author

zfields commented Mar 9, 2022

I feel like we are getting caught up in the details.

Zooming out...

  • I can supply any value to Serial.dtr() to make the API work correctly (sounds like an initialization problem).
  • Regardless of the call to Serial.dtr(), bytes can be sent from the device and received by the terminal, even though !Serial doesn't evaluate to false.
  • It sounds like you are suggesting that I write non-portable code Serial.dtr() for ?all? STM32 boards, or add this to my code:
    #if defined(ARDUINO_ARCH_STM32) && (Serial == USBSerial)
    Serial.dtr(false);
    #endif
  • !Serial works properly on ESP32 and AVR boards.

This really feels like a problem on your end and not mine. If you don't agree, could you offer a deeper explanation of your position? I'm honestly trying to understand your position, but the Swan is behaving radically different than all my other non-STM32 boards.

@fpistm
Copy link
Member

fpistm commented Mar 9, 2022

I'm using the terminal supplied the the Arduino extension in VSCode.

I've tested with default Arduino Serial monitor (1.8.19) and VScode (1.65.1 + Arduino extension v0.4.11) but on Windows. Don't know if it is the same configuration on Linux or even rely on a dedicated VSCode settings.
Seems some related issues have already been opened: ex: microsoft/vscode#125911

Other things which can be check is the udev rules as stated by one of our user:
https://github.com/stm32duino/wiki/wiki/FAQ#usb-serial-cdc-stalls-initially-and-connecting-using-a-serial-monitor-fails

  • I can supply any value to Serial.dtr() to make the API work correctly (sounds like an initialization problem).

That's what I explain before when you use Serial.dtr(true|false) in any case the dtrState value is by default set to true.
In that case the while() can passes if the value is not updated by the USBD_CDC_Control().
I don't remember why I set it to true here maybe it should not be set until it is properly updated or set only when dtr is disable or by forcing the update.

  • Regardless of the call to Serial.dtr(), bytes can be sent from the device and received by the terminal, even though !Serial doesn't evaluate to false.

I don't understand your point: as long as the while(!Serial) is not evaluated true and no Serial.print()... are called the device send nothing.

  • It sounds like you are suggesting that I write non-portable code Serial.dtr() for ?all? STM32 boards, or add this to my code:
    #if defined(ARDUINO_ARCH_STM32) && (Serial == USBSerial)
    Serial.dtr(false);
    #endif

I didn't suggest anything 😉 Just that dtr issue was raised before and several users were not aware about this (including me). That's why I asked you if your terminal managed or not the linestate and you didn't answer it.
Refers to #1193

  • !Serial works properly on ESP32 and AVR boards.
    Fine. It works also on my side with the SWAN. So I simply try to understand why it doesn't work on your side.
    When I deployed this I've tested with realTerm which allow to play with the dtr and all cases were tested successfully.

This really feels like a problem on your end and not mine. If you don't agree, could you offer a deeper explanation of your position? I'm honestly trying to understand your position, but the Swan is behaving radically different than all my other non-STM32 boards.

I not agree or disagree. As stated: I simply try to understand why it doesn't work on your side.
DTR feature is enabled by default because it is used on some platform to reset the device and reboot in bootloader mode.

@zfields
Copy link
Contributor Author

zfields commented Mar 9, 2022

Thank you for the detailed response! That really provided some insight. 👍

Let me better explain this statement I made earlier:

Regardless of the call to Serial.dtr(), bytes can be sent from the device and received by the terminal, even though !Serial doesn't evaluate to false.

You responsed, "I don't understand your point: as long as the while(!Serial) is not evaluated true and no Serial.print()... are called the device send nothing." I'm sorry I didn't do a good job of explaining myself; let me try again.

What I meant to say is, I am already able to send bytes (i.e. Serial.println("Hello, world!");) and see it in my terminal. However, when I add while (!Serial); (without Serial.dtr(true|false)) !Serial will NEVER evaluate to false and let me to move on to the next line of code.

That's why I asked you if your terminal managed or not the linestate and you didn't answer it.

I'm sorry about that, I don't know how to check. I was hoping that by sharing the fact it does work without checking !Serial would be enough to answer your question. If you can tell me how to check, I will definitely look for you.

Thanks for taking the time to clarify. Tell me how to collect the information you need and I will gladly do it.

@fpistm
Copy link
Member

fpistm commented Mar 10, 2022

Based on your input it seems the terminal does not manage the DTR which explain why it does not work.
Looking at the VSCode Arduino extension GH issue and found some linked to DTR issue management:
microsoft/vscode-arduino#1458
microsoft/vscode-arduino#1285
microsoft/vscode-arduino#867

Anyway, I made a commit to fix issue about the dtrState force to true by default when using the dtr(bool) api (6187a43).
Could you try on your side to use a terminal with DTR support to confirm it works as expected with your default sketch?
For example using the Serial monitor of the Arduino IDE.

@zfields
Copy link
Contributor Author

zfields commented Mar 10, 2022

Yes, I will try it later today. Thank you!

@fpistm
Copy link
Member

fpistm commented Mar 11, 2022

Hi @zfields
Any update on this ?

@zfields
Copy link
Contributor Author

zfields commented Mar 11, 2022

Not yet, I got hung up on the other issue. My expectation is that it would fail, because the line that you removed updates the value that is specifically returned by USBSerial();.

USBSerial must return true in order to fail out of while (!Serial);

@fpistm
Copy link
Member

fpistm commented Mar 11, 2022

If it fails it is because the serial console does not managed the DTR (Data Terminal Ready).
From core side this is what is it expected. Moreover the goal of the while(!Serial); is to ensure the terminal is ready to receive.

With Arduino IDE Serial monitor it should work and we officially support only Arduino not VScode.
As stated before some issues have been opened on VSCode Arudino extension about this.

@zfields
Copy link
Contributor Author

zfields commented Mar 11, 2022

I will test with the Arduino IDE and let you know.

@fpistm fpistm added invalid This doesn't seem right and removed waiting feedback Further information is required labels Mar 14, 2022
@fpistm
Copy link
Member

fpistm commented Mar 14, 2022

I close this one as it works as expected.

@fpistm fpistm closed this as completed Mar 14, 2022
STM32 core based on ST HAL automation moved this from To do to Done Mar 14, 2022
@zfields
Copy link
Contributor Author

zfields commented Mar 14, 2022

I haven't been able to confirm this, because of issue stm32duino/Arduino_Tools#84.

The device is not showing up in lsusb as a Virtual COM Port, so I cannot confirm this works in the Arduino IDE.

@fpistm
Copy link
Member

fpistm commented Mar 14, 2022

Hi @zfields
all that you said confirm this.
I've made several test and it works as expected (only the Serial.dtr(true) which forced by default to true which is now fixed).
If you use the while(!Serial) and your terminal does not manage the DTR then it's up to you to disable the DTR management if you want keep this terminal or change the terminal.

@zfields
Copy link
Contributor Author

zfields commented Mar 14, 2022

Okay, I've figured it out, and it works perfectly. The linked issue 👆 threw me off.

Once I flashed using DFU instead of SWD, then the Virtual COM Port appeared correctly. This is a necessary prerequisite to this function working as expected. In Arduino, the program will hang until I open the Serial Monitor (Ctrl+Shift+M), and in VSCode, the program will hang until I open the connection using the Serial Monitor (in OUTPUT tab of the Terminal window area).

This behaves EXACTLY as I had hoped! It appears I was getting tripped up by the board being misconfigured.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
invalid This doesn't seem right
Projects
Development

No branches or pull requests

2 participants