Skip to content

Commit

Permalink
Changed USB Serial so that it will send to the host even if DTR is no…
Browse files Browse the repository at this point in the history 
…t set. Note this does not effect the operation of the boolean operation e.g.. while(!Serial); used to wait for the Arduino terminal to open
  • Loading branch information
@rogerclarkmelbourne
rogerclarkmelbourne committed Nov 11, 2018
1 parent 61ed869 commit 9a489ca
Showing 1 changed file with 8 additions and 1 deletion.
9 changes: 8 additions & 1 deletion STM32F1/cores/maple/usb_serial.cpp
View file
Expand Up @@ -119,9 +119,16 @@ size_t n = 0;
size_t USBSerial::write(const uint8 *buf, uint32 len)
{
size_t n = 0;
if (!(bool) *this || !buf) {

#ifdef USB_SERIAL_REQUIRE_DTR
if (!(bool) *this || !buf) {
return 0;
}
#else
if (!buf || !(usb_is_connected(USBLIB) && usb_is_configured(USBLIB))) {
return 0;
}
#endif

uint32 txed = 0;
while (txed < len) {
Expand Down

16 comments on commit 9a489ca

@stevstrong
Copy link
Collaborator

@stevstrong stevstrong commented on 9a489ca Jan 16, 2019 •
edited

Choose a reason for hiding this comment

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

I have the feeling that this commit breaks the USB serial functionality in the meaning that if the USB connection is there (USB cable from PC plugged in the micro USB slot) but no host application takes the Tx bytes, then the software will be blocked till USB Tx buffer is emptied, e.g. the host application takes the data.
Anyone else can reproduce this experience?

@BennehBoy
Copy link

@BennehBoy BennehBoy commented on 9a489ca Jan 16, 2019 via email

Choose a reason for hiding this comment

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

@rogerclarkmelbourne
Copy link
Owner Author

Choose a reason for hiding this comment

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

OK

Something else will need to be changed to stop that happening.

On normal Arduino boards (both AVR and SAM / SAMD), DTR does not need to be set for Serial data to be sent.

But I presume the difference must be that any data sent when a USB Serial host is not connected, will be lost, whereas our Serial buffering scheme must be buffering and then blocking and further sends.

I know there was a poll a long time ago about whether serial should block when the buffer was full, but I know nothing happened with the results of the poll, so perhaps people thought that the current operation (to block was OK)

On the AVR I don't think it buffers at all, it probably sends straight to its hardware serial pins, and the FDTI chip simply ignores the input when a host is not connected.

I can't remember whether the Due uses on-board USB or whether it also uses a FDTI chip for USB Serial.
If it uses on-board serial, that would be the best place to look for the defacto functionality implementation.

Other popular devices like the ESP8266 and the ESP32 all use external USB to Serial, so would be no-blocking and discard data when not connected to a host.

So I think perhaps the fix that would be most consistent with other Arduino cores and hardware would probably be to discard the data rather than to block.

@GitMoDu
Copy link
Contributor

Choose a reason for hiding this comment

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

I agree, respect the Arduino API (and expectations) as much as possible.

@stevstrong
Copy link
Collaborator

Choose a reason for hiding this comment

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

Well, I cannot see any Arduino API related to USB serial and DTR handling.
OTOH, Arduino IDE sets DTR when it opens the serial port.
So I think we are Arduino "compatible" if we analyze DTR to figure out whether the host can receive data or not.
I do not see any feasible solution to check for opened/active host application ready to receive data other then evaluating DTR.
I know, many users have different serial host applications in use which do not set DTR when active.
But are these applications Arduino "compatible"? Should we support them?

@rogerclarkmelbourne
Copy link
Owner Author

Choose a reason for hiding this comment

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

Although the Arduino IDE terminal sets DTR the official boards don't need this to happen.

The problem that is often reported is that when the STM32 is connected to a PC e.g. for data logging, the USB serial doesnt work, but it does work if they use a Arduino Uno etc

I think that the IDE terminal setting DTR is irrelevant, and it was probably a mistake by Leaflabs to use it as an indication that data can be sent, since the AVR etc boards don't use it, and potentially the IDE could change to not use DTR - and the AVR boards would still work fine.

@stevstrong
Copy link
Collaborator

Choose a reason for hiding this comment

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

The most Arduino boards will work without DTR because they have USART serial.

When I fixed the USB serial problem for F4, I analyzed the complete USB serial protocol and had to come to the conclusion that, AFAIK, in case of USB serial , DTR is the only flag available for host application to let the client know that he can receive data. So I don't think it was a mistake from Leaflabs to use it.

Extract from wikipedia:
"Data Terminal Ready (DTR) is a control signal in RS-232 serial communications, transmitted from data terminal equipment (DTE), such as a computer, to data communications equipment (DCE), for example a modem, to indicate that the terminal is ready for communications and the modem may initiate a communications channel."

It is true, we have here USB serial instead of RS232, but the scope is the same.

@ag88
Copy link
Contributor

@ag88 ag88 commented on 9a489ca Jan 18, 2019

Choose a reason for hiding this comment

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

i'm thinking perhaps we'd leave the old DTR behavior intact and have and ifdef flag to 'disable' it if it is defined.

@stevstrong
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is a good idea, I support it.

@rogerclarkmelbourne
Copy link
Owner Author

Choose a reason for hiding this comment

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

Has anyone actually tested whether this is really an issue ?

I've just modified an existing sketch which reads from a BMP280 and prints to Serial, and added code to flash the LED each time it reads the BMP280, and it seems to work fine when I plug my Maple Mini into a USB charger supply.

And. Its been running for 15 mins with the LED still flashing, which seems to indicate that the code is not being blocked.

@stevstrong
Copy link
Collaborator

Choose a reason for hiding this comment

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

On USB charger you don't have active host, so it will work ok, as you realized.
The problem occurs when connecting to a PC which correctly recognizes and enumerates the board, but you don't open any serial application to get/show the serial data.

@rogerclarkmelbourne
Copy link
Owner Author

Choose a reason for hiding this comment

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

OK

I think this goes back to

https://www.stm32duino.com/viewtopic.php?t=2477

In that poll, most people wanted to be able to configure blocking or non blocking themselves, and the second most popular was to not block

So the ideal solution is to not block by default, but to add a function to enabled and disabling blocking

https://www.stm32duino.com/viewtopic.php?t=2477#p33241

However I tried to track down how the F4 core enabled and disabled blocking, but I'm not sure it actually fully implements that functionality.

Because
https://github.com/rogerclarkmelbourne/Arduino_STM32/blob/master/STM32F4/cores/maple/usb_serial.h#L66-L67

Calls usbEnableBlockingTx()

Arduino_STM32/STM32F4/cores/maple/usb_serial.cpp

Lines 154 to 156 in 9d46a1c

void USBSerial::enableBlockingTx(void) {
usbEnableBlockingTx();
}

which calls VCP_SetUSBTxBlocking

Arduino_STM32/STM32F4/cores/maple/libmaple/usbF4/usb.c

Lines 53 to 55 in 9d46a1c

void usbEnableBlockingTx(void) {
VCP_SetUSBTxBlocking(1);
}

Which sets UsbTXBlock

Arduino_STM32/STM32F4/cores/maple/libmaple/usbF4/VCP/usbd_cdc_vcp.c

Lines 125 to 128 in 9d46a1c

void VCP_SetUSBTxBlocking(uint8_t Mode)
{
UsbTXBlock = Mode;
}

But that variable doesn't seem to be used

I think the same principal can be used in the F1 core (perhaps a bit simplified), but needs to be fully implemented ;-)

e.g. in

Arduino_STM32/STM32F1/cores/maple/usb_serial.cpp

Lines 134 to 136 in 9a489ca

while (txed < len) {
txed += usb_cdcacm_tx((const uint8*)buf + txed, len - txed);
}

because usb_cdcacm)_tx says in its comments that its non-blocking.
If that comment is correct we just need to make sure the while loop does not block is blocking is disabled.

So that flag would go into usb_serial.cpp (or usb_serial.h)

If I get time later today (Sunday) I'll try implementing it.

@alexwhittemore
Copy link

Choose a reason for hiding this comment

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

Consider me subscribed to the discussion :)

(FWIW, my vote would be non-blocking by default to be consistent with the FTDI/no flow control scheme that most Arduino hardware uses, but at least now I know where to go looking if I desperately need to pick one or the other for myself).

@rogerclarkmelbourne
Copy link
Owner Author

Choose a reason for hiding this comment

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

I changed USB Serial to be non blocking in

49d0cd4

I also added a function to re-enable blocking if the user needs it

@alexwhittemore
Copy link

Choose a reason for hiding this comment

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

Oh beautiful! What a guy 👍

Is stm32duino/Arduino_Core_STM32 actively pulling from this repo anymore? I'm still pretty unclear on the split here.

@rogerclarkmelbourne
Copy link
Owner Author

Choose a reason for hiding this comment

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

No.
stm32duino never pulled from this core.

github.com/stm32duino has only ever contained the official ST Arduino core, which is based on their HAL

And I transferred ownership of the github.com/stm32duino to ST several months ago.
ST have started to use the term STM32duino to refer to their core, and I am also closing down www.stm32duino.com

However this core will remain, for legacy and archive purposes

Please sign in to comment.