EVerest / libocpp

C++ implementation of the Open Charge Point Protocol
Apache License 2.0
84 stars 45 forks source link

Discussion from OCPP WG: Duplicate Smart Charging implementations for 1.6J and 2.0.1 #468

Closed drmrd closed 3 days ago

drmrd commented 7 months ago

When work started on the Smart Charging functional block for OCPP 2.0.1, we thought that it would make sense to move business logic for functionality shared between OCPP 1.6 and 2.0.1 into common and share it between version implementations.[^messaging] Our efforts to wrap existing 1.6J behavior in tests and discussions from recent Cloud Communications Working Group meetings, however, have shown that there are fundamental incompatibilities between correct implementations of some use cases found in both OCPP 1.6J and 2.0.1. I've created this issue to document this and the takeaways from this work and these discussions for future reference. It can also serve as a natural place for further discussion.

A notable example of incompatibilities between OCPP versions arose when we introduced unit tests covering the ClearChargingProfiles request. @Pietfried confirmed that the behavior of ClearChargingProfiles is, in practice, fundamentally different between 1.6J and 2.0.1. When given multiple Charging Profile attributes to filter based upon, the business logic behind ClearChargingProfile behaves differently in 1.6 than in 2.0.1. In the former, the deleted profiles are the union of results matching some of the attribute filters, while in the latter, the intersection is always taken. I believe Piet said this was only clarified in an OCPP 1.6J erratum, but I don't have a reference handy.

This is a great example of why sharing business logic between the 1.6J and 2.0.1 implementations is more complex/problematic than anticipated. Without reflection, a new callback mechanism for version-specific code, elaborate feature flagging, or otherwise introducing a new pattern throughout the 1.6J codebase, the business logic for 1.6J and 2.0.1 can't be reconciled into a common shared library for all requests. Such an undertaking would be time intensive, require a strong grasp of both the 1.6J and 2.0.1 to be successful, and require much deeper investment into testing the legacy 1.6 codebase before we could be confident in a new 2.0.1 implementation. And all of this for an unclear amount of benefit.

With all this in mind, my team is moving forward with creating an implementation of the Smart Charging functional block for OCPP 2.0.1 that is based on the existing 1.6J code without committing to a more complex but DRYer solution with shared business logic for the two versions in common. Naturally, we will still use shared code in common where it makes sense, but the oftentimes minimal differences between 1.6J and 2.0.1 mean that there will still be some duplication for request/response pairs in both versions of the standard. If this proves to be a maintenance burden in the future, we can consider a more aggressive refactor at that time.

[^messaging]: Due to the differences between OCPP 1.6 and 2.0.1 messages, this would require that greater care be taken to separate OCPP message construction and handling from core business logic, which we consider to be a point in favor of the approach.

Pietfried commented 7 months ago

Hi @drmrd , I understand your concerns with moving the business logic for OCPP1.6 and OCPP2.0.1 smart charging into a the common domain because of the differences and incompatibilites that emerged over your analysis. We are fine with separating the implementations for smart charging between OCPP1.6 and OCPP2.0.1 and I agree with your assumption that this could even speed up the implementation and make it easier.

In my experience the most challenging task of the smart charging implementation is to handle the correct combining of ChargingProfiles. As far as I can see the combination of ChargingProfiles to calculate the composite schedule do not differ between OCPP1.6 and OCPP2.01 (except for one addtional ChargingProfilePurpose: ChargingStationExternalConstraints. I would highly recommend to have a look at the profile combination implementation in this function: https://github.com/EVerest/libocpp/blob/main/lib/ocpp/v16/smart_charging.cpp#L172 .

Happy to further discuss in todays WG call!

shankari commented 6 months ago

@Pietfried @drmrd is there a plan to pull out the composite schedule calculation into common code?

Pietfried commented 6 months ago

@Pietfried @drmrd is there a plan to pull out the composite schedule calculation into common code?

I would consider this after making sure there are no conflicts between 1.6 and 2.0.1 regarding the calculation of the composite schedule

Pietfried commented 3 days ago

Closing this because discussion is concluded.