-
Notifications
You must be signed in to change notification settings - Fork 112
Adding set_powersupply_capabilities cmd to ISO15118_charger interface #1390
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Adding set_powersupply_capabilities cmd to ISO15118_charger interface #1390
Conversation
Before merging this PR, EVerest/libiso15118#156 should be merged as well |
…. Now the min and max limits of the powersupply can be sent in the ChargeParameterDiscoveryRes messages Signed-off-by: Sebastian Lukas <[email protected]>
Signed-off-by: Sebastian Lukas <[email protected]>
627b4a1
to
b539c4d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks fine to me
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm, I'd recommend to reuse the existing power_supply_DC Capabilities type instead of introducing a new type with mostly similar properties
The supported grid code detection method supported by the EVSE | ||
type: string | ||
$ref: /iso15118#/GridCodeIslandingDetectionMethod | ||
Capabilities: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a reason why this existing type is not reused? https://github.com/EVerest/everest-core/blob/main/types/power_supply_DC.yaml#L36
BeforeCableCheck, | ||
}; | ||
|
||
struct PowerCapabilities { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: PowerSupplyCapabilities
types::iso15118::Capabilities capabilities; | ||
capabilities.max_charge_current = caps.max_export_current_A; | ||
capabilities.min_charge_current = caps.min_export_current_A; | ||
capabilities.max_charge_power = caps.max_export_power_W; | ||
capabilities.min_charge_power = caps.min_export_current_A * caps.min_export_voltage_V; | ||
capabilities.max_voltage = caps.max_export_voltage_V; | ||
capabilities.min_voltage = caps.min_export_voltage_V; | ||
capabilities.max_discharge_current = caps.max_import_current_A; | ||
capabilities.min_discharge_current = caps.min_import_current_A; | ||
capabilities.max_discharge_power = caps.max_import_power_W; | ||
capabilities.min_discharge_power = | ||
(caps.min_import_voltage_V.has_value() and caps.min_import_current_A.has_value()) | ||
? std::make_optional(caps.min_import_voltage_V.value() * caps.min_import_current_A.value()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could mostly be omited if types::power_supply_DC::Capabilities
is used
Describe your changes
Now the min and max limits of the powersupply can be sent in the ChargeParameterDiscoveryRes messages
Issue ticket number and link
The power supply limits should be sent to the EV in the DcChargeParameterDiscoveryRes and not the actual limits from the EnergyManager.
Checklist before requesting a review