search

mobilityhouse / ocpp

Python implementation of the Open Charge Point Protocol (OCPP).
MIT License
777 stars 306 forks source link

Return CallError if Call fails to unpack. #102

Open OrangeTux opened 4 years ago

OrangeTux commented 4 years ago

For every message ocpp.ChargePoint receives ocpp.messages.unpack() is called. See https://github.com/mobilityhouse/ocpp/blob/master/ocpp/charge_point.py#L135-L140

If unpacking fails ChargePoint.start() crashes with the exception thrown in unpack(). Depending on how ChargePoint is used the connection to the charger might be closed as well. See #101 .

Instead of crashing the ChargePoint an CallError could be returned if a Call is received that fails to unpack(). That will inform the charger about the problem and the connection stays in tact.

Not sure what to do with CallResult that fail to unpack(). We could return CallErrors on CallResults as well, I've seen at least 1 charger that implemented OCPP like that. But I'm afraid that the OCPP specification doesn't have a mechanism to inform the other party that it send an invalid CallResult. So returning CallErrors on CallResults would violate the specification.

DColadas commented 4 years ago

Keep in mind that a CallError must still have the UniqueId of the Call it answers to. A CallError should not be sent on every unpack() exception (using a new UniqueId could be worse than not returning an error at all, as it would violate the standard).

In my opinion, if the JSON cannot be parsed or the MessageType and UniqueId cannot be retrieved, the error should just be logged and not sent to the client.

OrangeTux commented 4 years ago

Good addition @DColadas. I totally agree.

Jared-Newell-Mobility commented 9 months ago

additional discussion in https://github.com/mobilityhouse/ocpp/issues/101