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

feat(esp32-s3): Add support for auto-baudrate-detection on S3 #7782

Closed
wants to merge 100 commits into from

Conversation

imwhocodes
Copy link

Add support for auto-baudrate-detection on S3 variant of esp32, with different clock support (could be extended to C3 variant)


Description of Change

Add support (fix hal wrapper) for auto-baudrate-detection to the esp32-S3
Currently there was no support for it and correct codepath was missing (and not even hinting to the missing support like done for the C3)

Added #ifdef for enabling the baudrate detection hardware
Added codepath for correctly computing baudrate given the clock source used for the uart hardware block

Tests scenarios

I have tested my Pull Request on Arduino-esp32 core master with ESP32-S3 Boards

Related links

Close #7718
Progress https://github.com/orgs/espressif/projects/3/views/15?pane=issue&itemId=19207281

Note about calculations and ESP32-C3

I cannot test because i have no board with this chip (esp32-C3) but I'm supposing that the same code could be used also for fixing this:

// ESP32-C3 requires further testing
// Baud rate detection returns wrong values
log_e("ESP32-C3 baud rate detection is not supported.");
return;

In particular S3 and C3 variant support other clock sources other than APB and they are in fact defaulted to XTAL clock as source by Arduino, so the previous problem could be by this line not tacking that in account:

unsigned long baudrate = getApbFrequency() / divisor;

@CLAassistant
Copy link

CLAassistant commented Feb 1, 2023

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
7 out of 9 committers have signed the CLA.

✅ facchinm
✅ pillo79
✅ lucasssvaz
✅ SuGlider
✅ imwhocodes
✅ mrengineer7777
✅ arkhipenko
❌ devrim-oguz
❌ jamesarm97
You have signed the CLA already but the status is still pending? Let us recheck it.

@SuGlider
Copy link
Collaborator

SuGlider commented Feb 1, 2023

CLA assistant check Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.

Luca seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

@imwhocodes - Please check the CLA and if you agree, please sign it.

@imwhocodes
Copy link
Author

imwhocodes commented Feb 1, 2023

CLA assistant check Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
Luca seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

@imwhocodes - Please check the CLA and if you agree, please sign it.

@SuGlider I accepted multiple times the CLA, if i click on the link it say 'You have agreed to the CLA for espressif/arduino-esp32' but yet if i come here it tell me I did't signed it yet

Edit: OK I pushed my first commit from a pc with the author/email configured wrongly

@SuGlider
Copy link
Collaborator

SuGlider commented Feb 5, 2023

@imwhocodes - Please check the CLA and if you agree, please sign it.

@SuGlider I accepted multiple times the CLA, if i click on the link it say 'You have agreed to the CLA for espressif/arduino-esp32' but yet if i come here it tell me I did't signed it yet

Edit: OK I pushed my first commit from a pc with the author/email configured wrongly

@imwhocodes

I'm sorry, but it seems that CI won't let it pass because of commit 16ade7e
It was done by another user (the wrongly configured one) and because it has no CLA/valid Github user, it won't be able to pass CI and then we can't merge it after reviewing it.

Please close this PR and open a new one with a single commit from you valid GH user ID.

@VojtechBartoska VojtechBartoska added the Resolution: Awaiting response Waiting for response of author label Feb 6, 2023
Copy link
Collaborator

@SuGlider SuGlider left a comment

Choose a reason for hiding this comment

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

@imwhocodes

I'm sorry but this PR doesn't work for ESP32-S3. Neither for the ESP32-C3.
The baud rate detection only works for the ESP32 and ESP32-S2

This is the sketch I have used to test it:

// This sketch works correctly with ESP32 and ESP32S2 only, so far.
// ESP32C3 and ESP32S3 seems to fail with baud rate detection
void setup() {
  Serial.begin(115200);
  Serial.println("\nTesting baud rate detection\n");
}

