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

Add half-duplex support to HardwareSerial. #643

Merged
merged 1 commit into from Oct 11, 2019

Conversation

@ghent360
Copy link
Contributor

@ghent360 ghent360 commented Sep 9, 2019

Summary
TMC2209 and 2208 drivers use UART communication with a single wire. Enabling half-duplex support in the HardwareSerial class would enable using these drivers without wasting another pin for the RX signal.

Proposed API is to enable half-duplex mode if the RX pin on the serial port is NC. Also added a method to set the half-duplex mode explicitly.

Validation

@fpistm fpistm added this to In progress in STM32 core based on ST HAL via automation Sep 9, 2019
STM32 core based on ST HAL automation moved this from In progress to Needs review Sep 9, 2019
Copy link
Member

@fpistm fpistm left a comment

Hi @ghent360
Thanks for the PR.
Some minor remarks.

Loading

cores/arduino/stm32/uart.c Outdated Show resolved Hide resolved
Loading
cores/arduino/stm32/uart.c Outdated Show resolved Hide resolved
Loading
cores/arduino/stm32/uart.c Outdated Show resolved Hide resolved
Loading
@AnHardt
Copy link
Contributor

@AnHardt AnHardt commented Sep 9, 2019

Proposed API is to enable half-duplex mode if the RX pin on the serial port is NC. Also added a method to set the half-duplex mode explicitly.

Could also be nice if TX_pin == RX_pin would set up half-duplex.

Loading

@fpistm
Copy link
Member

@fpistm fpistm commented Sep 9, 2019

Proposed API is to enable half-duplex mode if the RX pin on the serial port is NC. Also added a method to set the half-duplex mode explicitly.

Could also be nice if TX_pin == RX_pin would set up half-duplex.

With a SoftwareSerial this could be possible but with HardwareSerial there no U(S)ART RX and TX on the same pin, as far as I know.

Loading

Implement suggestions from fpistm.
Fix style issues.
@fpistm
Copy link
Member

@fpistm fpistm commented Sep 14, 2019

Thanks @ghent360 , I have to test it. Do you have a sketch example to test this? Else I will write one.

Loading

@fpistm fpistm added this to the 1.8.0 milestone Sep 25, 2019
@fpistm
Copy link
Member

@fpistm fpistm commented Oct 10, 2019

Proposed API is to enable half-duplex mode if the RX pin on the serial port is NC. Also added a method to set the half-duplex mode explicitly.

Could also be nice if TX_pin == RX_pin would set up half-duplex.

With a SoftwareSerial this could be possible but with HardwareSerial there no U(S)ART RX and TX on the same pin, as far as I know.

My bad, after some read some documentations this would be possible.
I'm currently reviewing this PR and it seems there is an issue on how the Tx pin is configured.
Documentation recommend this:

The USART can be configured to follow a single-wire half-duplex protocol where the Tx and Rx lines are internally connected. In this communication mode, only the Tx pin is used forboth transmission and reception.
The Tx pin is always released when no data is transmitted, thus, it acts as a standard I/O in idle or
reception modes.
This means that the I/O must be configured so that the Tx pin is configured as an alternate function open-drain with an external pull-up.

Currently this is not the case as by default it is set in PushPull

Loading

fpistm
fpistm approved these changes Oct 11, 2019
Copy link
Member

@fpistm fpistm left a comment

I will provide a PR to extend how enable half-duplex.
And a small fix when Serial is disabled.

Loading

STM32 core based on ST HAL automation moved this from Needs review to Reviewer approved Oct 11, 2019
@fpistm fpistm merged commit 2a64163 into stm32duino:master Oct 11, 2019
1 check passed
Loading
STM32 core based on ST HAL automation moved this from Reviewer approved to Done Oct 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants