Use dataclasses for ChargingProfile usage#648
Use dataclasses for ChargingProfile usage#648jainmohit2001 merged 21 commits intomobilityhouse:masterfrom
Conversation
In order for remove_nones to work with the charging profile data structures it needs to be able to remove nones of nested dictionaries, as well as dictionaries made of lists of dictionaries (for example, a list of charging schedules represented as dicts). This commit modifies the logic, and adds tests
For now, dicts are acceptable to maintain backwards compatibility. Perhaps in v1.0 only the dataclass will be accepted.
* Change Decimal to float * Move dataclasses to separate charging_profile module * Add ChargingProfile type to RemoteStartTransaction payload
…drian/add_charging_profile_types Signed-off-by: Jérôme Benoit <jerome.benoit@piment-noir.org>
Signed-off-by: Jérôme Benoit <jerome.benoit@piment-noir.org>
|
@jerome-benoit would you like me to create unit tests similar to #684 so that its clearer to the community that these kind of changes are non-breaking? hopefully, that'll allow the new maintainers confidence in accepting these changes as well as the changes in #680 |
If your time permits, it will be helpful. I think that PR is editable by maintainers. |
ajmirsky
left a comment
There was a problem hiding this comment.
I believe that the charging profile datatypes that are added in charging_profile.py already exist in ocpp/v16/datatypes.py and shouldn't be duplicated.
ajmirsky
left a comment
There was a problem hiding this comment.
charging_profile field is optional; typing it as None makes it a required field.
Signed-off-by: Jérôme Benoit <jerome.benoit@piment-noir.org>
|
@jainmohit2001 any feedback for this PR? |
|
Looks good. Just remove the comments. I don't think they would be necessary. |
|
LGTM. We can merge this. |
PR #172 updated with master.
Smart charging algos conception and validation has become a requirement for charging infrastructure. Every single code lines that can help at implementing and testing it worse the effort.
closes #172
Associated unit tests PR: #690