Skip to content

Serial as a #define #8798

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

Merged
merged 9 commits into from
Oct 25, 2023
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 2 additions & 5 deletions cores/esp32/HWCDC.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -436,12 +436,9 @@ void HWCDC::setDebugOutput(bool en)
}
}

#if ARDUINO_USB_MODE
#if ARDUINO_USB_CDC_ON_BOOT//Serial used for USB CDC
HWCDC Serial;
#else
#if ARDUINO_USB_MODE // Hardware JTAG CDC selected
// USBSerial is always available to be used
HWCDC USBSerial;
#endif
#endif

#endif /* SOC_USB_SERIAL_JTAG_SUPPORTED */
13 changes: 7 additions & 6 deletions cores/esp32/HWCDC.h
Original file line number Diff line number Diff line change
Expand Up @@ -103,12 +103,13 @@ class HWCDC: public Stream

};

#if ARDUINO_USB_MODE
#if ARDUINO_USB_CDC_ON_BOOT//Serial used for USB CDC
extern HWCDC Serial;
#else
extern HWCDC USBSerial;
#if ARDUINO_USB_MODE // Hardware JTAG CDC selected
#if ARDUINO_USB_CDC_ON_BOOT //Serial used for USB CDC
Copy link
Member

Choose a reason for hiding this comment

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

See my comment about USBCDC.h

// When using CDC on Boot, Arduino Serial is the USB device
#define Serial USBSerial
#endif
// USBSerial is always available to be used
extern HWCDC USBSerial;
#endif

#endif /* CONFIG_IDF_TARGET_ESP32C3 */
#endif /* SOC_USB_SERIAL_JTAG_SUPPORTED */
10 changes: 2 additions & 8 deletions cores/esp32/HardwareSerial.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -113,11 +113,8 @@ void serialEvent2(void) {}
#endif /* SOC_UART_NUM > 2 */

#if !defined(NO_GLOBAL_INSTANCES) && !defined(NO_GLOBAL_SERIAL)
#if ARDUINO_USB_CDC_ON_BOOT //Serial used for USB CDC
// There is always Seria0 for UART0
HardwareSerial Serial0(0);
#else
HardwareSerial Serial(0);
#endif
#if SOC_UART_NUM > 1
HardwareSerial Serial1(1);
#endif
Expand All @@ -127,11 +124,8 @@ HardwareSerial Serial2(2);

void serialEventRun(void)
{
#if ARDUINO_USB_CDC_ON_BOOT //Serial used for USB CDC
if(Serial0.available()) serialEvent();
#else
// Serial is always defined as something (UART0 or USBSerial)
if(Serial.available()) serialEvent();
Copy link
Member

Choose a reason for hiding this comment

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

Should be Serial0.available()

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I was thinking about it yesterday. If I use Serial.available(), it works with the CDC as well...
Serial0 will limit it to UART only.
Any opinion?

Copy link
Member

Choose a reason for hiding this comment

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

I think it should be left as Serial0 and then have other lines that handle USB. In case of CDC, Serial0 still should be handled here :0

#endif
#if SOC_UART_NUM > 1
if(Serial1.available()) serialEvent1();
#endif
Expand Down
14 changes: 7 additions & 7 deletions cores/esp32/HardwareSerial.h
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@
#include "esp32-hal.h"
#include "soc/soc_caps.h"
#include "HWCDC.h"
#include "USBCDC.h"

#include "freertos/FreeRTOS.h"
#include "freertos/task.h"
Expand Down Expand Up @@ -240,15 +241,14 @@ extern void serialEventRun(void) __attribute__((weak));
#ifndef ARDUINO_USB_CDC_ON_BOOT
#define ARDUINO_USB_CDC_ON_BOOT 0
#endif
#if ARDUINO_USB_CDC_ON_BOOT //Serial used for USB CDC
#if !ARDUINO_USB_MODE
#include "USB.h"
#include "USBCDC.h"
#if !ARDUINO_USB_CDC_ON_BOOT //Serial used for USB CDC
Copy link
Member

Choose a reason for hiding this comment

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

here you should have the checks from both CDCs as well. One place for all :)

// if not using CDC on Boot, Arduino Serial is the UART0 device
#define Serial Serial0
#endif

// There is always Seria0 for UART0
extern HardwareSerial Serial0;
#else
extern HardwareSerial Serial;
#endif

#if SOC_UART_NUM > 1
extern HardwareSerial Serial1;
#endif
Expand Down
5 changes: 3 additions & 2 deletions cores/esp32/USBCDC.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -455,8 +455,9 @@ USBCDC::operator bool() const
return connected;
}

#if ARDUINO_USB_CDC_ON_BOOT && !ARDUINO_USB_MODE //Serial used for USB CDC
USBCDC Serial(0);
#if !ARDUINO_USB_MODE // Native USB CDC selected
// USBSerial is always available to be used
USBCDC USBSerial(0);
#endif

#endif /* CONFIG_TINYUSB_CDC_ENABLED */
Expand Down
9 changes: 7 additions & 2 deletions cores/esp32/USBCDC.h
Original file line number Diff line number Diff line change
Expand Up @@ -141,8 +141,13 @@ class USBCDC: public Stream

};

