Stm32g4 #400
Stm32g4 #400
Conversation
… ft_c voltage, tf_a voltage, Add: missing i2c3, usb,
Add: usb cc resource in the stm32g4
There was a problem hiding this comment.
Pull Request Overview
This PR adds support for the STM32G431KB microcontroller variant along with a new USB CC mixin to enable USB power delivery support. Key changes include:
- Addition of the new microcontroller model file with updated pin resources and peripheral mappings.
- Introduction of the IoControllerUsbCc interface mixin for handling USB CC pins.
- Updates to module imports and registration for the new microcontroller.
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| edg/parts/Microcontroller_Stm32g431.py | New microcontroller variant with updated peripheral pin assignments |
| edg/abstract_parts/IoControllerInterfaceMixins.py | Added IoControllerUsbCc mixin for USB power delivery CC support |
| edg/abstract_parts/init.py | Registers the new IoControllerUsbCc mixin |
| edg/parts/init.py | Registers the new STM32G431KB variant |
Comments suppressed due to low confidence (2)
edg/parts/Microcontroller_Stm32g431.py:184
- The string 'PB6,' contains an extraneous comma; consider removing it to simply use 'PB6'.
PeripheralFixedResource('USBCC', UsbCcPort(DigitalBidir.empty()), { 'cc1': ['PB6,'], 'cc2': ['PB4'] }),
edg/parts/Microcontroller_Stm32g431.py:174
- [nitpick] Consider renaming the 'FDCAN' peripheral to align with the manufacturer's naming conventions if applicable.
PeripheralFixedResource('FDCAN', CanControllerPort(DigitalBidir.empty()), { 'tx': ['PA12', 'PB9'], 'rx': ['PA11', 'PB8'] }),
There was a problem hiding this comment.
Pull Request Overview
This PR introduces support for the STM32G431KB microcontroller variant by adding its dedicated implementation and updating associated system and interface mixins. Key changes include:
- Adding a new microcontroller variant (Stm32g431kb) and its implementation in edg/parts/Microcontroller_Stm32g431.py.
- Updating edg/parts/init.py to import the new variant.
- Adding the IoControllerUsbCc mixin to edg/abstract_parts to support USB CC functionality.
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| edg/parts/init.py | Import added for the new STM32G431KB variant |
| edg/parts/Microcontroller_Stm32g431.py | Complete implementation of the STM32G431KB microcontroller |
| edg/abstract_parts/init.py | Updated import list to include IoControllerUsbCc |
| edg/abstract_parts/IoControllerInterfaceMixins.py | New mixin class IoControllerUsbCc has been added |
Comments suppressed due to low confidence (2)
edg/parts/Microcontroller_Stm32g431.py:174
- [nitpick] Consider renaming 'FDCAN' to 'CAN' to align with the manufacturer's naming conventions.
PeripheralFixedResource('FDCAN', CanControllerPort(DigitalBidir.empty()), {
edg/parts/Microcontroller_Stm32g431.py:69
- [nitpick] Consider directly renaming the variable 'dio_tt_model' to 'dio_tta_model' at the time of its assignment to enhance clarity.
dio_tta_model = dio_tt_model
ducky64
left a comment
ducky64
left a comment
There was a problem hiding this comment.
Overall looks good, but a few style issues and one correctness issue (related to current draw).
| # Power and ground | ||
| self.pwr = self.Port(VoltageSink( | ||
| voltage_limits=(1.71, 3.6) * Volt, | ||
| current_draw=(0.001, 150.0) * mAmp # table 15 |
There was a problem hiding this comment.
150mA is a max limit, not the actual draw of the device. Table 21 gives the core running current, which specifies a worst-case max of 44mA (at a toasty 125C) and low of 14nA in sleep mode (Table 32).
At some point it would probably be worth modeling these limits (total current into device pins, total IO current), but we don't currently.
| ) | ||
| dac_model = AnalogSource.from_supply( | ||
| self.gnd, self.pwr, | ||
| # signal_out_bound=(0.2*Volt, -0.2*Volt), signal_out_bound only applies when output buffer on |
There was a problem hiding this comment.
Dead code is generally discouraged style-wise. Consider just leaving the bound in there (as a safe bound) and having a comment that it only applies in a specific case.
| 'sck': ['PB13', 'PF1'], 'ws': ['PB12', 'PF0'], 'sd': ['PA11', 'PB15'] | ||
| }), | ||
| PeripheralFixedResource('I2S3', I2sController(DigitalBidir.empty()), { | ||
| 'sck': ['PB3', ], 'ws': ['PA4', 'PA15'], 'sd': ['PB5'] |
There was a problem hiding this comment.
| 'sck': ['PB3', ], 'ws': ['PA4', 'PA15'], 'sd': ['PB5'] | |
| 'sck': ['PB3'], 'ws': ['PA4', 'PA15'], 'sd': ['PB5'] |
style nit
| "apply_defaults_to_fp_shapes": false, | ||
| "apply_defaults_to_fp_text": false, | ||
| "board_outline_line_width": 0.0381, | ||
| "board_outline_line_width": 0.038099999999999995, |
There was a problem hiding this comment.
This file shouldn't be changed since it's not relevant to this PR - can you revert it?
There was a problem hiding this comment.
dropped this commit, hence creating a new PR
|
Moved to #401 |
This is the same as #400, but it removed the accidental commit/push of EspLora. Also applies the fix to the comments. - [x] #400 (comment) - [x] #400 (comment) - [x] #400 (comment) - [x] #400 (comment)
This will add:
The following comments from @ducky64 have been reviewed and should be applied to it