void loop() {
  Serial.println("Started detection for next 20 seconds.");
  Serial.println("Use the Serial Monitor. Change the baud rate in the botton of the window.");
  Serial.println("Send characters using the Serial Monitor and get the detected baud rate.");
  Serial.flush();

  Serial.begin(0);
  unsigned long detectedBaudRate = Serial.baudRate();

  if (detectedBaudRate) {
    Serial.printf("\nDetected baudrate is %lu\n\n", detectedBaudRate);
  } else {
    // this may not display correctly in the Serial Monitor, given that it failed.
    Serial.println("\nNo baudrate detected, Serial will not work!\n\n");
  }
}

@SuGlider
Copy link
Collaborator

@VojtechBartoska @me-no-dev
I have tried several ways for a couple days to make it work, but it seems that both C3 and S3 can't make it work correctly.
Therefore, the current code is valid and works fine only for the ESP32 and ESP32S2.

@imwhocodes
Copy link
Author

@SuGlider I don't have a chip at hands right now, as I said in the issue I have this code running on production and I can confirm it working, as I said heavily tested on ESP32-WROOM-32UE and ESP32-S3-WROOM-1U-N8 at both 115200 and 460800 (other baudrates tested but not in production)

Two things:

  • On my test I have around 100 consecutive bytes every 200ms in the least busy moment
  • Could it be that on your configuration Serial is actually by default HWCDC (the usb one)

Let me know

@SuGlider
Copy link
Collaborator

SuGlider commented Feb 13, 2023

@SuGlider I don't have a chip at hands right now, as I said in the issue I have this code running on production and I can confirm it working, as I said heavily tested on ESP32-WROOM-32UE and ESP32-S3-WROOM-1U-N8 at both 115200 and 460800 (other baudrates tested but not in production)

It works correctly for any baud rate on the ESP32-WROOM-32, but not on the ESP32-S3-WROOM.
Using the testing sketch I had posted here, with the ESP32-S3, it works for a few baud rates, but not for any.
Therefore, it can't be approved for the S3.

Two things:

  • On my test I have around 100 consecutive bytes every 200ms in the least busy moment

I tried changing the number of bits read and other parameters with no success - using the S3.

  • Could it be that on your configuration Serial is actually by default HWCDC (the usb one)

I always tested the baud detection using UART.
HWCDC has not such API, therefore it would never get compiled.

@imwhocodes - If you have a working sketch that demonstrates this PR working correctly for any baud rate, please post it here with instructions on how to replicate the test case. I'll happily try it again.

@imwhocodes
Copy link
Author

@SuGlider I asked about HWCDC/USB only to be sure (the sketch/example provided by you should compile also for both type of serial)

I will try your sketch and explore the the influence of the length of consecutive bytes

Also I check and in my code the operations that I do are:

  1. HardwareSerial::begin(9600, SERIAL_8N1, ... )
  2. uartStartDetectBaudrate(uart_t)
  3. uartStartDetectBaudrate(uart_t)
  4. uartSetBaudRate(uart_t, unsigned long)

