Skip to content
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

feat: add custom_data attribute to payload-related dataclasses #434

Merged
merged 3 commits into from
Mar 22, 2023

Conversation

will-afs
Copy link
Contributor

Motivation

OCPP-2.0.1_part2_specification_edition2 states:

'A CustomData element exists as an optional element in the JSON schemas of all types.
CustomData is the only class in the JSON schema files that allows additional properties.
It can thus be used to add additional custom attributes to any type.'

With some more details, provided in OCPP-2.0.1_part4_ocpp-j-specification:

'Whereas the standard HeartbeatRequest has an empty body, a customized version,
that provides the value of the main meter and a count of all sessions to date,
could look like this:
{
"customData": {
"vendorId": "com.mycompany.customheartbeat",
"mainMeterValue": 12345,
"sessionsToDate": 342
}
}
A CSMS that has implemented this extension, identified by its "vendorId",
will be able to process the data.
Other CSMS implementations will simply ignore these custom properties.'

OCPP 2.0.1 specific

OCPP 1.6, for its part, does define the same concept of enabling the exchange of custom information between a CS and a CSMS, but through dedicated messages: please refer to "4.3. Data Transfer" in ocpp-1.6 edition 2.

Why not a Payload dataclass from which other payload dataclasses could inherit

I was thinking to just create a Payload dataclass, which all OCPP payloads (in ocpp.v201.call.py and ocpp.v201.call_result.py) could inherit from:

from dataclasses import dataclass, field
from typing import Any, Dict

@dataclass
class Payload:
    custom_data: Optional[Dict[str, Any]] = field(default=None, init=False)

Example:

@dataclass
class StatusNotificationPayload(Payload):
    timestamp: str
    connector_status: str
    evse_id: int
    connector_id: int

But the thing is, TMH currently uses Python>=3.7. And before Python 3.10, inheritance with dataclasses is quite messy: with the approach proposed above, as custom_data is a default argument, payload classes inheriting from Payload would require having all of their attributes defining default values, which we obviously don't want.
So, instead, I have resignated myself to modify all OCPP payloads directly to define a custom_data: Optional[Dict[str, Any]] = None attribute for each payload class in ocpp.v201.call.py and ocpp.v201.call_result.py.

OrangeTux
OrangeTux previously approved these changes Mar 21, 2023
Copy link
Collaborator

@OrangeTux OrangeTux left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Many thanks for the PR! And also many thanks for the clear description!

The tests fail, can you make sure that they pass? After that I can merge the PR.

@will-afs
Copy link
Contributor Author

Many thanks for the PR! And also many thanks for the clear description!

The tests fail, can you make sure that they pass? After that I can merge the PR.

Hey, thanks for your feedback. I pushed a change to sync the PR's head with upstream/HEAD and fix the failing test. I think you need to approve again so that GitHub Action's tests can be started again?

@will-afs
Copy link
Contributor Author

Many thanks for the PR! And also many thanks for the clear description!
The tests fail, can you make sure that they pass? After that I can merge the PR.

Hey, thanks for your feedback. I pushed a change to sync the PR's head with upstream/HEAD and fix the failing test. I think you need to approve again so that GitHub Action's tests can be started again?

@OrangeTux

@OrangeTux OrangeTux merged commit d102fb5 into mobilityhouse:master Mar 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants