Conversation
Summary of ChangesHello @wmlynik, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request integrates the Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds support for the armv8m55-stm32n6-nucleo target. The changes include updates to test configurations to exclude this target from certain tests, adding timeout configurations, and implementing the target-specific logic in new Python files.
The core implementation for the new target is in trunner/target/armv8m55.py and trunner/tools/gdb.py. My review focuses on these files and I've found several issues:
- The implementation includes interactive prompts (
input()) which are not suitable for CI/automation. - There are potential
AttributeErrors due to uncheckedNonevalues when searching for tool paths. - There's a bug where the serial port might not be opened if flashing is skipped.
- Some code is duplicated or redundant.
- There are some misleading comments and docstrings that seem to be from a copy-paste.
I've provided specific comments with suggestions to address these points. Addressing these will improve the robustness and usability of the new target support, especially in automated environments.
| print("Set BOOT1 jumper to 1 and power cycle the board") | ||
| input("Press Enter to continue...") | ||
| extra_args = [ | ||
| "-hardRst", | ||
| "-w", | ||
| self.image_file, | ||
| f"{self.image_addr}", | ||
| ] | ||
|
|
||
| STM32ProgrammerCLIProcess( | ||
| port="SWD", | ||
| mode="HOTPLUG", | ||
| extra_args=extra_args, | ||
| host_log=host_log, | ||
| cwd=Path(self.boot_dir()), | ||
| ).run() | ||
|
|
||
| except FileNotFoundError as e: | ||
| raise FlashError(msg=str(e)) from e | ||
| except STM32ProgrammerCLIError as e: | ||
| raise FlashError(msg=str(e)) from e | ||
| print("Set BOOT1 jumper to 0 WITHOUT power cycling the board") | ||
| input("Press Enter to continue...") |
There was a problem hiding this comment.
The use of print and input makes the test runner interactive, which is not suitable for automated environments like CI. This requires manual intervention to proceed. These should be removed. If manual steps are required, they should be documented, but ideally, the process should be fully automated, perhaps using GPIOs if the host supports it.
wmlynik
force-pushed
the
wmlynik/add_stm32n6
branch
4 times, most recently
from
aea1f9b to
aed41f3
Compare
Unit Test Results9 525 tests 8 933 ✅ 52m 9s ⏱️ Results for commit f62f592. ♻️ This comment has been updated with latest results. |
JIRA: CI-598
wmlynik
force-pushed
the
wmlynik/add_stm32n6
branch
from
aed41f3 to
f62f592
Compare
1 participant
JIRA: CI-598
Description
Motivation and Context
Types of changes
How Has This Been Tested?
Checklist:
Special treatment