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

3.0.0 - UART Peripheral Manager + Detach UART pins on begin()/setPins() #8719

Conversation

SuGlider
Copy link
Collaborator

@SuGlider SuGlider commented Oct 5, 2023

Description of Change

This PR implements the same changes as described in #8629
It also adds to the UART Peripheral Manager the control over UART pin and sets default console UART0 RX/TX.

The BareMinimum.ino example (empty sketch) will report RX/TX correctly when Verbose/Debug mode are set by using the Chip Debug Report feature.

Summary of changes:

  • It is now possible to dettach UART0 pins by calling Serial.end() with no previous begin()
  • It is now possible to call setPins() before begin() or in any order
  • setPins() will detach any previous pins that have been changed
  • begin(baud, rx, tx) will detach any previous attached pins
  • setPins() or begin(baud, rx, tx), when called at first, will detach console RX0/TX0, attached in boot
  • any pin set as -1 in begin() or setPins() won't be changed nor detached
  • begin(baud) will not changed any pins that have been set before this call through a previous begin(baud, rx, tx) or setPin()
  • If the application only uses RX or TX, begin(baud, -1, tx) or begin(baud, rx) will change only the assigned pin and keep the other unchanged.

Tests scenarios

Using ESP32

void setup() {
  printf("\nThis Line is printed fine because it uses TX Pin attached in Boot Time.\n\n");
  delay(100);
  Serial.end();                 // shall detach default UART0 RX/TX
  printf("1:: This Log line shall not be displayed!\n"); // using IDF console ets_printf() -- TX unattached!
  delay(100);
  
  Serial.begin(115200);  // set default RX/TX pin because nothing was defined in the call
  Serial.println("2:: This line shall be seen.");
  Serial.flush();

  Serial.setPins(-1, 2);    // shall set UART0 to GPIO2 and detach default TX 
  Serial.println("3:: This line shall NOT be seen.");  // default TX is no longer attached
  Serial.flush();
  
  Serial.end();
  Serial.setPins(-1, 2); // it sets TX as GPIO2
  Serial.begin(115200);  // This shall not set default TX pin because the previous setPin() was executed
  Serial.println("4:: This line shall NOT be seen.");  // default TX is no longer attached
  Serial.flush();
}

void loop() {}

Related links

Fix #8573
Fix #8607

Close Draft PRs: #8619 #8620

@me-no-dev me-no-dev merged commit 6b1933b into espressif:master Oct 6, 2023
30 checks passed
@TD-er
Copy link
Contributor

TD-er commented Oct 12, 2023

#define UART_MUTEX_LOCK()    if(uart->lock != NULL) do {} while (xSemaphoreTake(uart->lock, portMAX_DELAY) != pdPASS)
#define UART_MUTEX_UNLOCK()  if(uart->lock != NULL) xSemaphoreGive(uart->lock)

You are setting a semaphore with a timeout. But retry again as long as it fails.
Effectively this is the same as an infinite timeout.

Wouldn't it make more sense to return false in such cases where it is called?
Thus as macro parameter the return value when it fails or empty if you should not return any value (or void??) like in uartEnd

Something like this:

#define UART_MUTEX_LOCK(T_ERR)    if(uart->lock != NULL) { if (xSemaphoreTake(uart->lock, portMAX_DELAY) != pdPASS) return T_ERR; }

I see where this may be an issue, as you sometimes need to wait quite a long time to get data flushed (especially at low baud rates), but wouldn't it make more sense to have the user retry every now and then instead of potentially causing deadlocks?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Peripheral: UART Status: Pending Merge Pull Request is ready to be merged
Projects
5 participants