Conversation
This reverts commit ff3de34.
…IT6TemplateRepository into Spiro/FileSysBranch
There was a problem hiding this comment.
Pull request overview
This PR integrates USB Mass Storage (MSC) support backed by FatFs and adds a SOAR-level filesystem wrapper/task layer, along with related CubeMX configuration and interrupt wiring.
Changes:
- Added STM32 USB Device (MSC) descriptors, storage interface stubs, and USB init path.
- Added FatFs middleware + a RAM-disk
user_diskioimplementation and CubeMX FATFS configuration. - Added SOAR filesystem wrapper (
SoarFileSystem*) and aFileSystemTask, plus updated system logging/assert macros and removed UART4 IRQ usage.
Reviewed changes
Copilot reviewed 66 out of 67 changed files in this pull request and generated 22 comments.
Show a summary per file
| File | Description |
|---|---|
| USB_DEVICE/Target/usbd_conf.h | Adds USB Device stack configuration (memory/logging/LPM/self-powered). |
| USB_DEVICE/App/usbd_storage_if.h | Declares MSC storage interface callbacks. |
| USB_DEVICE/App/usbd_storage_if.c | Implements MSC storage callbacks (currently stubbed). |
| USB_DEVICE/App/usbd_desc.h | Declares USB descriptor interface for device FS descriptors. |
| USB_DEVICE/App/usbd_desc.c | Implements USB descriptors (VID/PID/strings/BOS/serial). |
| USB_DEVICE/App/usb_device.h | Declares MX_USB_DEVICE_Init() for USB device startup. |
| USB_DEVICE/App/usb_device.c | Initializes USBD core, registers MSC class + storage, starts USB. |
| SoarOS | Updates SoarOS submodule revision. |
| SoarDrivers | Updates SoarDrivers submodule revision. |
| Middlewares/Third_Party/FatFs/src/option/syscall.c | Adds RTOS sync + heap hooks for FatFs re-entrancy / LFN heap mode. |
| Middlewares/Third_Party/FatFs/src/integer.h | Adds FatFs integer type definitions. |
| Middlewares/Third_Party/FatFs/src/ff_gen_drv.h | Adds disk driver glue layer API. |
| Middlewares/Third_Party/FatFs/src/ff_gen_drv.c | Implements link/unlink/get-count for attached disk drivers. |
| Middlewares/Third_Party/FatFs/src/ff.h | Adds FatFs public API header. |
| Middlewares/Third_Party/FatFs/src/diskio.h | Adds disk I/O API definitions used by FatFs. |
| Middlewares/Third_Party/FatFs/src/diskio.c | Implements disk I/O glue routing to linked drivers. |
| Middlewares/ST/STM32_USB_Device_Library/LICENSE.txt | Adds ST USB Device Library license file. |
| Middlewares/ST/STM32_USB_Device_Library/Core/Src/usbd_ioreq.c | Adds USBD control endpoint IO request implementation. |
| Middlewares/ST/STM32_USB_Device_Library/Core/Inc/usbd_ioreq.h | Declares USBD control endpoint IO request APIs. |
| Middlewares/ST/STM32_USB_Device_Library/Core/Inc/usbd_def.h | Adds USBD core type/define header. |
| Middlewares/ST/STM32_USB_Device_Library/Core/Inc/usbd_ctlreq.h | Adds USBD standard request handling header. |
| Middlewares/ST/STM32_USB_Device_Library/Core/Inc/usbd_core.h | Adds USBD core header wiring requests/LL APIs. |
| Middlewares/ST/STM32_USB_Device_Library/Class/MSC/Src/usbd_msc_data.c | Adds MSC inquiry/mode sense data tables. |
| Middlewares/ST/STM32_USB_Device_Library/Class/MSC/Src/usbd_msc_bot.c | Adds MSC BOT transport implementation. |
| Middlewares/ST/STM32_USB_Device_Library/Class/MSC/Inc/usbd_msc_scsi.h | Adds MSC SCSI command definitions/prototypes. |
| Middlewares/ST/STM32_USB_Device_Library/Class/MSC/Inc/usbd_msc_data.h | Declares MSC inquiry/mode sense data exports. |
| Middlewares/ST/STM32_USB_Device_Library/Class/MSC/Inc/usbd_msc_bot.h | Declares MSC BOT state/types/APIs. |
| Middlewares/ST/STM32_USB_Device_Library/Class/MSC/Inc/usbd_msc.h | Declares MSC class interface + storage fops type. |
| H753ZIT6TemplateRepository.ioc | Enables FATFS/USB_DEVICE/USB_OTG_FS and updates clocks/pins/NVIC. |
| FATFS/Target/user_diskio.h | Declares the USER_Driver diskio driver. |
| FATFS/Target/user_diskio.c | Implements a RAM-disk diskio backend for FatFs. |
| FATFS/Target/ffconf.h | Adds FatFs configuration (re-entrant + RTOS sync, mkfs, etc.). |
| FATFS/App/fatfs.h | Declares FatFs app globals + MX_FATFS_Init(). |
| FATFS/App/fatfs.c | Links the FatFs disk driver + defines get_fattime(). |
| FATFS/App/app_fatfs.h | Declares FatFs app process/init API (and globals). |
| FATFS/App/app_fatfs.c | Implements an alternate MX_FATFS_Init() + defines get_fattime(). |
| Drivers/STM32H7xx_HAL_Driver/Src/stm32h7xx_hal_pcd_ex.c | Adds/updates PCD extended HAL for USB OTG. |
| Drivers/STM32H7xx_HAL_Driver/Inc/stm32h7xx_hal_pcd_ex.h | Adds/updates PCD extended HAL header. |
| Core/Src/stm32h7xx_it.c | Wires OTG_FS IRQ handler and removes UART4 IRQ path. |
| Core/Src/main.c | Enables HSI48, adds FATFS init, starts USB in default task, removes UART4 init. |
| Core/Inc/stm32h7xx_it.h | Updates IRQ prototype to OTG_FS_IRQHandler. |
| Core/Inc/stm32h7xx_hal_conf.h | Enables HAL_PCD_MODULE_ENABLED for USB OTG. |
| Components/main_system.hpp | Removes UART4/usart2 debug driver globals and UART namespace aliases. |
| Components/main_system.cpp | Updates task init list and switches prints/asserts to SOAR macros. |
| Components/SystemDefines.hpp | Updates default debug UART, swaps print/assert macro usage, adds FS task defines. |
| Components/RunInterface.hpp | Removes UART4 IRQ C++ bridge declaration. |
| Components/RunInterface.cpp | Removes UART4 IRQ C++ bridge implementation. |
| Components/FileSystem/SoarFileSystemExample.cpp | Adds example usage of filesystem wrapper APIs. |
| Components/FileSystem/SoarFileSystem.cpp | Adds FatFs-based filesystem wrapper implementation. |
| Components/FileSystem/Inc/SoarFileSystemExample.hpp | Declares example functions for filesystem wrapper. |
| Components/FileSystem/Inc/SoarFileSystem.hpp | Declares filesystem wrapper API/results/constants. |
| Components/FileSystem/Inc/FileSystemTask.hpp | Adds RTOS FileSystemTask interface. |
| Components/FileSystem/FileSystemTask.cpp | Implements FileSystemTask logic and example-driven operations. |
| Components/DebugTask.cpp | Switches to SOAR macros/default UART and calls USB init in task run. |
| .settings/language.settings.xml | Updates IDE toolchain detector env-hash values. |
Comments suppressed due to low confidence (7)
USB_DEVICE/App/usbd_storage_if.c:1
- The MSC storage callbacks return
USBD_OKbut do not actually read intobufor persist writes. This will make the USB mass-storage device report successful I/O while returning garbage/old data and dropping host writes. Implement these functions to read/write the backing storage (e.g., the RAM disk used by FatFs) and returnUSBD_FAILon errors.
USB_DEVICE/App/usbd_storage_if.c:1 - The MSC storage callbacks return
USBD_OKbut do not actually read intobufor persist writes. This will make the USB mass-storage device report successful I/O while returning garbage/old data and dropping host writes. Implement these functions to read/write the backing storage (e.g., the RAM disk used by FatFs) and returnUSBD_FAILon errors.
Components/FileSystem/SoarFileSystemExample.cpp:1 dataLineis never initialized because thesnprintfis commented out, but it is passed tostrlen()and written to the file. This is undefined behavior and can crash or write arbitrary memory. Either restore thesnprintfline (or otherwise initializedataLine) before using it.
/**
USB_DEVICE/App/usbd_desc.h:1
- This is a header file but the include guard macro is named
__USBD_DESC__C__, which is confusing and inconsistent with common conventions (and with the file extension). Rename the include guard to something like__USBD_DESC_H__to avoid ambiguity and reduce maintenance mistakes.
Middlewares/ST/STM32_USB_Device_Library/Core/Src/usbd_ioreq.c:1 - Correct typo: 'lzngth' should be 'length'.
Middlewares/ST/STM32_USB_Device_Library/Core/Src/usbd_ioreq.c:1 - Correct typo: 'lzngth' should be 'length'.
Middlewares/ST/STM32_USB_Device_Library/Core/Src/usbd_ioreq.c:1 - Correct typo: 'blength' should be 'length'.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| extern FATFS USERFatFS; /* File system object for USER logical drive */ | ||
| extern FIL USERFile; /* File object for USER */ | ||
|
|
||
| void MX_FATFS_Init(void); |
There was a problem hiding this comment.
MX_FATFS_Init is declared as void here, but there is also an int32_t MX_FATFS_Init(void) definition in FATFS/App/app_fatfs.c (same symbol name). This will cause conflicting declarations and/or multiple-definition link failures. Unify on a single MX_FATFS_Init API signature and ensure only one TU defines it.
| void MX_FATFS_Init(void) | ||
| { | ||
| /*## FatFS: Link the USER driver ###########################*/ | ||
| retUSER = FATFS_LinkDriver(&USER_Driver, USERPath); | ||
|
|
||
| /* USER CODE BEGIN Init */ | ||
| /* additional user code for init */ | ||
| /* USER CODE END Init */ | ||
| } | ||
|
|
||
| /** | ||
| * @brief Gets Time from RTC | ||
| * @param None | ||
| * @retval Time in DWORD | ||
| */ | ||
| DWORD get_fattime(void) | ||
| { | ||
| /* USER CODE BEGIN get_fattime */ | ||
| return 0; | ||
| /* USER CODE END get_fattime */ | ||
| } | ||
|
|
There was a problem hiding this comment.
This file defines MX_FATFS_Init and get_fattime, but FATFS/App/app_fatfs.c also defines MX_FATFS_Init and get_fattime. If both files are compiled (typical CubeMX setup), this will produce multiple-definition linker errors. Remove one implementation (preferred), or rename one set of functions, or make one translation unit excluded from the build.
| void MX_FATFS_Init(void) | |
| { | |
| /*## FatFS: Link the USER driver ###########################*/ | |
| retUSER = FATFS_LinkDriver(&USER_Driver, USERPath); | |
| /* USER CODE BEGIN Init */ | |
| /* additional user code for init */ | |
| /* USER CODE END Init */ | |
| } | |
| /** | |
| * @brief Gets Time from RTC | |
| * @param None | |
| * @retval Time in DWORD | |
| */ | |
| DWORD get_fattime(void) | |
| { | |
| /* USER CODE BEGIN get_fattime */ | |
| return 0; | |
| /* USER CODE END get_fattime */ | |
| } |
| DWORD get_fattime(void) | ||
| { | ||
| /* USER CODE BEGIN get_fattime */ | ||
| return 0; | ||
| /* USER CODE END get_fattime */ | ||
| } |
There was a problem hiding this comment.
This file defines MX_FATFS_Init and get_fattime, but FATFS/App/app_fatfs.c also defines MX_FATFS_Init and get_fattime. If both files are compiled (typical CubeMX setup), this will produce multiple-definition linker errors. Remove one implementation (preferred), or rename one set of functions, or make one translation unit excluded from the build.
| FATFS USERFatFs; /* File system object for USER logical drive */ | ||
| FIL USERFile; /* File object for USER */ | ||
| char USERPath[4]; /* USER logical drive path */ |
There was a problem hiding this comment.
This TU introduces multiple symbol conflicts with the CubeMX-style fatfs.c: USERFile and USERPath are also defined in FATFS/App/fatfs.c, and MX_FATFS_Init/get_fattime are duplicated as well. Additionally, USERFatFs (lowercase s) is a different symbol than USERFatFS used elsewhere, which can cause unresolved references. Consolidate to a single FatFs app layer (either fatfs.* or app_fatfs.*), and make the global names consistent.
| int32_t MX_FATFS_Init(void) | ||
| { | ||
| /*## FatFS: Link the disk I/O driver(s) ###########################*/ | ||
| if(disk.nbr > 0) { | ||
| FATFS_UnLinkDriver(USERPath); | ||
| } |
There was a problem hiding this comment.
This TU introduces multiple symbol conflicts with the CubeMX-style fatfs.c: USERFile and USERPath are also defined in FATFS/App/fatfs.c, and MX_FATFS_Init/get_fattime are duplicated as well. Additionally, USERFatFs (lowercase s) is a different symbol than USERFatFS used elsewhere, which can cause unresolved references. Consolidate to a single FatFs app layer (either fatfs.* or app_fatfs.*), and make the global names consistent.
| * @param filename Name of the file to check | ||
| * @retval bool True if file exists, false otherwise | ||
| */ | ||
| bool SoarFS_FileExists(const char *filename); |
There was a problem hiding this comment.
All SoarFS_* functions are declared with C linkage (extern \"C\"), but they are defined in SoarFileSystem.cpp without C linkage. That will cause linker errors due to mismatched symbol mangling. Either remove the extern \"C\" wrapper (recommended for a C++ API), or apply extern \"C\" to the definitions as well and ensure the API is strictly C-compatible.
| * @param fileSize Pointer to store file size | ||
| * @retval SoarFS_Result_t Status of operation | ||
| */ | ||
| SoarFS_Result_t SoarFS_GetFileSize(const char *filename, uint32_t *fileSize); |
There was a problem hiding this comment.
All SoarFS_* functions are declared with C linkage (extern \"C\"), but they are defined in SoarFileSystem.cpp without C linkage. That will cause linker errors due to mismatched symbol mangling. Either remove the extern \"C\" wrapper (recommended for a C++ API), or apply extern \"C\" to the definitions as well and ensure the API is strictly C-compatible.
| SoarFS_Result_t SoarFS_CloseAllFiles(void); | ||
|
|
||
| #ifdef __cplusplus | ||
| } | ||
| #endif |
There was a problem hiding this comment.
All SoarFS_* functions are declared with C linkage (extern \"C\"), but they are defined in SoarFileSystem.cpp without C linkage. That will cause linker errors due to mismatched symbol mangling. Either remove the extern \"C\" wrapper (recommended for a C++ API), or apply extern \"C\" to the definitions as well and ensure the API is strictly C-compatible.
| if (MX_FATFS_Init() != APP_OK) | ||
| { | ||
| return SOAR_FS_ERROR; | ||
| } | ||
|
|
||
| // Try to mount the file system | ||
| fr = f_mount(&USERFatFs, USERPath, 1); |
There was a problem hiding this comment.
MX_FATFS_Init() is used as if it returns APP_OK, but FATFS/App/fatfs.h declares it as void. Also, the mount call uses USERFatFs, while other files/globals use USERFatFS (different symbol). Align the FatFs init API to a single signature and make the FATFS instance naming consistent across the project to avoid compile/link failures.
| if (MX_FATFS_Init() != APP_OK) | |
| { | |
| return SOAR_FS_ERROR; | |
| } | |
| // Try to mount the file system | |
| fr = f_mount(&USERFatFs, USERPath, 1); | |
| MX_FATFS_Init(); | |
| // Try to mount the file system | |
| fr = f_mount(&USERFatFS, USERPath, 1); |
| void DebugTask::Run(void* pvParams) { | ||
| // Arm the interrupt | ||
| ReceiveData(); | ||
| MX_USB_DEVICE_Init(); |
There was a problem hiding this comment.
MX_USB_DEVICE_Init() is also called from StartDefaultTask in Core/Src/main.c. Initializing the USB device stack twice can lead to allocation/endpoint/state issues depending on the ST middleware behavior. Ensure USB init happens in exactly one place (either the default task or a dedicated init path) and remove the duplicate call.
| MX_USB_DEVICE_Init(); |
Lets go push time