(I'm not using HardwareSerial::begin(0, .... ) simply because I want to have unlimited timeout in case of no uart connected yet, but as I said baud are correctly recognised)

@SuGlider
Copy link
Collaborator

SuGlider commented Feb 13, 2023

(I'm not using HardwareSerial::begin(0, .... ) simply because I want to have unlimited timeout in case of no uart connected yet, but as I said baud are correctly recognised)

Ok. Please provide an Arduino example code that I can test here.

from https://github.com/espressif/arduino-esp32/blob/master/cores/esp32/HardwareSerial.h#L25-L42

 Modified 13 October 2018 by Jeroen Döll (add baudrate detection)
 Baudrate detection example usage (detection on Serial1):
   void setup() {
     Serial.begin(115200);
     delay(100);
     Serial.println();
     Serial1.begin(0, SERIAL_8N1, -1, -1, true, 11000UL);  // Passing 0 for baudrate to detect it, the last parameter is a timeout in ms
     unsigned long detectedBaudRate = Serial1.baudRate();
     if(detectedBaudRate) {
       Serial.printf("Detected baudrate is %lu\n", detectedBaudRate);
     } else {
       Serial.println("No baudrate detected, Serial1 will not work!");
     }
   }
 Pay attention: the baudrate returned by baudRate() may be rounded, eg 115200 returns 115201

@SuGlider
Copy link
Collaborator

Also I check and in my code the operations that I do are:

  1. HardwareSerial::begin(9600, SERIAL_8N1, ... )
  2. uartStartDetectBaudrate(uart_t)
  3. uartStartDetectBaudrate(uart_t)
  4. uartSetBaudRate(uart_t, unsigned long)

uart_t * isn't exposed by any Arduino API.
Maybe the code you used isn't Arduino Code, but some sort of modification.

This PR must work using only Arduino API from HardwareSerial class.

@me-no-dev me-no-dev removed this from the 2.0.7 milestone Feb 13, 2023
@imwhocodes
Copy link
Author

@SuGlider I run some test yesterday and indeed your example don't work

Something strange is happening, if you call HardwareSerial::begin(0 , ....) baud rate detected is double the one the wire, like if the UART block is gated by APB_CLK (usually 80Mhz) and not by XTAL_CLK (40Mhz)

What I'm doing on my prod codebase is adding this function to HardwareSerial (in prod codebase I'm inheriting the class )
It is simply the core detection part from HardwareSerial::begin

unsigned long HardwareSerial::detectBaudRate(const unsigned long timeout_ms) //timeout 0 for infinite timeout
{
    uartStartDetectBaudrate(_uart);

    unsigned long detectedBaudRate = 0;

    unsigned long start_detection = millis();

    while( ( ( ( millis() - start_detection ) < timeout_ms ) || timeout_ms == 0 ) && ( !( detectedBaudRate = uartDetectBaudrate(_uart) ) ) ){
        delay(1);
    };

    return detectedBaudRate;
}

And the with:

#include <Arduino.h>
void setup() {
  Serial.begin(230400);
  Serial.println("\nTesting baud rate detection\n");
}

void loop() {
  Serial.println("Started detection for next 20 seconds.");
  Serial.println("Use the Serial Monitor. Change the baud rate in the botton of the window.");
  Serial.println("Send characters using the Serial Monitor and get the detected baud rate.");
  Serial.flush();

  unsigned long detectedBaudRate = Serial.detectBaudRate(20000);
  // unsigned long detectedBaudRate = Serial.baudRate();


  if (detectedBaudRate) {
    Serial.updateBaudRate(detectedBaudRate);
    Serial.printf("\nDetected baudrate is %lu\n\n", detectedBaudRate);
  } else {
    // Serial.updateBaudRate(115200);
    // this may not display correctly in the Serial Monitor, given that it failed.
    Serial.println("\nNo baudrate detected, Serial will not work!\n\n");
  }
}

And baud rate is correctly detected

Strange thing is that it keep detecting (seem like hardware register with values for detection are not cleared)

Copy link
Contributor

github-actions bot commented Feb 6, 2024

Warnings
⚠️

Some issues found for the commit messages in this PR:

  • the commit message "Add Arduino Nano ESP32 target (#8417)":
    • summary looks empty
    • type/action looks empty
  • the commit message "Add Arduino Nano ESP32 target (#8417)":
    • summary looks empty
    • type/action looks empty
  • the commit message "Add required callbacks for TinyUSB DFU":
    • summary looks empty
    • type/action looks empty
  • the commit message "Add required callbacks for TinyUSB DFU":
    • summary looks empty
    • type/action looks empty
  • the commit message "Add setMode function HardwareSerial.c to set the esp32 uart mode for use with RS485 auto RTS (#7935)":
    • body's lines must not be longer than 100 characters
    • summary looks empty
    • type/action looks empty
  • the commit message "Add setMode function HardwareSerial.c to set the esp32 uart mode for use with RS485 auto RTS (#7935)":
    • body's lines must not be longer than 100 characters
    • summary looks empty
    • type/action looks empty
  • the commit message "Add support for esp-elf-gdb":
    • summary looks empty
    • type/action looks empty
  • the commit message "Add support for esp-elf-gdb":
    • summary looks empty
    • type/action looks empty
  • the commit message "BugFix FlashStringHelper Macros (#8143)":
    • summary looks empty
    • type/action looks empty
  • the commit message "BugFix FlashStringHelper Macros (#8143)":
    • summary looks empty
    • type/action looks empty
  • the commit message "Disable Ethernet library if CONFIG_ETH_ENABLED not defined in sdkconfig.h (#8595)":
    • summary looks empty
    • type/action looks empty
  • the commit message "Enable DFU":
    • summary looks empty
    • type/action looks empty
  • the commit message "Enable DFU":
    • summary looks empty
    • type/action looks empty
  • the commit message "Fix compilation issue caused by ESP-Insights":
    • summary looks empty
    • type/action looks empty
  • the commit message "Fix compilation issue caused by ESP-Insights":
    • summary looks empty
    • type/action looks empty
  • the commit message "Fix escaping issues in 2.0.10 (#8433)":
    • summary looks empty
    • type/action looks empty
  • the commit message "Fix escaping issues in 2.0.10 (#8433)":
    • summary looks empty
    • type/action looks empty
  • the commit message "Fix pin attachInterrupt(digitalPinToInterrupt(48)) on all S3 based SOCs (#8600)":
    • probably contains Jira ticket reference (ESP-100). Please remove Jira tickets from commit messages.
    • summary looks empty
    • type/action looks empty
  • the commit message "Fix the F_CPU frequency definition for the ESP32-S3 in esp32-hal.h (#7913)":
    • footer's lines must not be longer than 100 characters
    • summary looks empty
    • type/action looks empty
  • the commit message "HTTPClient - Fix case sensitiveness for header keys (#8632)":
    • summary looks empty
    • type/action looks empty
  • the commit message "I2S - Revert espressif/esp-idf@73ca054 to fix I2S crackling (#8583)":
    • summary looks empty
    • type/action looks empty
  • the commit message "I2S - Revert espressif/esp-idf@73ca054 to fix I2S crackling (#8583)":
    • summary looks empty
    • type/action looks empty
  • the commit message "Nano ESP32 file system option (#8566)":
    • summary looks empty
    • type/action looks empty
  • the commit message "Nano ESP32 file system option (#8566)":
    • summary looks empty
    • type/action looks empty
  • the commit message "Nano ESP32: add debug support (#8567)":
    • summary looks empty
    • type/action looks empty
  • the commit message "Nano ESP32: add debug support (#8567)":
    • summary looks empty
    • type/action looks empty
  • the commit message "Nano ESP32: add pin numbering option (#8565)":
    • summary looks empty
    • type/action looks empty
  • the commit message "Nano ESP32: add pin numbering option (#8565)":
    • summary looks empty
    • type/action looks empty
  • the commit message "Nano ESP32: fix digital, analog and GPIO pin counts (#8586)":
    • summary looks empty
    • type/action looks empty
  • the commit message "Nano ESP32: fix digital, analog and GPIO pin counts (#8586)":
    • summary looks empty
    • type/action looks empty
  • the commit message "Remove insights component since it's provided by RainMaker":
    • summary looks empty
    • type/action looks empty
  • the commit message "Remove insights component since it's provided by RainMaker":
    • summary looks empty
    • type/action looks empty
  • the commit message "Set ref to 2.0.13 (#8615)":
    • summary looks empty
    • type/action looks empty
  • the commit message "Uart detach 2.0.13 (#8629)":
    • summary looks empty
    • type/action looks empty
  • the commit message "Update ESP-IDF libs to v4.4.4":
    • summary looks empty
    • type/action looks empty
  • the commit message "Update ESP-IDF libs to v4.4.4":
    • summary looks empty
    • type/action looks empty
  • the commit message "Update ESP-IDF to v4.4.5":
    • summary looks empty
    • type/action looks empty
  • the commit message "Update ESP-IDF to v4.4.5":
    • summary looks empty
    • type/action looks empty
  • the commit message "Update ESP-IDF to v4.4.6 (with I2S Rollback) (#8701)":
    • summary looks empty
    • type/action looks empty
  • the commit message "Update RainMaker and components":
    • summary looks empty
    • type/action looks empty
  • the commit message "Update RainMaker and components":
    • summary looks empty
    • type/action looks empty
  • the commit message "Update core version to 2.0.10":
    • summary looks empty
    • type/action looks empty
  • the commit message "Update core version to 2.0.10":
    • summary looks empty
    • type/action looks empty
  • the commit message "Update core version to 2.0.14":
    • summary looks empty
    • type/action looks empty
  • the commit message "Update esptool to v4.4":
    • summary looks empty
    • type/action looks empty
  • the commit message "Update esptool to v4.4":
    • summary looks empty
    • type/action looks empty
  • the commit message "Update esptool to v4.5":
    • summary looks empty
    • type/action looks empty
  • the commit message "Update esptool to v4.5":
    • summary looks empty
    • type/action looks empty
  • the commit message "Update esptool to v4.5.1":
    • summary looks empty
    • type/action looks empty
  • the commit message "Update esptool to v4.5.1":
    • summary looks empty
    • type/action looks empty
  • the commit message "Update get.py to support Apple ARM64":
    • summary looks empty
    • type/action looks empty
  • the commit message "Update get.py to support Apple ARM64":
    • summary looks empty
    • type/action looks empty
  • the commit message "Update package version":
    • summary looks empty
    • type/action looks empty
  • the commit message "Update package version":
    • summary looks empty
    • type/action looks empty
  • the commit message "Update package_esp32_index.template.json":
    • summary looks empty
    • type/action looks empty
  • the commit message "Update package_esp32_index.template.json":
    • summary looks empty
    • type/action looks empty
  • the commit message "Update toolchains and openocd for ESP-IDF v4.4.4":
    • summary looks empty
    • type/action looks empty
  • the commit message "Update toolchains and openocd for ESP-IDF v4.4.4":
    • summary looks empty
    • type/action looks empty
  • the commit message "Update version to 2.0.11":
    • summary looks empty
    • type/action looks empty
  • the commit message "Update version to 2.0.11":
    • summary looks empty
    • type/action looks empty
  • the commit message "Update version to 2.0.8":
    • summary looks empty
    • type/action looks empty
  • the commit message "Update version to 2.0.8":
    • summary looks empty
    • type/action looks empty
  • the commit message "Update version to 2.0.9":
    • summary looks empty
    • type/action looks empty
  • the commit message "Update version to 2.0.9":
    • summary looks empty
    • type/action looks empty
  • the commit message "WFG Crashfix (#8043)":
    • summary looks empty
    • type/action looks empty
  • the commit message "WFG Crashfix (#8043)":
    • summary looks empty
    • type/action looks empty
  • the commit message "WIP, need cleaning":
    • summary looks empty
    • type/action looks empty
  • the commit message "WiFiUDF Low memory fix (#8065)":
    • body's lines must not be longer than 100 characters
    • summary looks empty
    • type/action looks empty
  • the commit message "WiFiUDF Low memory fix (#8065)":
    • body's lines must not be longer than 100 characters
    • summary looks empty
    • type/action looks empty
  • the commit message "avoid buffer overflow in debug":
    • summary looks empty
    • type/action looks empty
  • the commit message "feat(esp32-s3): Add support for auto-baudrate-detection on S3 variant of Esp32, with different clock support":
    • summary appears to be too long
  • the commit message "fix attach pin for S3":
    • summary looks empty
    • type/action looks empty
  • the commit message "fix attach pin for S3":
    • summary looks empty
    • type/action looks empty
  • the commit message "io_pin_remap fixes for the Arduino Nano ESP32 (#8489)":
    • summary looks empty
    • type/action looks empty
  • the commit message "io_pin_remap fixes for the Arduino Nano ESP32 (#8489)":
    • summary looks empty
    • type/action looks empty
  • the commit message "searching for failure of detection on 3686400 bauds":
    • summary looks empty
    • type/action looks empty
  • the commit message "stress/fuzzing testing":
    • summary looks empty
    • type/action looks empty

Please fix these commit messages - here are some basic tips:

  • follow Conventional Commits style
  • correct format of commit message should be: <type/action>(<scope/component>): <summary>, for example fix(esp32): Fixed startup timeout issue
  • allowed types are: change,ci,docs,feat,fix,refactor,remove,revert,test
  • sufficiently descriptive message summary should be between 20 to 72 characters and start with upper case letter
  • avoid Jira references in commit messages (unavailable/irrelevant for our customers)

TIP: Install pre-commit hooks and run this check when committing (uses the Conventional Precommit Linter).

⚠️ Please consider squashing your 100 commits (simplifying branch history).
⚠️

The source branch "S3-autobaud" incorrect format:

  • contains uppercase letters. This can cause troubles on case-insensitive file systems (macOS).
    Please rename your branch.
Messages
📖 This PR seems to be quite large (total lines of code: 100009), you might consider splitting it into smaller PRs

👋 Hello imwhocodes, we appreciate your contribution to this project!


Click to see more instructions ...


This automated output is generated by the PR linter DangerJS, which checks if your Pull Request meets the project's requirements and helps you fix potential issues.

DangerJS is triggered with each push event to a Pull Request and modify the contents of this comment.

Please consider the following:
- Danger mainly focuses on the PR structure and formatting and can't understand the meaning behind your code or changes.
- Danger is not a substitute for human code reviews; it's still important to request a code review from your colleagues.
- Resolve all warnings (⚠️ ) before requesting a review from human reviewers - they will appreciate it.
- Addressing info messages (📖) is strongly recommended; they're less critical but valuable.
- To manually retry these Danger checks, please navigate to the Actions tab and re-run last Danger workflow.

Review and merge process you can expect ...


We do welcome contributions in the form of bug reports, feature requests and pull requests.

1. An internal issue has been created for the PR, we assign it to the relevant engineer.
2. They review the PR and either approve it or ask you for changes or clarifications.
3. Once the GitHub PR is approved we do the final review, collect approvals from core owners and make sure all the automated tests are passing.
- At this point we may do some adjustments to the proposed change, or extend it by adding tests or documentation.
4. If the change is approved and passes the tests it is merged into the default branch.

Generated by 🚫 dangerJS against bf39067

@Jason2866
Copy link
Collaborator

Is the auto detection working now and if yes for all speeds too?
If not the PR should imho be changed to draft.

@imwhocodes imwhocodes marked this pull request as draft February 6, 2024 13:33
@imwhocodes
Copy link
Author

Is the auto detection working now and if yes for all speeds too? If not the PR should imho be changed to draft.

It should, I wanted to write before but i didn't had time yet
It run (on production) on some of my project, I will run a new test suite on it and come back with a final PR

@imwhocodes imwhocodes reopened this Feb 9, 2024
@imwhocodes
Copy link
Author

imwhocodes commented Feb 9, 2024

Master branch do not compile (on platformIO) so I based my test on top of v2.0.14

Next week I will clean up the code and do more tests (now it work but btw)

Here is a code snippet that I used to test the auto-baud function (and connecting TX of Serial1 to RX of Serial2):

#define NO_GLOBAL_SERIAL

#include <Arduino.h>

HardwareSerial log_stream = HardwareSerial(0);
HardwareSerial snd_serial = HardwareSerial{1};
HardwareSerial rcv_serial = HardwareSerial{2};


static const unsigned long default_rates[] = {  
                                                0,        // TEST FOR FALSE POSITIVE
                                                // 600,   // THIS FAIL, PULSE COUNTER IS LIMITED AT 4095
                                                // 1200,  // THIS FAIL, PULSE COUNTER IS LIMITED AT 4095
                                                // 2400,  // THIS FAIL, PULSE COUNTER IS LIMITED AT 4095
                                                4800,
                                                9600,
                                                19200,
                                                38400,
                                                57600,
                                                74880,
                                                115200,
                                                230400,
                                                256000,
                                                460800,
                                                921600,
                                                1843200,
                                                // 3686400  // THIS ALWAYS FAIL, Need investigation
                                              };

#define ARRAY_SIZE(arr)     (sizeof(arr) / sizeof((arr)[0]))


TaskHandle_t parse_task;

TaskHandle_t send_task;

void sendTask(void *pvParameters){

  while(true){

    log_stream.println("\n\n\n");

    const auto & rate = default_rates[random(0, ARRAY_SIZE(default_rates) - 1)];

    if(rate){
      log_stream.printf("SND opening with: %d bauds\n", rate);

      snd_serial.begin(rate, SERIAL_8N1, 0, 1, false);

      xTaskNotify(parse_task, rate, eSetValueWithOverwrite);

      const size_t qty = random(128, 1024);

      log_stream.printf("SND will send: %d bytes\n", qty);

      for(size_t i = 0; i < qty; i++){
        const uint8_t b = random();
        snd_serial.write(b);
      }

      snd_serial.flush();

      log_stream.println("SND Finished");
    }
    else{
      log_stream.println("SND Not Opening");
      xTaskNotify(parse_task, rate, eSetValueWithOverwrite);
    }

    uint32_t rcv_rate;
    xTaskNotifyWait(0, 0, &rcv_rate, portMAX_DELAY);

    delay(1000);

  }

}



void parseTask(void *pvParameters){


  while(true){

    uint32_t should_be;

    xTaskNotifyWait(0, 0, &should_be, portMAX_DELAY);

    rcv_serial.begin(0, SERIAL_8N1, 45, 46, false, 5000);
    const auto detected_rate = rcv_serial.baudRate();

    if(should_be){
      if(detected_rate){

        const float_t min = should_be * 0.9;
        const float_t max = should_be * 1.1;

        const bool valid = ( detected_rate > min ) && ( detected_rate < max );

        if(valid){
          log_stream.printf("RCV SUCCESS!\t%d\tis\t%d\n", detected_rate, should_be);;
          xTaskNotify(send_task, detected_rate, eSetValueWithOverwrite);
        }
        else{
          log_stream.printf("RCV FAIL\t%d\tis not\t%d\n", detected_rate, should_be);
        }
      }
      else{
        log_stream.println("RCV FAILED DETECTION!!!");
      }    
    }
    else{
      log_stream.println("RCV SUCCESS! (nothing was send)");
      xTaskNotify(send_task, detected_rate, eSetValueWithOverwrite);
    }

  }

}

void setup() {

    log_stream.begin(921600);

    delay(100);
    while (!log_stream)
    {
      delay(100);
    }
    
    log_stream.setDebugOutput(true);

    log_i("!INIT!");

    log_i("Connected!");

  	assert( pdPASS ==
      xTaskCreatePinnedToCore(
                  sendTask,         /* Function to implement the task */
                  "sendTask",       /* Name of the task */
                  1024 * 4,         /* Stack size in bytes */
                  NULL,             /* Task input parameter */
                  5,                /* Priority of the task */
                  &send_task,       /* Task handle. */
                  PRO_CPU_NUM       /* Core where the task should run */
      )
	  );

  	assert( pdPASS ==
      xTaskCreatePinnedToCore(
                  parseTask,        /* Function to implement the task */
                  "parseTask",      /* Name of the task */
                  1024 * 4,         /* Stack size in bytes */
                  NULL,             /* Task input parameter */
                  5,                /* Priority of the task */
                  &parse_task,      /* Task handle. */
                  APP_CPU_NUM       /* Core where the task should run */
      )
	  );
    log_e("Task Created:\t%d", millis());
}


void loop() {
  vTaskDelete(NULL);
}

@imwhocodes imwhocodes changed the title Add support for auto-baudrate-detection on S3 variant of esp32 feat(esp32-s3): Add support for auto-baudrate-detection on S3 Feb 9, 2024
@imwhocodes
Copy link
Author

imwhocodes commented Feb 10, 2024

About the detection of seem like there is something broken on the original rounding algorithm

As you can see from this log:

SND opening with: 1843200 bauds
SND will send: 1018 bytes
[  5760][I][esp32-hal-uart.c:655] uartBaudrateDetect(): rxd_edge_cnt = 46 lowpulse_min_cnt = 9 hightpulse_min_cnt = 10
[  5761][V][esp32-hal-uart.c:830] uartDetectBaudrate(): Rounded 2222222 to      3686400
[  5762][V][esp32-hal-uart.c:831] uartDetectBaudrate(): From Previous   ( 1843200 )     :       ( 379022 )
[  5763][V][esp32-hal-uart.c:832] uartDetectBaudrate(): From Current    ( 3686400 )     :       ( 1464178 )
SND Finished
RCV FAIL        3699421 is not  1843200

2222222 get rounded to 3686400 even if:

abs(1843200 - 2222222) = 379022
abs(3686400 - 2222222) = 1464178

@OpenUAS
Copy link

OpenUAS commented Feb 16, 2024

@imwhocodes Your work is really appreciated in the sad state of affair of Espressif S3/C3 autobauding. A finalized PR welcomed, can validate on all ESP32 MCU if still needed. BTW "Git Squash" welcomed ;)

