Feature/add protocol state to interface #136
Conversation
| await self.comm_session.evse_controller.set_precharge( | ||
| precharge_req.ev_target_voltage, precharge_req.ev_target_current | ||
| ) | ||
| self.expect_pre_charge_req = False |
There was a problem hiding this comment.
since we are setting it at every time, I dont think we need the flag anymore and can be removed
There was a problem hiding this comment.
In my opinion it is still used for the function "self.check_msg_dinspec(....)".
But i moved the line (where the flag is set) to the end of the State "PreCharge". Like in other states.
| await self.comm_session.evse_controller.set_precharge( | ||
| precharge_req.ev_target_voltage, precharge_req.ev_target_current | ||
| ) | ||
| self.precharge_req_was_reveived = True |
There was a problem hiding this comment.
same comment as for the DIN
There was a problem hiding this comment.
same for DIN
| DIN_REAL_WORLD_MESSAGES = [ | ||
| TestMessage( | ||
| message_name="ChargeParameterDiscoveryReq", | ||
| json_str='{"V2G_Message": {"Header": {"SessionID": "C427C77F5FAA1DD5"},' | ||
| ' "Body": {"ChargeParameterDiscoveryReq": ' | ||
| '{"EVRequestedEnergyTransferType": "DC_extended", ' | ||
| '"DC_EVChargeParameter": {"DC_EVStatus": {"EVReady": false, ' | ||
| '"EVErrorCode": "NO_ERROR", "EVRESSSOC": 43}, ' | ||
| '"EVMaximumCurrentLimit": {"Multiplier": -1, "Value": 3500}, ' | ||
| '"EVMaximumVoltageLimit": {"Multiplier": -1, "Value": 4690}, ' | ||
| '"EVEnergyRequest": {"Multiplier": 0, "Value": 500}}}}}}', | ||
| description="EV: VW ID3; Date: 06.09.2022; Element 'Unit' in " | ||
| "PhysicalValuetype in DIN is optional. " | ||
| "In ISO it is mandatory.", | ||
| ), | ||
| ] | ||
|
|
||
|
|
There was a problem hiding this comment.
I dont think it is necessary to create a separate test for this case. We can simply add the payload to the DIN_TEST_MESSSAGES. The description on the the TestMessage is enough to understand the purpose
| @@ -328,6 +328,139 @@ class PVStartValue(PhysicalValue): | |||
| unit: Literal[UnitSymbol.WATT] = Field(..., alias="Unit") | |||
|
|
|||
|
|
|||
| class PVEVEnergyCapacityDin(PVEVEnergyCapacity): | |||
There was a problem hiding this comment.
for clarity reasons, add comment on top stating that due to the fact the unit field is Option in the DIN, we have to override the ISO 15118 datatypes
| super(PVEVMaxVoltageLimit) | ||
| unit: Literal[UnitSymbol.VOLTAGE] = Field(None, alias="Unit") | ||
|
|
||
|
|
||
| class PVEVSECurrentRegulationToleranceDin(PVEVSECurrentRegulationTolerance): | ||
| """See section 9.5.2.4 in DIN SPEC 70121""" | ||
|
|
||
| super(PVEVSECurrentRegulationTolerance) | ||
| unit: Literal[UnitSymbol.AMPERE] = Field(None, alias="Unit") | ||
|
|
||
|
|
||
| class PVEVSEEnergyToBeDeliveredDin(PVEVSEEnergyToBeDelivered): | ||
| """See section 9.5.2.4 in DIN SPEC 70121""" | ||
|
|
||
| super(PVEVSEEnergyToBeDelivered) | ||
| unit: Literal[UnitSymbol.WATT_HOURS] = Field(None, alias="Unit") | ||
|
|
||
|
|
||
| class PVEVSEMaxCurrentLimitDin(PVEVSEMaxCurrentLimit): | ||
| """See section 9.5.2.4 in DIN SPEC 70121""" | ||
|
|
||
| super(PVEVSEMaxCurrentLimit) | ||
| unit: Literal[UnitSymbol.AMPERE] = Field(None, alias="Unit") | ||
|
|
||
|
|
||
| class PVEVSEMaxPowerLimitDin(PVEVSEMaxPowerLimit): | ||
| """See section 9.5.2.4 in DIN SPEC 70121""" | ||
|
|
||
| super(PVEVSEMaxPowerLimit) | ||
| unit: Literal[UnitSymbol.WATT] = Field(None, alias="Unit") | ||
|
|
||
|
|
||
| class PVEVSEMaxVoltageLimitDin(PVEVSEMaxVoltageLimit): | ||
| """See section 9.5.2.4 in DIN SPEC 70121""" | ||
|
|
||
| super(PVEVSEMaxVoltageLimit) | ||
| unit: Literal[UnitSymbol.VOLTAGE] = Field(None, alias="Unit") | ||
|
|
||
|
|
||
| class PVEVSEMinCurrentLimitDin(PVEVSEMinCurrentLimit): | ||
| """See section 9.5.2.4 in DIN SPEC 70121""" | ||
|
|
||
| super(PVEVSEMinCurrentLimit) | ||
| unit: Literal[UnitSymbol.AMPERE] = Field(None, alias="Unit") | ||
|
|
||
|
|
||
| class PVEVSEMinVoltageLimitDin(PVEVSEMinVoltageLimit): | ||
| """See section 9.5.2.4 in DIN SPEC 70121""" | ||
|
|
||
| super(PVEVSEMinVoltageLimit) | ||
| unit: Literal[UnitSymbol.VOLTAGE] = Field(None, alias="Unit") | ||
|
|
||
|
|
||
| class PVEVSEPeakCurrentRippleDin(PVEVSEPeakCurrentRipple): | ||
| """See section 9.5.2.4 in DIN SPEC 70121""" | ||
|
|
||
| super(PVEVSEPeakCurrentRipple) | ||
| unit: Literal[UnitSymbol.AMPERE] = Field(None, alias="Unit") | ||
|
|
||
|
|
||
| class PVEVSEPresentCurrentDin(PVEVSEPresentCurrent): | ||
| """See section 9.5.2.4 in DIN SPEC 70121""" | ||
|
|
||
| super(PVEVSEPresentCurrent) | ||
| unit: Literal[UnitSymbol.AMPERE] = Field(None, alias="Unit") | ||
|
|
||
|
|
||
| class PVEVSEPresentVoltageDin(PVEVSEPresentVoltage): | ||
| """See section 9.5.2.4 in DIN SPEC 70121""" | ||
|
|
||
| super(PVEVSEPresentVoltage) | ||
| unit: Literal[UnitSymbol.VOLTAGE] = Field(None, alias="Unit") | ||
|
|
||
|
|
||
| class PVEVTargetCurrentDin(PVEVTargetCurrent): | ||
| """See section 9.5.2.4 in DIN SPEC 70121""" | ||
|
|
||
| super(PVEVTargetCurrent) | ||
| unit: Literal[UnitSymbol.AMPERE] = Field(None, alias="Unit") | ||
|
|
||
|
|
||
| class PVEVTargetVoltageDin(PVEVTargetVoltage): | ||
| """See section 9.5.2.4 in DIN SPEC 70121""" | ||
|
|
||
| super(PVEVTargetVoltage) | ||
| unit: Literal[UnitSymbol.VOLTAGE] = Field(None, alias="Unit") | ||
|
|
||
|
|
||
| class PVRemainingTimeToFullSOCDin(PVRemainingTimeToFullSOC): | ||
| """See section 9.5.2.4 in DIN SPEC 70121""" | ||
|
|
||
| super(PVRemainingTimeToFullSOC) | ||
| unit: Literal[UnitSymbol.SECONDS] = Field(None, alias="Unit") | ||
|
|
||
|
|
||
| class PVRemainingTimeToBulkSOCDin(PVRemainingTimeToBulkSOC): | ||
| """See section 9.5.2.4 in DIN SPEC 70121""" | ||
|
|
||
| super(PVRemainingTimeToBulkSOC) | ||
| unit: Literal[UnitSymbol.SECONDS] = Field(None, alias="Unit") |
There was a problem hiding this comment.
you dont need the super. Overriding the unit will do the trick. Check:
In [1]: class Base:
...: unit: int = 3
...:
In [2]: class Child(Base):
...: unit: str = "lala"
...:
In [3]: test_base = Base()
In [4]: test_child = Child()
In [5]:
In [5]: test_base.unit
Out[5]: 3
In [6]: test_child.unit
Out[6]: 'lala'There was a problem hiding this comment.
Yes, you are right.
|
Hi André, thanks for the review! |
| @@ -1950,7 +1950,7 @@ async def process_message( | |||
| msg = self.check_msg_v2( | |||
| message, | |||
| [PreChargeReq, PowerDeliveryReq], | |||
| not self.precharge_req_was_reveived, | |||
| not self.expecting_precharge_req, | |||
There was a problem hiding this comment.
this now is confusing... I guess you meant to set the expecting_precharge_req to True, so that here we actually are checking for the presence of it and not for the not self.expecting_precharge_req
There was a problem hiding this comment.
Yes you are right.
I fixed it und pushed it.
lukaslombriserdesignwerk
force-pushed
the
feature/add_protocol_state_to_interface
branch
from
84cf7f1 to
9a17cc9
Compare
…oesn't send units)
lukaslombriserdesignwerk
force-pushed
the
feature/add_protocol_state_to_interface
branch
from
1cff7f7 to
9a6c734
Compare
|
HI André would be great if you could do the review/merge before the next branch will be merged. Otherwise I have to rebase again :). best regards. |
Added the following features: