Develop/nn#1
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds STM32F407VGT6 MCU support to the project by integrating STM32 HAL drivers, CMSIS libraries, and core peripheral initialization code. The changes also introduce a new L0_Assets layer for resource management and update the build system to support multiple MCU targets.
Changes:
- Added complete STM32F407VGT6 HAL driver implementation with CMSIS support
- Introduced L0_Assets layer for static resource management
- Updated CMake build system to support MCU selection and cross-layer linking
Reviewed changes
Copilot reviewed 49 out of 119 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| L1_MCU/STM32F407VGT6_GCC/Drivers/ | STM32 HAL drivers and CMSIS headers for F4 family |
| L1_MCU/STM32F407VGT6_GCC/Core/ | MCU initialization, GPIO, UART, and interrupt handling |
| L0_Assets/ | New resource layer with documentation and build configuration |
| CMakeLists.txt | Updated to integrate L0_Assets and improve linking strategy |
| L1_MCU/CMakeLists.txt | Simplified to remove stub files and add L0_Assets dependency |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| * | ||
| * This software is licensed under terms that can be found in the LICENSE file | ||
| * in the root directory of this software component. | ||
| * If no LICENSE file comes with this software, it is provided AS-IS.Clause |
There was a problem hiding this comment.
The word 'Clause' appears to be incorrectly appended to 'AS-IS.' and should be removed.
| * If no LICENSE file comes with this software, it is provided AS-IS.Clause | |
| * If no LICENSE file comes with this software, it is provided AS-IS. |
| #ifndef STM32f4xx_HAL_EXTI_H | ||
| #define STM32f4xx_HAL_EXTI_H |
There was a problem hiding this comment.
The include guard macro uses lowercase 'f' in 'STM32f4xx' which is inconsistent with the uppercase convention used in other headers (e.g., 'STM32F4xx'). This should be 'STM32F4xx_HAL_EXTI_H' for consistency.
| #ifndef STM32f4xx_HAL_EXTI_H | |
| #define STM32f4xx_HAL_EXTI_H | |
| #ifndef STM32F4xx_HAL_EXTI_H | |
| #define STM32F4xx_HAL_EXTI_H |
| * @{ | ||
| */ | ||
| #define FLASH_FLAG_EOP FLASH_SR_EOP /*!< FLASH End of Operation flag */ | ||
| #define FLASH_FLAG_OPERR FLASH_SR_SOP /*!< FLASH operation Error flag */ |
There was a problem hiding this comment.
The macro FLASH_FLAG_OPERR is defined as FLASH_SR_SOP, but according to STM32F4 reference manual, the operation error flag should be FLASH_SR_OPERR. Using FLASH_SR_SOP (which typically refers to Start of Programming) would result in incorrect error flag checking.
| #define FLASH_FLAG_OPERR FLASH_SR_SOP /*!< FLASH operation Error flag */ | |
| #define FLASH_FLAG_OPERR FLASH_SR_OPERR /*!< FLASH operation Error flag */ |
| } | ||
| #endif | ||
|
|
||
| #endif /* ___STM32F4xx_HAL_DEF */ |
There was a problem hiding this comment.
The include guard closing comment has three underscores '___STM32F4xx_HAL_DEF' but the opening guard at line 21 uses two underscores '__STM32F4xx_HAL_DEF'. These should match.
| #endif /* ___STM32F4xx_HAL_DEF */ | |
| #endif /* __STM32F4xx_HAL_DEF */ |
| #define PHY_SR ((uint16_t)) /*!< PHY status register Offset */ | ||
|
|
||
| #define PHY_SPEED_STATUS ((uint16_t)) /*!< PHY Speed mask */ | ||
| #define PHY_DUPLEX_STATUS ((uint16_t)) /*!< PHY Duplex mask */ |
There was a problem hiding this comment.
These PHY register and mask definitions have empty casts '((uint16_t))' without any actual value. They should either be assigned meaningful hexadecimal values or removed if unused.
| # 链接所有层级生成的库 | ||
| # 注意顺序：通常 顶层 依赖 底层，但链接器有时需要注意符号解析顺序。 | ||
| # 如果使用 .a 静态库，建议将依赖方放在前面 (L5 依赖 L4...) | ||
| # 强制链接器解析 ek_main 符号（跨层调用 L1->L5） |
There was a problem hiding this comment.
Add a comment explaining why the --undefined flag is necessary for ek_main, as this is a non-standard linking requirement that forces symbol resolution across layers.
| # 强制链接器解析 ek_main 符号（跨层调用 L1->L5） | |
| # NOTE: Non-standard linking requirement. | |
| # ek_main is implemented in a higher layer (e.g. L5) but is entered from lower layers (L1->L5), | |
| # so the top-level executable does not directly reference ek_main. Without this flag the linker | |
| # may discard the object file containing ek_main during garbage collection. We therefore force | |
| # the linker to treat ek_main as an undefined symbol at link time so that it is resolved across | |
| # layers and the correct entry point is retained. |
No description provided.