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

Refactoring of composite schedule calculation #943

Merged
merged 2 commits into from
Feb 17, 2025

Conversation

marcemmers
Copy link
Contributor

Describe your changes

The main purpose of this change was to fix an issue discovered during testing with OCTT. The scenario:

  • The CS has multiple EVSEs
  • It has a ChargePointMaxProfile with a limit of 32A
  • It has a TxDefaultProfile with a limit of 10A installed on evseId 0 (so this counts for all EVSEs)
  • Getting a composite schedule for evseId 0 should then calculate the sum of the schedules for each evse and combine that with the ChargePointMaxProfile. This should result in a limit in the composite schedule of 20A.

Along the way I fixed another issue: Conversion between power and current was done to early resulting in issues especially when the number of phases was non default.

The bulk of the functionality to calculate the composite profile in profile.cpp has been split into several functions to make it easier to understand how the code is composed.

There are also still some gaps left in this code. We currently have default limits that are global, so the same for the whole charging station and EVSEs. It could be the case that different EVSEs have different default limits or that the CS has a higher limit (since it needs to support multiple EVSEs. This is something that will need to be added in a later PR.

Issue ticket number and link

Checklist before requesting a review

@marcemmers
Copy link
Contributor Author

I have disabled a bunch of unit tests in test_profile.cpp. These are tests that were built on the old profile.cpp implementation. I would like to propose to remove them all together since it is testing implementation details which will already be tested by test_composite_schedule.cpp and potentially others

@marcemmers marcemmers force-pushed the me-prepare-smart-charging-for-stationmaxcurrent-fix branch from aa4773b to d81e88c Compare January 22, 2025 08:46
Base automatically changed from me-prepare-smart-charging-for-stationmaxcurrent-fix to main January 22, 2025 16:32
@marcemmers marcemmers force-pushed the me-smart-charging-refactoring branch from 6a69867 to 73c840f Compare January 22, 2025 16:33
@marcemmers marcemmers force-pushed the me-smart-charging-refactoring branch from cc52536 to 5f00f03 Compare January 24, 2025 12:35
@marcemmers marcemmers force-pushed the me-smart-charging-refactoring branch from 5f00f03 to bedd08c Compare February 7, 2025 13:27
@marcemmers marcemmers requested a review from james-ctc February 10, 2025 08:47
Copy link
Contributor

@james-ctc james-ctc left a comment

Choose a reason for hiding this comment

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

the approach looks good. I've not spotted any code issues.

@Pietfried Pietfried merged commit 50520ce into main Feb 17, 2025
7 of 8 checks passed
@Pietfried Pietfried deleted the me-smart-charging-refactoring branch February 17, 2025 09:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants