ESP32 S3 - USB Serial JTAG Console not Support in current code

[SimonGAndrews]

There seems to be an issue in that the ESP32S3 build does not include support for the USB Serial JTAG Console. That is boards where the USB connector (for connection to the host/IDE) is routed directly to the processor (GPIO 19/ D+ and 20 / D- ) and not through a USB to Serial Convert chip to 43/U0TXD and 44/ U0RXD.

The symptom is no response in the IDE after flashing. The espressif monitor confirms flashing is ok and the espruino banner displays after a reboot, but then no console/repl.

Ive seen other forum posts that indicate the same.

In the current code ../targets/ESP32/jshardwareUart.c handles this for the ESP32C3 but the #ifdef in the code does not include the S3. Ive thad a go to fix in threee places:

Which has improved in that I get a console now. But it now has the problem seen during the initial port where there is no echo till enter key is pressed.

I Also note the need to define USB_CDC in board.py, whcih ive done. And it looks to be set in the IDE.

WIll keep going to figure it out, but I cant initially see where to fix the Echo, given it was fixed in the C3.

[gfwilliams]

Thanks for looking into this! IIRC Espruino was getting the data as written and processing it, but the data was only being sent out to USB after a newline. I'm pretty sure this was the code (esp32USBUARTWasUsed), so hopefully a check for ESP32S3 in there too would fix it:

https://github.com/espruino/Espruino/blob/master/targets/esp32/jshardware.c#L595-L599

[gfwilliams]

I think you need it here too: https://github.com/espruino/Espruino/blob/master/targets/esp32/main.c#L47-L51

[SimonGAndrews]

Ok there are three places in main Looks like need the change :

Are we ok just to check ?
#if defined(CONFIG_IDF_TARGET_ESP32C3) || defined(CONFIG_IDF_TARGET_ESP32S3)

Or should we check for JTAG Serial use also ie
#if (defined(CONFIG_IDF_TARGET_ESP32C3) || defined(CONFIG_IDF_TARGET_ESP32S3)) && defined(USB_CDC)

But this may be more usefull going forward:
#if (ESP_IDF_VERSION_MAJOR >= 4) && defined(USB_CDC)

[SimonGAndrews]

Sorry one more consideration:

does @MaBecker , @gfwilliams or anyone object to changing USB_CDC to USB_SERIAL_JTAG
In board.py and throughout.
Arguably more intutive and but is in line with the ESPIDF config whcih provides these thre config parameters

CONFIG_ESP_CONSOLE_USB_SERIAL_JTAG
CONFIG_ESP_CONSOLE_USB_CDC 
CONFIG_ESP_CONSOLE_UART

and then our condition becomes
#if (ESP_IDF_VERSION_MAJOR >= 4) && defined(USB_SERIAL_JTAG)

I would test throughout on c3, s3 with and without Jtag and on ESP32

---

Im not suggesting to use these ESPIDF configs now , we stick witth the def in board.py , But see this example for how we could look in the future.

CONFIG_ESP_CONSOLE_USB_CDC is USB OTG configuration for the console. See here

[gfwilliams]

#if (ESP_IDF_VERSION_MAJOR >= 4) && defined(USB_CDC)

Sounds good to me. Is this IDF4+ specific, or is it for devices with USB built in though? Eg would it work for ESP32_IDF4 where it's just an ESP32 on IDF4? Might be worth having something in platform_config (built with build_platform_config.py) which was just ESPR_SERIAL_JTAG or something?

anyone object to changing USB_CDC to USB_SERIAL_JTAG

I'm fine to change it, but personally I'd like to change the polarity to something like ESP_FORCE_NO_USB_SERIAL or similar.

I feel like the vast majority of ESP32C3/S3/etc are going to have serial over USB - that's one of the great things of the chip. It should really be opt-out IMO

[SimonGAndrews]

ok that would work - so im testing based upon this - but any feedback shout