@SuGlider
Copy link
Collaborator

@imwhocodes - I think that it will be necessary to rebase this PR.
There 100 commits and the mergeable content doesn't match the PR objective.

@imwhocodes
Copy link
Author

@imwhocodes - I think that it will be necessary to rebase this PR. There 100 commits and the mergeable content doesn't match the PR objective.

I will rebase the commit, it is not ready to be merged anyway (currently is rebased on v2.0.14)
@SuGlider I can't change the target branch for the PR (current is master), I would like to be include in the future v2.0.15
I'm not testing/rebasing on master because if I try to compile with PlatformIO I get:
KeyError: 'framework-arduinoespressif32-libs':

My platformio.ini is:

[env:autobaudtest]
platform = https://github.com/platformio/platform-espressif32.git#v6.5.0
board = esp32-s3-devkitc-1
board_build.mcu = esp32s3

framework = arduino

platform_packages = 
	platformio/framework-arduinoespressif32 @ symlink:///<RedactedFolder>/arduino-esp32
	espressif/toolchain-xtensa-esp32 @ 12.2.0+20230208

@Jason2866
Copy link
Collaborator

@imwhocodes You can use branch master with Platformio using this PR platformio/platform-espressif32#1281

@SuGlider
Copy link
Collaborator

SuGlider commented Feb 17, 2024

