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

boards/nucleo-f302r8: add rs485 support #16035

Merged
merged 1 commit into from
Mar 24, 2025

Conversation

raiden00pl
Copy link
Member

Summary

add rs485 support for nucleo-f302r8 based on RS485 CAN Waveshare shield.

Impact

rs485 can be used with nucleo-f302r8.

Testing

custom modbus slave example

add rs485 support for nucleo-f302r8 based on RS485 CAN Waveshare shield.

Signed-off-by: raiden00pl <[email protected]>
@github-actions github-actions bot added Board: arm Size: S The size of the change in this PR is small labels Mar 22, 2025
@nuttxpr
Copy link

nuttxpr commented Mar 22, 2025

[Experimental Bot, please feedback here]

No, this PR does not fully meet the NuttX requirements as described. While it provides a summary of the change, it lacks crucial details. Here's a breakdown of what's missing:

  • Summary: While the "what" is present, the "why" and "how" are missing. Is this a new feature request? Was there a bug prompting this addition? How specifically is RS485 support implemented (driver changes, configuration options, etc.)? Linking to related issues in the NuttX or NuttX-Apps repositories is also essential if applicable.

  • Impact: The current impact description is extremely vague. Each "Impact on..." section requires a specific "YES" or "NO" answer. If "YES," a detailed explanation is mandatory. For example, "Impact on hardware: YES. This change modifies the STM32F302R8 board configuration to support RS485 on UART2 and requires the Waveshare RS485 CAN shield."

  • Testing: The testing section is insufficient. It needs details about the build host and target environments. It also lacks "before" and "after" logs demonstrating the change's effectiveness. Simply stating "custom modbus slave example" doesn't provide any verifiable evidence that the change works as intended. Provide concrete examples of commands used and the output observed, both before and after the change.

Example of a better Testing section:

I confirm that changes are verified on local setup and works as intended:
* Build Host(s): Linux (Ubuntu 20.04), x86_64, GCC 9.4.0
* Target(s): ARM (STM32F302R8), nucleo-f302r8:default

Testing logs before change:

nsh> modbus_slave /dev/ttyS2
ERROR: RS485 not supported on this board.


Testing logs after change:

nsh> modbus_slave /dev/ttyS2
Starting Modbus Slave on /dev/ttyS2...
Received Modbus request: ... (show actual data)
Responding with: ... (show actual data)


In short, the PR needs significantly more detail to meet the NuttX requirements. Be explicit and thorough in all sections to ensure reviewers can properly assess and integrate the changes.

@acassis
Copy link
Contributor

acassis commented Mar 23, 2025

@raiden00pl please include a board profile and the Documentation/ entry to it.

@xiaoxiang781216 xiaoxiang781216 merged commit d9ee02b into apache:master Mar 24, 2025
25 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.

5 participants