Skip to content

Conversation

@jasonbu
Copy link
Contributor

@jasonbu jasonbu commented Sep 27, 2024

Summary

1.add pin type to cfg gpio as wake up source
2.add IOEXPANDER_OPTION_WAKEUPCFG for set wake up option

Impact

Add more feature in ioexpand.

Testing

CI-test.

@jasonbu jasonbu marked this pull request as ready for review September 27, 2024 10:13
@github-actions github-actions bot added the Size: S The size of the change in this PR is small label Sep 27, 2024
@nuttxpr
Copy link

nuttxpr commented Sep 27, 2024

[Experimental Bot, please feedback here]

No, this PR does not fully meet the NuttX requirements.

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

Summary

  • Insufficient Detail: The summary provides a high-level overview but lacks specifics.
    • Why is this change necessary? What problem does it solve, or what new capability does it provide?
    • What functional part of the code is changed? Be more specific than "ioexpand". Mention the files or modules affected.
    • How does it work? Provide a technical explanation of the implementation. For example, how are pin types configured for wake-up sources? How is IOEXPANDER_OPTION_WAKEUPCFG used?
  • Missing Issue References: If this PR addresses a specific issue in the NuttX or NuttX Apps repositories, include the issue numbers.

Impact

  • Too Vague: The impact section needs to be more specific. Address all the bullet points with "NO" or "YES" and provide detailed explanations where applicable:
    • New Feature: You've indicated a new feature is added. Describe it clearly. What can users now do that they couldn't before?
    • User Impact: Will users need to change their code or configuration to use this feature? If so, how?
    • Build Impact: Will adding this feature require changes to the build system (Kconfig, Makefiles, etc.)?
    • Hardware Impact: Does this affect specific architectures, boards, or drivers? List them.
    • Documentation Impact: Does this PR require documentation updates? If so, have you included the updates in the PR?
    • Security Impact: Could this change introduce any security vulnerabilities? Consider potential risks carefully.
    • Compatibility Impact: Does this affect backward compatibility, forward compatibility, or interoperability with other systems/software?

Testing

  • Insufficient Information:
    • Local Setup: Specify the details of your local build host (OS, CPU, compiler) and target(s) (architecture, board, configuration) used for testing.
    • CI Tests Alone Are Not Enough: While CI tests are important, they might not cover all edge cases. Describe the specific tests you performed locally to validate the changes.
    • No Logs: You haven't provided any testing logs to demonstrate the functionality before and after the change. Include relevant log snippets to show the issue and the fix.

How to Improve:

  1. Expand on the Summary: Provide a clear and concise explanation of the change, its purpose, and its technical details.
  2. Address All Impact Points: Go through each impact bullet point and provide a specific "NO" or "YES" response along with a detailed explanation if necessary.
  3. Provide Comprehensive Testing Details: Describe your local test setup, the specific tests performed, and include relevant log snippets that demonstrate the change's effect.

Copy link
Contributor

@acassis acassis left a comment

Choose a reason for hiding this comment

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

Please update Documentation/

@github-actions github-actions bot added the Area: Documentation Improvements or additions to documentation label Sep 29, 2024
@jasonbu
Copy link
Contributor Author

jasonbu commented Sep 29, 2024

Please update Documentation/

Please review the append patch describe about io expander

dulibo1 and others added 2 commits September 29, 2024 17:09
1.add pin type to cfg gpio as wake up source
2.add IOEXPANDER_OPTION_WAKEUPCFG for set wake up option

Signed-off-by: dulibo1 <dulibo1@xiaomi.com>
Signed-off-by: buxiasen <buxiasen@xiaomi.com>
Signed-off-by: buxiasen <buxiasen@xiaomi.com>
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: 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