I will rebase the commit, it is not ready to be merged anyway (currently is rebased on v2.0.14)
@SuGlider I can't change the target branch for the PR (current is master), I would like to be include in the future v2.0.15

@imwhocodes - This PR should be use release/v2.x as branch, not master. This will make it merge against future Core v2.0.15.

I have run exaustive testing and also I have checked ESP32-S3 and ESP32-C3 datasheet and TRM.
My conclusions are:

  • Baudrate detection only works when UART source clock is XTAL. It doesn't work correctly when using APB (this is a problem for Arduino Core 3.0.0 because it sets APB by default).
  • S3 and C3 has a 12 bits resolution, therefore, it can detect only more than 9600 bps when using XTAL.
  • The best baudrate calculation is XTAL Freq 40MHz / (Negative Pulse Count + 1)

In order to get the Negative Pulse Count, the code shall use uart_ll_get_neg_pulse_cnt(uart_dev_t *hw)

additional note:

I found a couple issues when the APB Freq is lower than 80MHz - ESP32 and ESP32-S2.
Therefore, I have created a new PR rebased to be merged to 2.0.15.
I'll try to find a solution for Arduino Core 3.0.0, considering that it uses APB as UART CLK SOURCE for the S3 and C3.

Please check PR #9261 - This works for 2.0.14

@lucasssvaz
Copy link
Collaborator

Closing this in favor of #9261. If you find a solution for 3.0.0, please open a new PR.

@lucasssvaz lucasssvaz closed this Feb 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Chip: ESP32-S3 Issue is related to support of ESP32-S3 Chip Peripheral: UART
Projects
Development

Successfully merging this pull request may close these issues.

Esp32-S3 Baud Rate Detection missing