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

{bp-15288} stm32_i2c: fix sending large data over i2c #15363

Merged
merged 1 commit into from
Dec 28, 2024

Conversation

jerpelea
Copy link
Contributor

Summary

To trigger TC interrupt NBYTES needs to be set before RELOAD is disabled similar to previous commitdone on stm32h7 510b622

Impact

RELEASE

Testing

CI

To trigger TC interrupt NBYTES needs to be set before RELOAD is disabled
similar to previous commitdone on stm32h7 510b622
@github-actions github-actions bot added Arch: arm Issues related to ARM (32-bit) architecture Size: S The size of the change in this PR is small labels Dec 27, 2024
@nuttxpr
Copy link

nuttxpr commented Dec 27, 2024

[Experimental Bot, please feedback here]

No. The PR description lacks crucial information. While it references a related commit, it doesn't explain what the change is, how it works, or why it's necessary beyond a vague reference to triggering a TC interrupt. The Impact section is insufficient with just "RELEASE", offering no details. The Testing section merely states "CI", failing to provide any actual test results or specifics on the test environment. This makes it impossible to assess the validity and correctness of the change.

Here's what's missing and needs to be added:

  • Summary:

    • A clear explanation of the problem this PR solves. What was the specific issue with triggering the TC interrupt?
    • A concise description of the code changes made to address the issue. What exactly was modified in terms of code logic?
    • Mention the affected files.
    • Link to the relevant NuttX issue.
  • Impact: Expand on "RELEASE."

    • Is new feature added? Is existing feature changed? Answer these explicitly.
    • Address all the impact points (user, build, hardware, documentation, security, compatibility) with "YES/NO" and provide details if "YES". For example, if the change impacts specific hardware, list the affected architectures/boards. If there are backward compatibility concerns, explain them.
  • Testing: "CI" is inadequate.

    • Build Host(s): Specify the OS, CPU architecture, and compiler used for building.
    • Target(s): Specify the target architecture, board, and configuration used for testing.
    • Testing logs before change: Provide actual logs demonstrating the issue before applying the patch.
    • Testing logs after change: Provide actual logs demonstrating the fix after applying the patch. Show that the TC interrupt is now triggering correctly.

Without these details, the PR cannot be properly reviewed and merged.

@xiaoxiang781216 xiaoxiang781216 merged commit 5cc9288 into apache:releases/12.8 Dec 28, 2024
13 of 25 checks passed
@jerpelea jerpelea deleted the bp-15288 branch December 28, 2024 08:35
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 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