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

[UART/SERIAL] CTS / RTS pins were swapped in this API #6816

Merged
merged 1 commit into from May 27, 2022

Conversation

mazgch
Copy link
Contributor

@mazgch mazgch commented May 27, 2022

Description of Change

espressif/esp-idf / components/driver/include/driver/uart.h defines the API:
esp_err_t uart_set_pin(uart_port_t uart_num, int tx_io_num, int rx_io_num, int rts_io_num, int cts_io_num);

uartSetPins uses that api but calls it with swapped CTS/RTS pins as its API uses a different pin ordering:
uart_set_pin(uart->num, txPin, rxPin, ctsPin, rtsPin);

This fixes the wrong order in the function uartSetPins

Users that have so far successfully used the API so far will need to change their code. Another possibility would be to swap the arguments in the functions uartSetPins of the cores/esp32/esp32-hal-uart.c driver and up in the HardwareSerial. But then the order of the arguments is less logic: as TX/RX is different in arduino and esp_idf but handled correctly in this function.

espressif/esp-idf / components/driver/include/driver/uart.h defines the API:
esp_err_t uart_set_pin(uart_port_t uart_num, int tx_io_num, int rx_io_num, int **rts_io_num**, int **cts_io_num**);
uartSetPins uses that api but alls it with swapped CTS/RTS pins as its API uses a different pin ordering: 
uart_set_pin(uart->num, txPin, rxPin, **ctsPin**, **rtsPin**); 

This fixes the wrong order in the function uartSetPins
@SuGlider
Copy link
Collaborator

@mazgch - Thanks for the PR! Nice catch!

@SuGlider
Copy link
Collaborator

@me-no-dev - All set. Ready for merging.

@me-no-dev me-no-dev merged commit 5484129 into espressif:master May 27, 2022
@me-no-dev
Copy link
Member

done :) thanks all

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants