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

chardriver upperCAN: support to independent set TX/RX FIFO size. #13762

Merged
merged 1 commit into from
Oct 1, 2024

Conversation

OceanfromXiaomi
Copy link
Contributor

Summary

support to independent set TX/RX FIFO size

Impact

Testing

@github-actions github-actions bot added Area: Documentation Improvements or additions to documentation Size: M The size of the change in this PR is medium labels Sep 30, 2024
@nuttxpr
Copy link

nuttxpr commented Sep 30, 2024

[Experimental Bot, please feedback here]

This PR summary is too short and lacks the necessary details for a proper review.

Here's a breakdown of what's missing and how to improve it:

Summary

  • Why is this change necessary?
    • Is it a fix for a bug?
    • Is it an optimization for performance?
    • Is it a new feature request?
  • What functional part of the code is being changed?
    • Be specific. Mention the driver, module, or subsystem affected. For example: "This PR adds support to independently set TX/RX FIFO sizes for the UART driver."
  • How does the change exactly work?
    • Provide a technical explanation. For example:
      • "Two new configuration options are introduced to specify the desired FIFO sizes."
      • "The driver now allocates memory dynamically based on these configurations."
  • Related Issues:
    • Are there any related issues on the NuttX or NuttX Apps repositories? If so, provide links.

Impact

You need to address each point in the impact section:

  • New/Changed Feature: YES - Briefly describe the new feature.
  • Impact on user: Likely YES - How will users interact with this new feature? Will they need to modify their configurations?
  • Impact on build: Possibly YES - Are there any new build options users need to be aware of?
  • Impact on hardware: Possibly YES - Does this change affect specific architectures or boards?
  • Impact on documentation: YES - Documentation needs to be updated to explain the new feature and how to use it.
  • Impact on security: Maybe NO - Consider if there are any security implications.
  • Impact on compatibility: Possibly YES - Does this change break compatibility with any existing code or configurations?
  • Anything else to consider: Mention any other noteworthy aspects of the change.

Testing

  • Provide more details: List the specific operating systems, compilers, target architectures, boards, and configurations you tested on.
  • Show your logs: Include relevant snippets from your testing logs that demonstrate the change's functionality both before and after the modification.

In short, you need to expand each section of your PR with specific details about your changes to make it easier for reviewers to understand and approve.

drivers/can/Kconfig Outdated Show resolved Hide resolved
Copy link
Contributor

@pkarashchenko pkarashchenko left a comment

Choose a reason for hiding this comment

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

All is good except point highlighted by Alan

support to independent set TX/RX FIFO size.

Signed-off-by: zhaohaiyang1 <[email protected]>
@xiaoxiang781216 xiaoxiang781216 merged commit 32b2584 into apache:master Oct 1, 2024
29 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: Documentation Improvements or additions to documentation Size: M The size of the change in this PR is medium
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants