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

stm32h7/linum-stm32h753bi: fix fdcan configuration #15343

Merged
merged 1 commit into from
Dec 26, 2024

Conversation

JorgeGzm
Copy link
Contributor

Summary

Fix fdcan configuration

Impact

Use the socketcan with the linum board

Testing

Build socketcan project

$ ./tools/configure.sh linum-stm32h753bi:socketcan

Testing comunication

 # Configure the can0 and can1 to send messages
  nsh> ifup can0
  ifup can0...OK
  nsh> ifup can1
  ifup can1 ...OK
  nsh> cansend can0 123#DEADBEEF
  nsh> cansend can1 5A1#11.2233.44556677.88

  # Reset the board and configure the can0 peripheral to receive messages
  nsh> ifup can0
  ifup can0...OK
  nsh> candump can0
    can0  051   [8]  00 11 22 33 44 55 66 77
    can0  051  [16]  00 11 22 33 44 55 66 77 88 99 AA BB CC DD EE FF

@github-actions github-actions bot added Area: Documentation Improvements or additions to documentation Board: arm Size: S The size of the change in this PR is small labels Dec 26, 2024
@nuttxpr
Copy link

nuttxpr commented Dec 26, 2024

[Experimental Bot, please feedback here]

No, this PR description does not fully meet the NuttX requirements. Here's why and how to fix it:

Missing Information in Summary:

  • Why is the change necessary? What was the original problem with the FDCAN configuration? Was it not working? Were there errors? Be specific. Example: "Fixes an issue where FDCAN configuration on the linum-stm32h753bi board prevented proper communication using socketcan."
  • What functional part of the code is being changed? Which files/modules were modified? For example: "Modifies the FDCAN driver and board configuration files for the linum-stm32h753bi."
  • How does the change exactly work? This needs more detail. Don't just say "Use socketcan." Explain the technical details of the fix. For example: "Corrected the bit timing settings in the board configuration. Added handling for X feature in the FDCAN driver. Enabled Y interrupt."
  • Issue References: Are there any related NuttX or NuttX Apps issues or pull requests? If so, provide the links.

Missing Information in Impact:

  • Most of the impact sections are answered with "NO" implicitly. Explicitly state "NO" for clarity.
  • Impact on User: Even if the answer is no, explain briefly why. Example: "NO. Existing applications using socketcan should work correctly after this fix."
  • Impact on hardware: You mentioned the linum board. Be specific: "YES. Changes the FDCAN configuration on the linum-stm32h753bi board."
  • Impact on Documentation: If no documentation changes are needed, state "NO. This fix does not require documentation updates." If the fix should have documentation updates, then the PR should include them.

Issues with Testing:

  • Build Host: Provide details about your build host environment. Example: "Linux, x86_64, GCC 12.2.0"
  • Target: You mentioned linum, but be more specific about the configuration: "linum-stm32h753bi:socketcan"
  • Testing logs before change: You need to show the output before the fix was applied. This demonstrates the problem that the PR is solving. Show the errors or incorrect behavior.
  • Testing logs after change: The "after" logs demonstrate that the fix works. Your current logs look good, but make sure they show the complete and successful operation.

Example of an Improved Summary:

This PR fixes an issue where FDCAN communication was not working correctly on the linum-stm32h753bi board when using the socketcan driver. The problem was caused by incorrect bit timing settings in the board configuration and a missing interrupt handler in the FDCAN driver.  This PR modifies the `stm32_fdcan.c` driver and the `linum-stm32h753bi/configs/socketcan/defconfig` board configuration file.  The fix corrects the bit timing parameters to match the CAN bus requirements and adds the necessary interrupt handling to ensure reliable data transmission and reception.  Resolves #[NuttX Issue Number, if applicable].

By providing more complete information, you significantly improve the review process and increase the chances of your PR being merged quickly.

@xiaoxiang781216
Copy link
Contributor

@hujun260 please fix this error:

chip/efm32_leserial.c: In function 'efm32_restoreuartint':
Error: chip/efm32_leserial.c:329:37: error: 'len' undeclared (first use in this function); did you mean 'ien'?
  329 |   efm32_restoreuartint_nolock(priv, len);
      |                                     ^~~
      |                                     ien

@xiaoxiang781216 xiaoxiang781216 merged commit d4acd69 into apache:master Dec 26, 2024
20 of 26 checks passed
@JorgeGzm JorgeGzm deleted the fix_fdcan_linum branch December 26, 2024 17:32
@hujun260
Copy link
Contributor

chip/efm32_leserial.c: In function 'efm32_restoreuartint':
Error: chip/efm32_leserial.c:329:37: error: 'len' undeclared (first use in this function); did you mean 'ien'?
329 | efm32_restoreuartint_nolock(priv, len);
| ^~~
| ien

#15353

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 Board: arm 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