Condition for logic
#if defined(CONFIG_ESP_CONSOLE_USB_SERIAL_JTAG) && !defined(ESP_FORCE_NO_USB_SERIAL)

and

Setup in board.py

and using ESP menuconfig to set CONFIG_ESP_CONSOLE_USB_SERIAL_JTAG on the appropriate targets config file. Eg targets/esp32/IDF4/sdkconfig_s3

which builds the config as

[gfwilliams]

That looks excellent, thanks! Way better being able to use something SDK-defined!

[SimonGAndrews]

@MaBecker , hi are you ok with this approach?

[MaBecker]

Yes, go - thanks for your effort

[SimonGAndrews]

Having tested changing parameters in the SDKConfigs to identify target boards uisng the ESP_CONSOLE_USB_SERIAL_JTAG im not going to include that in the solution. The available SDKCONFIG parameters were not working well in in the cases we are trying to override to use the USB to Serial conversion chips.

Instead im proposing to use a macro in the targets/ESP32/JShardwareUart.h like this

and a test condition in the the code
#if TARGET_PROVIDES_USB_SERIAL_JTAG && !defined(ESP_FORCE_NO_USB_SERIAL_JTAG)

It does mean the Espruino code will need to be modified if another board comes along with the USB_SERIAL_JTAG functionality. But it will only need changing in the one place instead of the eight places in three different files where the condition is used.

The Boardfile.py changes as follows are working fine:

Testing notes and PR to follow

[SimonGAndrews]

@gfwilliams / @MaBecker the port name for flashing changes with the new override as expected.

I can see that scripts/get_makefile_decls.py ESP32C3_IDF4 output the ESP_FORCE_NO_USB_SERIAL_JTAG in /gen/CURRENTBOARD.make if defined.
I could modify the target makefile to put an if condition on the port setting ??

[gfwilliams]

Instead im proposing to use a macro in the targets/ESP32/JShardwareUart.h like this

That sounds fine to me! I guess we just need to ensure JShardwareUart.h is included wherever we check for that define? Otherwise we could put the code in build_platform_config.py so it ends up in the platform_config.h file which is already included pretty much everywhere?

I could modify the target makefile to put an if condition on the port setting ??

