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
Refactor uart #13585
base: master
Are you sure you want to change the base?
Refactor uart #13585
Conversation
- port related resources (tx/rx/inverted,dma) - are stored sequentially, with space in place of ports that are not enabled (RESOURCE_UART_COUNT + RESOURCE_LPUART_COUNT + RESOURCE_SOFTSERIAL_COUNT) - RESOURCE_UART_OFFSET, RESOURCE_LPUART_OFFSET, RESOURCE_SOFTSERIAL_OFFSET - resource entries are pointing into this array (UART, LPUART, SOFTSERIAL) - both pins and DMA - inverter is supproted only for UART + LPUART (makes sense only for UART, to be removed) - softSerialPinConfig is removed, it is no longer necessary - serialResourceIndex(identifier) is used universally to get correct index into resource array - unified handling of resources where possible - serialOwnerTxRx() + serialOwnerIndex() are used for displaying resources correctly to user. - serialType(identifier) implemented - serialOwnerTxRx / serialOwnerIndex are trivial with it - large switch() statemens are greatly simplified
- drivers/serial_uart_hw.c contains merged serialUART code. Code did not match exactly. Obvious cases are fixed, more complicated use #ifs - pin inversion unified - uartOpen is refactored a bit
- use 'compressed' enum from uartDeviceIdx_e. Only enabled ports are in this enum. - uartDeviceIdx directly indexes uartDevice (no search necessary, no wasted space) - use `serialPortIdentifier_e identifier;` for uartHardware - for DMA remap, define only entries for enabled ports (uartDeviceIdx_e does not exist disabled port)
New implementation is trivial
There is only one IRQ for serial
Generated code to normalize target configuration. jinja2 template is used to generate header file. Port handling is unified a lot. SERIAL_<type><n>_USED 0/1 - always defined, value depends on target configuration SERIAL_<type>_MASK - bitmask of used ports or given type. <port>1 is BIT(0) SERIAL_<type>_COUNT - number of enabled ports of given type SERIAL_<type>_MAX - <index of highest used port> + 1, 0 when no port is enabled
serial_post.h generated it
LPUART is mostly handled as another UART port, this change reflects it
replaces constant that may not match actual array size
Is this useful ?
Do you want to test this code? You can flash it directly from Betaflight Configurator:
WARNING: It may be unstable. Use only for testing! |
A bit bigger PR .. but it removes ~930 lines of code (generated serial_post.h is 530 lines) I can cleanup it a bit or remove more controversial parts. |
include serial_post.h, some ports are defined, so normalization will work fine
#ifdef USE_LPUART1 | ||
DEFA( OWNER_LPUART_TX, PG_SERIAL_PIN_CONFIG, serialPinConfig_t, ioTagTx[SERIAL_UART_COUNT], SERIAL_LPUART_COUNT ), | ||
DEFA( OWNER_LPUART_RX, PG_SERIAL_PIN_CONFIG, serialPinConfig_t, ioTagRx[SERIAL_UART_COUNT], SERIAL_LPUART_COUNT ), | ||
#if defined(USE_LPUART) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
#if defined(USE_LPUART) | |
#if defined(USE_LPUART1) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
USE_LPUART is intended. Port use is normalized, it will work for any number of ports of given type(VCP is partial exception).
#endif | ||
|
||
#if defined(USE_UART) || defined(USE_SOFTSERIAL) | ||
#if defined(USE_UART) || defined(USE_LPUART) || defined(USE_SOFTSERIAL) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
#if defined(USE_UART) || defined(USE_LPUART) || defined(USE_SOFTSERIAL) | |
#if defined(USE_UART) || defined(USE_LPUART1) || defined(USE_SOFTSERIAL) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only have one LPUART at the moment - but we have ID's 40-49 reserved for LPUART 1-10 so maybe we go with USE_LPUART
instead to be consistent.
Impressive amount of work. Have you tested DMA operation? This is code we don’t use and so isn’t tested at the moment, but in future and with more DMA channels this may be resurrected. |
@SteveCEvans : DMA should be unchanged (same enums, with possibly different values). I'll double-check it. |
serial - refactor serial resource handling
serial - merge code duplicated in all UART implementations
Code did not match exactly. Obvious cases are fixed, more complicated use #ifs
serial - refactor uartDevice
serialPortIdentifier_e identifier;
for uartHardwareserial - apply changes to inverter
New implementation is trivial
serial - HAL - rxIrq, txIrq replaces by irqn
There is only one IRQ for serial
serial - serial_post.h
Generated code to normalize target configuration.
jinja2 template is used to generate header file. Port handling is unified a lot.
SERIAL_<type><n>_USED 0/1
- always defined, value depends on target configurationSERIAL_<type>_MASK
- bitmask of used ports or given type. 1 is BIT(0)SERIAL_<type>_COUNT
- number of enabled ports of given typeSERIAL_<type>_MAX
- + 1, 0 when no port is enabledtargets - remove automatically calculated valued from configs
serial_post.h generated them
serial - remove definitions replaced by serial_post.h
serial - change LPUART to UART_LP1 in places
LPUART is mostly handled as another UART port, this change reflects it
serial - use ARRAYLEN / ARRAYEND in some places
replaces constant that may not match actual array size
serial - adapt softserial to changes
serial - whitespace, comments
softserial - fix serialTimerConfigureTimebase
serial - suspicious code
serial - unittests
serial - cleanup
serial - simpler port names
Is this useful ?