Add dataclasses for ChargingProfile and its constituents

[grutts]

Some questions:

• These objects don't quite fit into calls.py, because they're not call payloads. On the other hand, they don't really fit in enums.py either. I considered adding a new file like types.py, but figured it best to see what your preference is?

• I also wanted to add a test to ensure a SetChargingProfilePayload is serialised to the correct representation when using these objects. I noticed the codebase doesn't have such tests for other payloads, so I left it. Would you like to keep it this way?

Thanks!

[OrangeTux]

Thanks for your PR!

I'm unsure what to do this PR. I think your code is an improvement and should be included at some point.. However, the changes are backwards incompatible with the current code.

There are more backwards incompatible changes that we want to integrate on our way to a v1.0.0 release. I'll discuss with others how and what to do.

In the mean time there are some things that you can improve to your code.

Nested dataclasses can't be deserialized. Consider the following example:

from dataclasses import asdict
from ocpp.v16.call import ChargingSchedule, ChargingProfile, ChargingSchedulePeriod
from ocpp.v16.enums import ChargingProfilePurposeType, ChargingProfileKindType, ChargingRateUnitType

charging_schedule_period = { 'start_period': 0, 'limit': 10 }

charging_schedule = {
        'charging_rate_unit': ChargingRateUnitType.amps,
        'charging_schedule_period': [charging_schedule_period],
    }

charging_profile = {
    'charging_profile_id': 1,
    'stack_level': 0,
    'charging_profile_purpose': ChargingProfilePurposeType.tx_profile,
    'charging_profile_kind': ChargingProfileKindType.absolute,
    'charging_schedule': charging_schedule,
}


profile = ChargingProfile(**charging_profile)

# This fails as `profile.charging_schedule` is of type `dict`.
assert type(profile.charging_schedule) == ChargingSchedule

Can you you modify the code and make sure that nested dataclasses are deserialized correctly? I think you can use __post_init__() for that.

Regarding the location, I suggest to put the code in it's own module ocpp.v16.charging_profile. It doesn't really fit in ocpp.v16.call as that module contains only payloads to construct OCPP Calls. It also doesn't fit in ocpp.v16.enums because it isn't an enum.

[grutts]

No problem!

Good point regarding the de-serialisation, I had missed that. I'll look to make changes to support this.

Apologies, I don't quite understand the backwards incompatibility. My understanding when the payload is passed to Call all dataclasses are converted to dictionaries (similarly before any validation), and there is no enforcing of the types on the dataclass, and thus any code which is passing dictionaries to cs_charging_profiles would continue to work. Similarly, my understanding is we don't de-serialise back to the payload class automatically anywhere (nor validate against the dataclass). I do see a backwards incompatibility in static type checking (i.e. mypy), and if this is your concern I would propose we modify the type to still accept dicts (cs_charging_profiles: Untion[ChargingProfile, Dict]) ? Or am I missing something?

[OrangeTux]

Changing type of SetChargingProfilePayload.cs_charging_profiles from Dict to ChargingProfile is a backwards incompatible change.

However, you're right when you write "my understanding is we don't de-serialise back to the payload class automatically anywhere ". In practice it means users implementing a @on() handler for SetChargingProfile still get cs_charging_profiles as a Dict.

Changing the type hints, as you suggested, is required to express that cs_charging_profiles can be both Dict or ChargingProfile. And with this modification this PR is backwards compatible again.

I think for v1.0.0 we should drop Dict as possible type for cs_charging_profiles and only allow ChargingProfile. That means this library should automatically deserialize incoming SetChargingProfile requests into ChargingProfile and the other nested dataclasses. But these change are in the scope of a different issue.

[grutts]

Ok, I follow you now. I'll go with Union for now then to avoid making a backwards incompatible change.

I agree with the changes on (de)serialisation for v1.0.0 - we're doing this internally on top of the library at the minute at it would be handy to have it upstream!

[grutts]

remove_nones needed to change to remove keys where the value is None for nested dicts, and dicts which contains lists of dicts. These structures appear in the charging profile code. I've added that in this commit 19bfe11

If we agree that deserialisation of dicts to dataclasses is out of scope and something for v1.0 then I think this PR is ready to go.

[proelke]

There is a PR I'd like to submit and was working on a variation of your change to remove_nones. +1 on that change from me.

[grutts]

@OrangeTux @tropxy I think this is ready to go. Are there any other changes you need?

[jerome-benoit]

The PR is quite interesting in the context of smart charging algos conception and validation, which has become a requirement for charging infrastructure.

What is the rationale for not next merging it since 3 years?