I think setting the PORT is probably a step too far - after all a bunch of users are on Mac OS and the ports there are totally different :(

[SimonGAndrews]

Thanks , i will leave the port setting alone.

I didnt have to change any includes , JShardwareUart.h , was included in all the files that needed the condition.

[gfwilliams]

I didnt have to change any includes , JShardwareUart.h ,

Sounds great then!

[gfwilliams]

How's this looking? I see you've made a few commits (on your branch?). Do you think they'd be ready for a PR to get this fixed?

[SimonGAndrews]

Apologies for the delay. the fix is ready for a final test on the S3/C3 and ESP32 to make sure its still ok. I recommited my changes to my branch tonight becuse there were other changes in the same files during my delay. Ive unpicked them and my branch is synced except the PR with my changes ready to go after testing. The build workflows all completed in my branch tonight. pretty confident with it but would prefer to test again. Sorry but im on holiday till march 17 so can test then. Happy to send the PR if someone else wants to test.

[gfwilliams]

Ok, great - thanks!

No rush at all (unless as you say someone else really wants to test this) - hope you have a nice holiday!

[SimonGAndrews]

Quick Status @gfwilliams . I have a tested version that gets the s3 with jtag USB working and regression is ok with ESP32 and ESP32C3. However there are a couple of test cases that are failing where board provides aditional USB or ESP prog port wired to the uart . My plan was these would work with the Serial override flag set. One of the boards is the Espressif Devkit that @rgomezwap was using , and the serial connected USB did work prior to my changes, so would like to see that work. Im pusing on with fixing that . But if you want an interimum PR with that has this as an outstanding issue shout.
PS there is a new issue in the ESP32S3_IDF4.py file after the last commit.
pins = pinutils.generate_pins(0,39) Needs updating to 48 pins give the new default on pin 43 for USART1_TX.
I will fix .

[SimonGAndrews]

Also just for consideration , do we have a gap in the approach to adding the USB via JTAG as the console. E.getConsole shows the console as 'Serial1' , as one might expect with the current approach. Whats involved in adding a USB device to board file and using the JTAG driver in the case of the USB being the default console, maybe next setep is sort setConsole and GetConsole to properly flip between USB and Serial1 uart ? One area I dont fully understand is the Device handling in the IO Event queue , is that necessary or can we just use similiar to the exisitng methods in main.c and jshardwareuart.c of flipping to the USB jtag or UART just in support of the console, But utilising the jsiGetConsoleDevice() global variables and similiar functions ?

[SimonGAndrews]

Im going to test a version of this fix which uses the setting of default_console in the board.py file (and including a USB device) to set USBSERIAL as the default. In theory it should allow the E.SetConsole() function to change to Serial on the fly whcih would be usefull with the dual USB port boards.

I think ive traced through the setup of the serial devices device down to initSerial() in jshardwareuart.c and I ve modified to include EV_USBSERIAL (See pic) and put the Jtag serial initialisations there.

The consoleToEspruino() function can then use the jsiGetConsoleDevice() function to select the correct serial read function (pic 3). Ive kept the flushing bits for the USB jtag. I had to hack the is USB connected function as there is not an available espidf call until v5.

I may not get it right but I think its fundementally the right approach to suit the underlying Espruino console support. Its not building yet! and Testing is next.

Any thoughts, advice or directions , please shout as usual.

[gfwilliams]

Sorry for the delay - yes using EV_USBSERIAL for the built-in USB support sounds like a great idea. As-is it feels like it gets in the way of using Serial1?

What'd be ideal is if there was a way of detecting if the USB JTAG was connected. Maybe use bool usb_serial_jtag_is_connected(void); in jshIsUSBSERIALConnected and then add what we do for STM32 into the ESP32 jshIdle handler:

Espruino/targets/stm32/jshardware.c

Lines 1558 to 1565 in d2cbb39

	if (USBConnected && jsiGetConsoleDevice()!=EV_LIMBO) {
	if (!jsiIsConsoleDeviceForced())
	jsiSetConsoleDevice(EV_USBSERIAL, false);
	} else {
	if (!jsiIsConsoleDeviceForced() && jsiGetConsoleDevice()==EV_USBSERIAL)
	jsiSetConsoleDevice(jsiGetPreferredConsoleDevice(), false);
	jshTransmitClearDevice(EV_USBSERIAL); // clear the transmit queue
	}

So then hopefully if we're connected by USB JTAG it'll swap automatically, so on boards with it available it'll swap over automatically, but otherwise it'll use whatever the default console device is (Serial1?) on those other devices?

pins = pinutils.generate_pins(0,39) Needs updating to 48 pins

Oh yes, thanks! I just put that in.

[SimonGAndrews]

Thanks @gfwilliams , Making some progress. Having introduced USB as a device in the board file, It took me a while to understand the importance of the function jsiOneSecondAfterStartup() to move the console device out of 'Limbo' when USB is defined. This function is not currently executed in the ESP32 build and hence console was switching to Limbo and staying there. Im adding the execution of the function in main. . So I have a working build with USB defined in the board file now!

Regarding

usb_serial_jtag_is_connected(void);

unfortunately its not available in the current ESPIDF V4 were using. Its in V5. WIll try to get a solution without this or a hack for now and the dynamic switching over as in STM example could follow in V5. Ive made a note in the function jshIsUSBSERIALConnected as todo for V5

If you have a minute to explain the concept behind jshPushIOCharEvents, KickUart and jshTransmit, I would appreciate it.
Thanks Again

[gfwilliams]

Ahh, great! Thanks for spotting the jsiOneSecondAfterStartup issue - this all looks really promising.

'EV_LIMBO' is there so we can start running JS code at boot before the USB/whatever has properly initialised without losing data sent to the port - because at startup we don't want to sent any data to Serial if the board is actually going to end up connected to USB.

usb_serial_jtag_is_connected(void) unfortunately its not available in the current ESPIDF V4

Argh, that's a pain. I guess worst case we could detect when a character arrives on USB JTAG and use that as a queue to switch over?

If you have a minute to explain the concept behind jshPushIOCharEvents, KickUart and jshTransmit, I would appreciate it.

Sure! There are two FIFOs in Espruino - one of them (txBuffer) is for transmission - code can push stuff into it (jshTransmit), and then interrupt handlers generally take it out - for instance an interrupt handler for Serial1 might take characters out of txBuffer whenever it received a 'buffer empty' interrupt. However suppose we just wanted to start transmitting on Serial and we weren't before - in that case we need to start the Serial peripheral transmitting - and that's where KickUart gets called usually.

However under an RTOS like the ESP32 I believe there are already output buffers for each peripheral, so potentially there's something to be said for not ever putting data into txBuffer - you could add something to jshTransmit which would just push the data direct to the RTOS.

The second buffer is ioBuffer and that comes from Interrupts to the event loop (outside of interrupts). So any Serial character received, or pin changing state, or Bluetooth message gets shoved in there. jshPushIOCharEvents is what you'd call to to put data in when serial data is received.

... so on ESP32/ESP8266 it's less than ideal as Espruino has these buffers which were designed for very low-level use when running bare metal on microcontrollers, and we're effectively using those and ESP32's RTOS ones as well. I think when getting data from Serial devices, jshPushIOCharEvents/ioBuffer is probably still the right way - but when sending data txBuffer doesn't make that much sense.

[SimonGAndrews]

With regard to regression testing , changes in the console device setup:
I picked up on @GaryOtt other discussions that he connected a REPL client to the C3 via bluetooth. Which to be honest I had not realised was supported in the ESP32 family. I was equally pleased to hear it worked :). Ive since quickly tried this on the current released code for C3, S3 and ESP32 and they all work, I can see also the console device switching from EV_Serial1 to Bluthooth using E.setConsole(), in the as is case before any changes, Which is great.
So these will all need a regression test.
My first question is other than these bluetooth , the new SerialUSB Jtag, and the UART tests , is there any other console setups that need checking; eg Wifi?