#if ARDUINO_USB_CDC_ON_BOOT && !ARDUINO_USB_MODE //Serial used for USB CDC
extern USBCDC Serial;
#if !ARDUINO_USB_MODE // Native USB CDC selected
#if ARDUINO_USB_CDC_ON_BOOT //Serial used for USB CDC
// When using CDC on Boot, Arduino Serial is the USB device
#define Serial USBSerial
#endif
// USBSerial is always available to be used
Copy link
Member

Choose a reason for hiding this comment

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

the whole #if ARDUINO_USB_CDC_ON_BOOT block should be in HardwareSerial.h. Same goes for HWCDC.h.

#if !ARDUINO_USB_MODE        // Native USB CDC selected
extern USBCDC USBSerial;
#endif

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

OK.

extern USBCDC USBSerial;
#endif

#endif /* CONFIG_TINYUSB_CDC_ENABLED */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,16 +16,6 @@
// This makes the code transparent to what SoC is used.
#include "soc/soc_caps.h"

// In case that the target has USB CDC and it has being selected to be enable on boot,
// the console output will into USB (Serial).
// Otherwise the output will be sent to UART0 (Serial) and we have to redefine Serial0
#ifndef ARDUINO_USB_CDC_ON_BOOT
#define ARDUINO_USB_CDC_ON_BOOT 0
#endif
#if ARDUINO_USB_CDC_ON_BOOT == 0 // No USB CDC
#define Serial0 Serial // redefine the symbol Serial0 to the default Arduino
#endif

// This example shall use UART1 or UART2 for testing and UART0 for console messages
// If UART0 is used for testing, it is necessary to manually send data to it, using the Serial Monitor/Terminal
// In case that USB CDC is available, it may be used as console for messages.
Expand Down
6 changes: 0 additions & 6 deletions libraries/USB/examples/CompositeDevice/CompositeDevice.ino
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,7 @@ void loop(){}
#if !ARDUINO_USB_MSC_ON_BOOT
FirmwareMSC MSC_Update;
#endif
#if ARDUINO_USB_CDC_ON_BOOT
#define HWSerial Serial0
#define USBSerial Serial
#else
#define HWSerial Serial
USBCDC USBSerial;
#endif

USBHID HID;
USBHIDKeyboard Keyboard;
Expand Down
6 changes: 0 additions & 6 deletions libraries/USB/examples/FirmwareMSC/FirmwareMSC.ino
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,7 @@ void loop(){}
#if !ARDUINO_USB_MSC_ON_BOOT
FirmwareMSC MSC_Update;
#endif
#if ARDUINO_USB_CDC_ON_BOOT
#define HWSerial Serial0
Copy link
Member

Choose a reason for hiding this comment

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

you no longer need this define. Change the rest of the example code to call Serial0 for UART and USBSerial for CDC. Same for the other applicable examples.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

That's something I was thinking about yesterday as well...
I left it there because the user could change it to any other Print communication channel...
What do you say? We can change it to "PrintObj" instead of "HWSerial"

Copy link
Member

Choose a reason for hiding this comment

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

Looking at the code, I think that with those changes, it needs to just call Serial and not even check this define. If it's set to be UART, it will go there, else whatever CDC it's set to.

Copy link
Member

Choose a reason for hiding this comment

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

Replace all HWSerial with Serial and delete references to USBSerial

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

OK.

#define USBSerial Serial
#else
#define HWSerial Serial
USBCDC USBSerial;
#endif

static void usbEventCallback(void* arg, esp_event_base_t event_base, int32_t event_id, void* event_data){
if(event_base == ARDUINO_USB_EVENTS){
Expand Down
6 changes: 0 additions & 6 deletions libraries/USB/examples/USBMSC/USBMSC.ino
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,7 @@ void loop(){}
#include "USB.h"
#include "USBMSC.h"

#if ARDUINO_USB_CDC_ON_BOOT
#define HWSerial Serial0
#define USBSerial Serial
#else
#define HWSerial Serial
USBCDC USBSerial;
#endif

USBMSC MSC;

Expand Down
6 changes: 0 additions & 6 deletions libraries/USB/examples/USBSerial/USBSerial.ino
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,7 @@ void loop(){}
#else
#include "USB.h"

#if ARDUINO_USB_CDC_ON_BOOT
#define HWSerial Serial0
#define USBSerial Serial
#else
#define HWSerial Serial
USBCDC USBSerial;
#endif

static void usbEventCallback(void* arg, esp_event_base_t event_base, int32_t event_id, void* event_data){
if(event_base == ARDUINO_USB_EVENTS){
Expand Down
4 changes: 0 additions & 4 deletions libraries/USB/examples/USBVendor/USBVendor.ino
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,7 @@ void loop(){}
#include "USB.h"
#include "USBVendor.h"

#if ARDUINO_USB_CDC_ON_BOOT
#define HWSerial Serial0
#else
#define HWSerial Serial
#endif

USBVendor Vendor;
const int buttonPin = 0;
Expand Down