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

Feature/k02 05 cumulative #760

Merged
merged 8 commits into from
Sep 17, 2024
Merged

Conversation

couryrr-afs
Copy link
Contributor

Describe your changes

  • K04.FR.01 added get_profiles_on_evse() to verify that profiles are EVSE-specific and are only applied to that EVSE.
  • Added AddChargingProfileSource to distinguish profile source for validation. Specifically, when validate_and_add_profile is called from RequestStartTransactionRequest.
  • Removed AddChargingProfileSource::Unknown defaults to
    AddChargingProfileSource::SetChargingProfile for the most validation
    checks.
  • Added a check on SmartChargingCtrlrAvailableEnabled for F01.FR26,
    K05.FR04 and K05.FR05
  • Added check for ChargingProfilePurposeEnum::TXProfile K05.FR02
  • K02.FR.05 delete transaction-specific profile from database.

Issue ticket number and link

Checklist before requesting a review

@couryrr-afs
Copy link
Contributor Author

@shankari - this is ready for your review.

Copy link
Contributor

@Pietfried Pietfried left a comment

Choose a reason for hiding this comment

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

In general this looks good to me. The most relevant in line comment is about how to handle a profile that is not a TxProfile as part of a RequestStartTransactionRequest.

This branch currently has conflicts that need to be resolved.

doc/ocpp_201_status.md Outdated Show resolved Hide resolved
include/ocpp/v201/smart_charging.hpp Show resolved Hide resolved
include/ocpp/v201/smart_charging.hpp Show resolved Hide resolved
lib/ocpp/v201/charge_point.cpp Show resolved Hide resolved
Copy link
Contributor

@Pietfried Pietfried left a comment

Choose a reason for hiding this comment

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

lgtm 👍

@couryrr-afs couryrr-afs force-pushed the feature/k02-05-cumulative branch 2 times, most recently from 8740842 to 9dabfec Compare September 5, 2024 20:00
Copy link

@shankari shankari left a comment

Choose a reason for hiding this comment

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

I have some suggested cleanup changes, but nothing to hold up approval.

doc/ocpp_201_status.md Outdated Show resolved Hide resolved
include/ocpp/v201/smart_charging.hpp Show resolved Hide resolved
tests/lib/ocpp/v201/test_charge_point.cpp Show resolved Hide resolved
@couryrr-afs couryrr-afs force-pushed the feature/k02-05-cumulative branch 2 times, most recently from 58ffa24 to 7aa94d3 Compare September 6, 2024 13:20
@couryrr-afs
Copy link
Contributor Author

@Pietfried - looks like we are failing here with a SegFault again. This passed yesterday just fine none of the recent updates changes behavior for the code.

@couryrr-afs
Copy link
Contributor Author

@shankari I responded to your comments. Should this be approved?

@shankari
Copy link

shankari commented Sep 6, 2024

It was approved 10 hours ago


Screenshot 2024-09-06 at 7 46 11 AM

@couryrr-afs couryrr-afs marked this pull request as ready for review September 9, 2024 12:56
K04.FR.01 states that charging schedules should be managed per EVSE.
This commit adds a test (and a helper function,
`get_profiles_on_evse()`) to verify that profiles are EVSE-specific
are only applied to that EVSE.

Signed-off-by: Christopher Davis <[email protected]>
Use the new `get_profiles_for_evse()` helper function.

Signed-off-by: Christopher Davis <[email protected]>
- removed AddChargingProfileSource::Unknown defaults to
AddChargingProfileSource::SetChargingProfile for the most validation
checks.

- added a check on SmartChargingCtrlrAvailableEnabled for F01.FR26,
K05.FR04 and K05.FR05

- added check for ChargingProfilePurposeEnum::TXProfile K05.FR02

Signed-off-by: Coury Richards <[email protected]>
Delete transaction-specific profile from database.
When the transaction ends, then delete TxProfiles.

Signed-off-by: Christoph <[email protected]>
Co-authored-by: Gianfranco Berardi <[email protected]>
@couryrr-afs
Copy link
Contributor Author

@Pietfried this has been rebased and is ready to be merged.

@Pietfried Pietfried merged commit 71f8b1a into EVerest:main Sep 17, 2024
4 checks passed
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.

6 participants