Skip to content

Conversation

@jerpelea
Copy link
Contributor

Summary

he features CONFIG_IMXRT_LPUART_SINGLEWIRE and CONFIG_IMXRT_LPUART_INVERT will get build errors when CONFIG_SERIAL_TERMIOS or CONFIG_SERIAL_TIOCSERGSTRUCT are not enabled.

This happens because INVERT and SINGLEWIRE rely on variables that are only declared when TERMIOS or TIOCSERGSTRUCT are enabled.

Fixed by making those vars available in all cases.

Impact

RELEASE

Testing

CI

@github-actions github-actions bot added Arch: arm Issues related to ARM (32-bit) architecture Arch: arm64 Issues related to ARM64 (64-bit) architecture Size: S The size of the change in this PR is small labels Dec 16, 2024
@jerpelea jerpelea changed the title lpuart: fix build errors for SINGLEWIRE and INVERT without SERIAL_TER… {bp-15183} lpuart: fix build errors for SINGLEWIRE and INVERT without SERIAL_TER… Dec 16, 2024
@nuttxpr
Copy link

nuttxpr commented Dec 16, 2024

[Experimental Bot, please feedback here]

Fill In The Commit Message: This PR contains a Commit with an Empty Commit Message. Please fill in the Commit Message with the PR Summary.

Yes, this PR appears to meet the NuttX requirements, although the provided information is minimal.

Here's a breakdown of why and where it could be improved:

Strengths:

  • Clear Summary: The summary explains the problem, the solution, and the affected features.
  • Impact Marked: The RELEASE tag in Impact signifies a potential breaking change, drawing attention to it.
  • Testing Mentioned: Mentioning CI implies testing was performed, though more detail would be beneficial.

Weaknesses & Suggestions for Improvement:

  • Missing Issue References: If this PR addresses a specific issue, linking the relevant NuttX or NuttX-apps issue would be helpful.
  • Vague Impact: While RELEASE is a start, more explanation is needed. Does this fix a bug in a released version? Will this change existing behavior for users? Specify which architectures or boards are affected.
  • Insufficient Testing Detail: "CI" is not enough. List the specific build hosts and target platforms where testing was conducted. While full logs might not be necessary, some evidence of testing should be included. What functionality was tested? What were the specific results? Even a brief summary like "LPUART single-wire mode now functions correctly on the STM32F4Discovery board" adds much-needed context.
  • Missing Documentation Impact: If the fix requires documentation updates, mention whether they are included in the PR or if a separate documentation update will be needed.

In short, the core information is there, but the PR description needs more detail to be truly comprehensive. Providing specific examples of the build errors, the modified code, and the test results would significantly improve its clarity and facilitate review.

@xiaoxiang781216 xiaoxiang781216 merged commit 84ab0a3 into apache:releases/12.8 Dec 16, 2024
27 checks passed
@jerpelea jerpelea deleted the bp-15183 branch December 17, 2024 02:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Arch: arm Issues related to ARM (32-bit) architecture Arch: arm64 Issues related to ARM64 (64-bit) architecture Size: S The size of the change in this PR is small

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants