Skip to content

cpu/cc2538: Add periph_uart_mode implementation#11503

Merged
leandrolanzieri merged 1 commit intoRIOT-OS:masterfrom
MrKevinWeiss:pr/cc2538/uartmode
May 29, 2019
Merged

cpu/cc2538: Add periph_uart_mode implementation#11503
leandrolanzieri merged 1 commit intoRIOT-OS:masterfrom
MrKevinWeiss:pr/cc2538/uartmode

Conversation

@MrKevinWeiss
Copy link
Copy Markdown
Contributor

Contribution description

This pr adds the periph_uart_mode USEMODULE
It implements all functionality defined in the common uart driver
This means all parity modes, data bits, and stop bits

Testing procedure

Run USEMODULE=periph_uart_mode BOARD=remote-revb make flash term -C tests/periph_uart/
In the terminal run
init 1 115200
and change all different modes (databits 5-8, parity n/e/o/m/s, and stop bits 1/2)
eg. mode 1 5 o 2
note: the API says that init must be called before mode however this implementation allows the user to also call mode at any time
Verify with a scope or analyser that the changes work/
Verify loopback works as well.

Issues/PRs references

Implementation based off RFC #10743
Taken over from #5899

@MrKevinWeiss MrKevinWeiss added Type: new feature The issue requests / The PR implemements a new feature for RIOT Impact: minor The PR is small in size and might only require a quick look of a knowledgeable reviewer CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Area: cpu Area: CPU/MCU ports labels May 9, 2019
@MrKevinWeiss MrKevinWeiss requested review from aabadie and smlng May 9, 2019 10:25
@MrKevinWeiss
Copy link
Copy Markdown
Contributor Author

ping @yegorich

} uart_conf_t;
/** @} */

/**
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Without #ifndef DOXYGEN the UART API documentation is broken i.e. you have redundant defines.

What about excluding the redefines for now till there is a final solution for this issue.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done

@MrKevinWeiss MrKevinWeiss force-pushed the pr/cc2538/uartmode branch from dcd596b to 952cccd Compare May 9, 2019 10:59
Copy link
Copy Markdown
Contributor

@yegorich yegorich left a comment

Choose a reason for hiding this comment

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

Looks good to me. ACK

{
assert(uart < UART_NUMOF);

assert( data_bits == UART_DATA_BITS_5 ||
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

remove space at the beginning and re-align accordingly - apply below too

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Addressed!

This commit adds the periph_uart_mode USEMODULE
It implements all functionality defined in the common uart driver
This means all parity modes, data bits, and stop bits
Copy link
Copy Markdown
Member

@smlng smlng left a comment

Choose a reason for hiding this comment

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

untested ACK, otherwise this looks good

@smlng smlng added Reviewed: 1-fundamentals The fundamentals of the PR were reviewed according to the maintainer guidelines Reviewed: 2-code-design The code design of the PR was reviewed according to the maintainer guidelines Reviewed: 4-code-style The adherence to coding conventions by the PR were reviewed according to the maintainer guidelines labels May 20, 2019
@PeterKietzmann
Copy link
Copy Markdown
Member

@smlng gave his untested ACK. Is anyone willing to test this PR maybe @yegorich, @aabadie?

@leandrolanzieri leandrolanzieri added the Reviewed: 3-testing The PR was tested according to the maintainer guidelines label May 29, 2019
Copy link
Copy Markdown
Contributor

@leandrolanzieri leandrolanzieri left a comment

Choose a reason for hiding this comment

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

I tested this, works well. ACK.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area: cpu Area: CPU/MCU ports CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Impact: minor The PR is small in size and might only require a quick look of a knowledgeable reviewer Reviewed: 1-fundamentals The fundamentals of the PR were reviewed according to the maintainer guidelines Reviewed: 2-code-design The code design of the PR was reviewed according to the maintainer guidelines Reviewed: 3-testing The PR was tested according to the maintainer guidelines Reviewed: 4-code-style The adherence to coding conventions by the PR were reviewed according to the maintainer guidelines Type: new feature The issue requests / The PR implemements a new feature for RIOT

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants