Standardize power path current draw conventions#411
Merged
Conversation
There was a problem hiding this comment.
Pull Request Overview
Standardizes current draw conventions for buck/boost converter power paths, adds inductor current peak tracking, fixes boost current limits and efficiency factors, and ensures rds_on is passed to FET blocks.
- Adopt buck switch as a
VoltageSinkdrawing average inductor current and boost switch as aVoltageSourceproviding max average output current - Introduce
actual_inductor_current_peakeverywhere and refactor chains to use it for FET sizing - Correct boost limit calculation (ratio and efficiency) and propagate
fet_rdsthrough custom converters
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| edg/parts/FanConnector.py | Updated comment to reflect correct startup duration and steady-state current draw |
| edg/parts/BuckConverter_Custom.py | Plumbed rds_on into FetHalfBridge, renamed force ports, switched to peak current |
| edg/parts/BuckBoostConverter_Custom.py | Added VoltageSourceConnector, removed voltage_drop, refactored chain logic |
| edg/abstract_parts/AbstractPowerConverters.py | Updated port types/comments, added actual_inductor_current_peak, adjusted connects |
Comments suppressed due to low confidence (3)
edg/parts/BuckConverter_Custom.py:41
- [nitpick] The forced-chain variables mix
pwr_in_forcewithsw_out_force. Consider using a consistent prefix/suffix (e.g.,sw_in_force/sw_out_forceorpwr_in_force/pwr_out_force) to improve clarity.
(self.pwr_in_force, ), _ = self.chain( # use average current draw for boundary ports
edg/abstract_parts/AbstractPowerConverters.py:396
- The new
actual_inductor_current_peakparameter isn’t covered by existing tests. Add unit tests to verify it’s computed correctly and propagated to downstream blocks.
self.assign(self.actual_inductor_current_peak,
edg/parts/BuckBoostConverter_Custom.py:22
- [nitpick] This docstring is ambiguous: the connector uses
VoltageSinkports on both sides. Clarify that it links a source port to a sink port and update parameter descriptions accordingly.
"""Connects two voltage sources together (inductor output to FET center 'source')."""
| self.Block(VoltageSinkConnector(self.output_voltage, | ||
| self.power_path.actual_inductor_current, | ||
| self.power_path.actual_avg_current_rating)), | ||
| Range.all(), # unused, port actually in reverse |
There was a problem hiding this comment.
[nitpick] Passing Range.all() as a placeholder argument is confusing. Consider overloading VoltageSinkConnector/VoltageSourceConnector or providing a named constant to make intent explicit.
Suggested change
| Range.all(), # unused, port actually in reverse | |
| UNUSED_CURRENT_LIMIT, # explicit placeholder for unused current limit |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Standardize conventions defined in the Buck/BoostConverterPowerPath: switch on buck is an VoltageSink that draws the average inductor current, and switch on the boost is a VoltageSource that provides the max average output current. Refactors the BuckBoostConverterPowerPath to use this convention. This isn't the cleanest, but this allows these parameters to propagate to the connected device (eg, buck converter controller IC's power-in pin) through the connection instead of needing to be plumbed explicitly. This makes the custom discrete converter a bit tougher, but probably a good trade-off.
Additional fixes:
Adds the actual inductor current peak for all converter power paths. Makes the discrete custom buck converter use this, instead of calculating it independently.
Resolves #406 and #307
TODO