Add NDA submodule, generic vsr format, initialize can receiving tasks, etc

[shua1090]

Description

This code sets up a base set of code that sets up a CAN interface, initializes the Vehicle State Register, and
sends speed data to the motor controller at 200 Hz. It also receives any data back and saves it to the VSR.

More importantly: It hides the motor-controller specific code behind our private submodule, meaning
that all the code protected by the NDA is hidden. As it is a submodule, editing the code still works really well overall.

Currently the code is "headless", it only transmits a default 0 RPM speed and doesn't do any error checking.
The ability to change code (via a console/accelerator pedal) and error checking will be in follow-up PRs.
These issues are tracked in: #16 and #17

Type of change

Please put an x on what type of change this is:

• Configuration File Update
• Documentation or workflow update
• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• This change requires a documentation update

How Has This Been Tested?

This PR has not been tested yet; A follow up issue (#18) has been made tracking the required testing,
and additional PRs will be made if any errors in testing have been found.

Checklist:

Please mark off items in the checklist as applicable; Only PRs that
maintain acceptable style, have meaningful functionality updates, and
add thorough documentation will be approved.

This checklist is not necessary to be completed for documentation changes.

• My code follows the style guidelines of this project (check README)
• I have performed a self-review of my code
• I have commented my code, particularly in hard-to-understand areas
• I have made corresponding changes to the documentation
• My changes generate no new warnings
• Any dependent changes have been merged and published in downstream modules

[gemini-code-assist]

Hello @shua1090, I'm Gemini Code Assist¹! 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!

Summary of Changes

Hello team, Gemini here with a summary of this pull request. This PR lays the groundwork for the main MCU (Microcontroller Unit) functionality. The primary goal is to establish communication with the motor controller via CAN, introduce a central data structure called the Vehicle State Register (VSR) to hold vehicle state, and importantly, integrate the motor-controller specific code as a private submodule (motor_h300) to handle NDA requirements. The current implementation focuses on setting up the CAN interface, initializing the VSR, sending a default 0 RPM speed command at 200 Hz, and receiving data back into the VSR. It's noted that this is an initial setup and lacks features like console input for speed control or comprehensive error checking, which are planned for future PRs.

Highlights

• NDA Submodule Integration: Adds the src/mcu/motor_h300 submodule to encapsulate proprietary motor controller communication logic, keeping NDA-protected code separate.
• Vehicle State Register (VSR) Introduction: Introduces a generic vehicle_status_reg_s struct (vsr.h, vsr.c) to serve as a central repository for all vehicle state data. It uses an X-Macro (VSR_ITEMS) and mutexes to ensure thread-safe access to different parts of the register.
• CAN Interface and Task Initialization: Sets up the TWAI (CAN) driver for the motor control bus and initializes FreeRTOS tasks (src/mcu/main.c, src/mcu/tasks.h) for reading incoming CAN messages, sending motor commands (currently a fixed 0 RPM at 200 Hz), and processing received motor data using a dedicated queue.
• Build System Updates: Modifies platformio.ini and src/CMakeLists.txt to define a new mcu build environment and include the necessary source files for the core MCU application, including those from the new submodule.

Changelog

Click here to see the changelog

• .gitignore
  • Uncomments sdkconfig.* to include the generated configuration file in version control.
• .gitmodules
  • Adds a new submodule entry for src/mcu/motor_h300 pointing to the private motor_h300.git repository.
• platformio.ini
  • Renames the build environment section from [env:motorControllerCANUART] to [env:mcu] (line 39).
  • Updates the board_build.cmake_extra_args from -DCMAKE_MOTOR_CONTROLLER=ON to -DCMAKE_MCU_ESP_BUILD=ON (line 41).
• sdkconfig.mcu
  • Adds a comprehensive ESP-IDF project configuration file, likely generated via idf.py menuconfig, enabling various peripherals, FreeRTOS settings, and other build options for the ESP32 target.
• src/CMakeLists.txt
  • Updates the conditional block for the MCU build (CMAKE_MCU_ESP_BUILD) to include new source files: mcu/main.c, mcu/vsr.c, mcu/motor_h300/h300.c, and mcu/motor_h300/motor_send.c (lines 51-54).
  • Changes the conditional check from CMAKE_MOTOR_CONTROLLER to CMAKE_MCU_ESP_BUILD (line 48).
• src/mcu/README.md
  • Adds a new README file providing an overview of the MCU's role, instructions for cloning the submodule, notes on running the code, and details on the VSR architecture and initial task breakdown on Core 0.
• src/mcu/motor_h300
  • Adds the motor_h300 submodule at commit ea287075243621e180199c6077aa2284808c5477.
• src/mcu/tasks.h
  • Adds a new header file declaring the FreeRTOS tasks (read_can_data, handle_h300, send_motor_data) with their priorities and stack size.
  • Declares external TWAI handles and the queue for receiving H300 motor data.
• src/mcu/vsr.c
  • Adds a new source file implementing the vsr_init function.
  • Initializes the global vehicle_status_register struct and creates static mutexes for each VSR item defined by the VSR_ITEMS macro.
• src/mcu/vsr.h
  • Adds a new header file defining the vehicle_status_reg_s struct using an X-Macro (VSR_ITEMS) to include motor-specific status structs and associated mutex handles.
  • Defines the ACQ_REL_VSRSEM macro for safely acquiring and releasing VSR item mutexes.
  • Declares the global vehicle_status_register.
• src/motorConfig/main.c
  • Deletes the old main.c file, which is replaced by src/mcu/main.c.

Using Gemini Code Assist

The 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 in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in issue comments and review comments.
Help	/gemini help	Displays a list of available commands.

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 .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist is currently in preview and 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 to provide feedback.

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.

---

CAN bus sends the word,
Tasks awake, data stirred.
VSR holds the state,
Mutex guards the gate.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[gemini-code-assist]

Code Review

This pull request introduces a foundational software layer for the MCU, including CAN interface setup, Vehicle State Register (VSR) initialization, and basic motor communication tasks. The approach of using a submodule for NDA-protected code and the VSR architecture with mutexes is well-conceived.

However, the most significant concern is that the PR description states, "This PR has not been tested yet". For firmware, especially involving motor control, it's critical to perform at least basic testing on target hardware before merging to ensure stability and correctness. I strongly recommend addressing this before merging.

No explicit style guide was provided, so feedback is based on general C and embedded best practices focusing on clarity, correctness, and maintainability.

Summary of Findings

• Testing: The PR description explicitly states that the code has not been tested. This is a critical oversight for firmware, especially one interacting with hardware like a motor controller. At least basic functional testing on the target hardware should be performed.
• Scheduler Initialization: The app_main function calls vTaskStartScheduler(). In ESP-IDF, the scheduler is typically already running when app_main is invoked. This call is likely unnecessary and potentially problematic.
• Configuration Discrepancy: There's a mismatch in the CPU frequency mentioned in the README (240 MHz) and the sdkconfig.mcu file (160 MHz).
• Hardcoded Values: TWAI GPIO pins are hardcoded in main.c. Consider making them configurable.
• Incomplete Error Handling: Several TODO comments in main.c point to missing error handling for queue operations. These should be addressed for robustness.
• Unused Code/Variables: The general_control_bus variable is declared but not used in main.c. The arg parameter in read_can_data is also unused. These were not commented on directly due to review settings (low severity).
• Misleading Comment: In main.c (line 122), the comment // core 1 is associated with a task pinned to core 0. This is confusing and should be corrected. This was not commented on directly due to review settings (low severity).
• Wi-Fi Configuration: sdkconfig.mcu has CONFIG_ESP_WIFI_ENABLED=y. If Wi-Fi is not required for this MCU application, disabling it could save resources. This was not commented on directly due to review settings (low severity).
• Stack Size: DEFAULT_STACK_SIZE in tasks.h is set to 10000 bytes. While this provides a safety margin, it might be overly generous for some tasks and could be optimized after profiling. This was not commented on directly due to review settings (low severity).

Merge Readiness

This pull request makes significant progress in setting up the MCU firmware. However, due to the critical issue of the code being untested and the high-severity concern regarding the vTaskStartScheduler() call, I cannot recommend merging it in its current state.

Please address the testing concern and the identified high and medium severity issues. Once these are resolved, the PR will be in much better shape. As a reviewer, I am not authorized to approve pull requests; please ensure further review and approval from designated maintainers after addressing the feedback.