[SimonGAndrews]

Thanks @gordon for the details above, ive had a good look at this over the weekend and your start helped me understand alot more. And I believe I can see the way through to fix the issue by setting up the USB for Jtag on the C3/S3. But apologies another question:

It looks like the main route to a console from txbuffer is via jshTransmit() whcih calls jshUSARTKick(). JshUSARKick() has a switch on the device name and handles each device seperately. For the fix it needs USB case adding. My question is what would you suggest has to be done in the USB case?
I can see in the case of Serial that the characters are pulled out of the buffer till its empty but in the case of bluetooth gatts_sendNUSNotifications is executed. I dont understand the bluetooth stack but as its not passing characters I assume its kicking off something to trigger the transfer out (Im reading round this now)

This is my chart mainly to help me understand it. My current thinking is the USB case will write the txbuffer characters to the USB with usb_serial_jtag_write_bytes (maybe in a modified writeSerial(device,char) function with devices cases) and then flush via esp32USBUARTWasUsed.

PS you may have seen a different question in here prior but I read the logic wrong , working too late again

[gfwilliams]

is there any other console setups that need checking; eg Wifi?

I don't think so - as long as it switches console back to (eg) bluetooth when you disconnect USB I think we're good

My question is what would you suggest has to be done in the USB case?

What you're proposing there looks good - although I would keep esp32USBUARTWasUsed in there rather than calling usb_serial_jtag_ll_txfifo_flush directly. My understanding is that usb_serial_jtag_ll_txfifo_flush causes a USB packet to be sent, so if you call it every character it'll slow transmission down quite a lot (it's why it goes into uartTask).

Longer term, I think we could look at some other changes though....

• we could move the uart_tx_one_char code from jshUSARTKick into uartTask? That way if uart_tx_one_char blocks then it won't stop espruino from executing until it fills the output buffer
• Or maybe just move the if (device==EV_USBSERIAL into jshTransmit (maybe I'd have to add a function to jshardware.h so we could keep it platform-specific) and avoid putting USB Serial (or normal serial) into the output buffer at all as I guess the ESP32 buffers them itself

[SimonGAndrews]

Thank you @gfwilliams , Ive got it now. Ill push on with the fix and tests.

[SimonGAndrews]

Sorry @gfwilliams Im still working / testing this and have another question regarding the use of "Limbo" .
Looking at the code in jsiOneSecondAfterStartup. (which is not currently in the ESP32 build (but nore is USBSERIAL):

So I assumed that the txbuffer content, with a device flag of 'limbo', was being preserved while in limbo so that the jshTransmitMove(EV_LIMBO, consoleDevice) and subsequent USARTKick , in OneSecondAfterStartup, would then transmit all that was sent while in limbo. But during testing I found jshTransmit() was being called with the device limbo to transmit a load of stuff out. I forced a stack dumb in this case (next pic) and got what follows:

So it looks like in the case of #ifdef USB , jsiInit() having set consoleDevice == EV_LIMBO is then actually transmitting out of the txbuffer. Am I missing something ?

As it stands im thinking to ignore the limbo concept and flip consoleDevice back to DEFAULTCONSOLE, ASAP Here, in the ESP32/jshardware.c :

/**
 * Re-init the ESP32 after a soft-reset
 */
void jshSoftInit() {
 // jsiSetConsoleDevice(DEFAULT_CONSOLE_DEVICE,false);  // SGA TODO Comment here
  if(ESP32_Get_NVS_Status(ESP_NETWORK_WIFI)) jswrap_esp32_wifi_soft_init();
}

which is convieniently called in jsiSemiInit() in the case of #ifdef ESP32, after JSInit() sets console to limbo but before any serious jsiConsolePrintf()'s in the whole initialisation.
I will carry on.

[SimonGAndrews]

Sorry I should have started with my issue is that I was seeing jshUSARTKick being called with device EV_LIMBO . Its not so much that jshTransmit was being called with it. I suppose jshTransmit is the best place to trap and handle any transmission while in Limbo. My question is , I do I understand this correctly, in that calls to jshUSART with device limbo should not be happenning.

[SimonGAndrews]

@gfwilliams sorry to drag you into this. And to save on iterations. Having considered further , unless I’ve got wrong end of stick, my proposal is to modify jsiinit()

#if defined(LINUX) || !defined(USB)
  consoleDevice = jsiGetPreferredConsoleDevice();
#else
  consoleDevice = EV_LIMBO;
#endif 

To:

#if defined(LINUX) || !defined(USB) || defined(ESP32)
  consoleDevice = jsiGetPreferredConsoleDevice();
#else
  consoleDevice = EV_LIMBO;
#endif

So that setting consoleDevice = EV_Limbo does not apply in the case of ESP32 and the esp32 build continues to not have the 'Limbo' state after the mod to add USB devices and a call to jsiOneSecondAfterStartup() will not be required (As now)

[gfwilliams]

Thanks for looking into this.

My question is , I do I understand this correctly, in that calls to jshUSART with device limbo should not be happening.

I feel like it's fine to call jshUSARTKick on EV_LIMBO, but jshUSARTKick should just do nothing in that case (so they shouldn't take data out and try and transmit it).

Is there a case in ESP32 where data sent to USB immediately could get lost if we send it out too quickly? I guess not - so if that's the case we could do the defined(ESP32) as you say...

If jshUSARTKick on EV_LIMBO happens on other platforms it's something I'll have to fix myself anyway though