Hi @tobozo ,CoreS3 PSRAM is best set to on by default, otherwise it will affect the normal work of the camera.

Hi @tobozo ,CoreS3 PSRAM is best set to on by default, otherwise it will affect the normal work of the camera.

AMF you do not need that menu at all. Board comes with 32MB Flash and 8MB QSPI PSRAM. Clocks should be set to 80MHz and configuration options for those items should not exist.

@Tinyu-Zhao thanks for your input I'll remove the PSRAM block as suggested.

locks should be set to 80MHz and configuration options for those items should not exist.

@me-no-dev Should I also remove the FlashMode block, or maybe just the Flashmode.*.flash_freq lines?

@tobozo I think the default setting for theUSB CDC On Boot: Disabled is a better choice, otherwise many examples and the kernel is used are USBSerial, there will be convenient not through the case.

@Tinyu-Zhao I understand it affects the example sketch, however changing the default option value in the boards.txt will not prevent users from changing it in the menu, and the error can happen anyway.

Moreover, using Serial everywhere is much more intuitive, e.g when re-using sketches from other ESP32 products that do not handle USBCDC, or when the sketch is using a different library (M5Unified, LovyanGFX, TFT_eSPI, Arduino_GFX).

The official USBSerial example suggests using a safeguard to properly assign objects in a transparent fashion.

Some context: many ESP32-S3 board profiles enable CDC at boot: enabling this option solves many problems that occur in default situation e.g. debug logs not visible, late init, and no tty visible from the PC when in a crash loop.

In practice, enabling USB-CDC at boot aliases Serial instead of USBSerial.
Since Serial and USBSerial are both hardware CDC, using Serial.begin() with USB-CDC is the same as using USBSerial.begin() without USB-CDC.

Please let me know what the M5 team decides and I'll comply.

@tobozo @Tinyu-Zhao Is this PR ready for reviewing / merging? :) 2.0.10 release is close. Thanks

@tobozo It is recommended to keep USBSerial as the serial port, as many users will not be able to compile after the update if the new release is not properly adapted.

@Tinyu-Zhao I've updated the branch accordingly, however I have some afterthoughts:

whatever choice is made, users will have troubles:

• without CDC on boot users may experience problems to flash CoreS3 when not using <M5CoreS3.h> library
• with CDC on boot user have to manually alias the USBSerial when using M5CoreS3 library

I think it is safe to assume the problem is isolated to M5CoreS3 library and could be adressed by patching M5CoreS3.h with the conditional aliasing, so that USBSerial is available in both situations.

I have modified the M5CoreS3 library locally to not use USBSerial, but a combination of Serial + log_x, in order to get best compatibility. CDC_ON_BOOT is ON. IMHO M5CoreS3 lib should be adjusted. @Tinyu-Zhao some of the drivers are missing there as well. Could those be added? Publishing the source of the FactoryTest default firmware could also be helpful!

any updates on this?

sorry guys, PR was automatically closed when I synced the branch, so I re-opened it

CDC_ON_BOOT is now on by default as suggested by @me-no-dev

I have modified the M5CoreS3 library locally to not use USBSerial, but a combination of Serial + log_x, in order to get best compatibility. CDC_ON_BOOT is ON.

@Tinyu-Zhao if renaming USBSerial to Serial in M5CoreS3 library&examples is not satisfying, I have submitted an alternate solution to handle any combination of USB_MODE and CDC_ON_BOOT menu options.

@tobozo I agree with the changes. Let's hope that @Tinyu-Zhao will also agree

@me-no-dev the CoreS3 User Demo project (the FactoryTest default firmware) can be found here

all checks have passed but git complains, I'm not sure what to do with this, should I merge or rebase?

I'll try it later.

M5cores3 works great, thank you all for your help.

all checks have passed but git complains, I'm not sure what to do with this, should I merge or rebase?

I think "Update with merge commit" might be good.

What do we do with this PR now? We are about to release the last 2.x, so if we miss this, it will go for 3.0.0, where many things are different.

What do we do with this PR now? We are about to release the last 2.x, so if we miss this, it will go for 3.0.0, where many things are different.

Is it too late to merge? It's been checked and it's fine.

Not too late!