Incorrect profile selection for GetCompositeSchedule.req handler

[Pietfried]

OCPP Version

OCPP2.0.1

Describe the bug

Bug

Currently the GetCompositeSchedule.req handler does not include all required profiles of purpose TxDefaultProfile and TxProfile in its calculation.

Reason

This is an excerpt of the GetCompositeSchedule.req handler:

std::vector<ChargingProfile> valid_profiles = this->smart_charging_handler->get_valid_profiles(request.evseId);

auto schedule = this->smart_charging_handler->calculate_composite_schedule(
        valid_profiles, start_time, end_time, request.evseId, request.chargingRateUnit);

The issue is inside get_charging_profiles_for_evse which is called in get_valid_profiles:

std::vector<ChargingProfile> SmartChargingHandler::get_valid_profiles_for_evse(int32_t evse_id) {
    std::vector<ChargingProfile> valid_profiles;

    auto evse_profiles = this->database_handler->get_charging_profiles_for_evse(evse_id);
    for (auto profile : evse_profiles) {
        if (this->validate_profile(profile, evse_id) == ProfileValidationResultEnum::Valid) {
            valid_profiles.push_back(profile);
        }
    }

    return valid_profiles;
}

This function calls validate_profile with a profile that has already been validated when it was added. This leads to the fact that evse specific profiles are not returned as part of get_valid_profiles because reasons like DuplicateProfileValidityPeriod or TxProfileConflictingStackLevel occur since the profiles are basically compared to themselves.

To Reproduce

Run OCTT test case TC_K_41_CS

Anything else?

While I'm not completely aware of all implications, I think this can simply be solved by removing the validate_profile call inside get_valid_profiles_for_evse to avoid the repeated validation with the consequences described above.

We can then also think of naming this function get_profiles rather then get_valid_profiles assuming that all profiles that have been added previously are valid.

This would require us to drop K08_GetValidProfiles_IfInvalidProfileExists_ThenThatProfileIsNotReturned.

I think we should make add_profile private and only provide validate_and_add_profile as part of the public API.

[christopher-davis-afs]

While I'm not completely aware of all implications, I think this can simply be solved by removing the validate_profile call inside get_valid_profiles_for_evse to avoid the repeated validation with the consequences described above.

I don't think this the correct solution. Profiles can be invalidated by internal changes to the system, such as changes to EVSEs, ongoing transactions, or device model configuration values. So we should check that all profiles we compile into a composite schedule are valid. 1.6 has a get_valid_profiles () that checks for similar state that can change and is used in the same context.

With regards to the bug:

This leads to the fact that evse specific profiles are not returned as part of get_valid_profiles because reasons like DuplicateProfileValidityPeriod or TxProfileConflictingStackLevel occur since the profiles are basically compared to themselves.

Generally when comparing profiles, we ensure that we let pairs of profiles with the same profile.id pass through. There's one exception to this: the comparison for TxProfileConflictingStackLevel within validate_tx_profile(). It seems like we forgot to accept profiles with the same ID there. In all other cases, we shouldn't be running into any issues comparing profiles against themselves. To verify this we wrote tests to check all of the validation functions that would compare given profiles against existing profiles, and only validate_tx_profile() failed.

So the proper solution would be to add a check in validate_tx_profile() to allow profiles with the same ID.

[christopher-davis-afs]

If you would like to check for yourself, these are the tests:

TEST_F(SmartChargingHandlerTestFixtureV201, K01_ValidateChargingStationMaxProfile_AllowsExistingMatchingProfile) {
    auto profile =
        SmartChargingTestUtils::get_charging_profile_from_file("max/ChargingStationMaxProfile_grid_hourly.json");
    auto res = handler.validate_and_add_profile(profile, STATION_WIDE_ID);
    ASSERT_THAT(res.status, testing::Eq(ChargingProfileStatusEnum::Accepted));

    auto sut = handler.validate_charging_station_max_profile(profile, STATION_WIDE_ID);

    EXPECT_THAT(sut, testing::Eq(ProfileValidationResultEnum::Valid));
}

TEST_F(SmartChargingHandlerTestFixtureV201, K01_ValidateTxDefaultProfile_AllowsExistingMatchingProfile) {
    auto profile = SmartChargingTestUtils::get_charging_profile_from_file("singles/Absolute_301.json");
    auto res = handler.validate_and_add_profile(profile, DEFAULT_EVSE_ID);
    ASSERT_THAT(res.status, testing::Eq(ChargingProfileStatusEnum::Accepted));

    auto sut = handler.validate_tx_default_profile(profile, DEFAULT_EVSE_ID);

    EXPECT_THAT(sut, testing::Eq(ProfileValidationResultEnum::Valid));
}

TEST_F(SmartChargingHandlerTestFixtureV201, K01_ValidateTxProfile_AllowsExistingMatchingProfile) {
    auto profile = SmartChargingTestUtils::get_charging_profile_from_file("baseline/TxProfile_1.json");
    this->evse_manager->open_transaction(DEFAULT_EVSE_ID, profile.transactionId.value());

    auto res = handler.validate_and_add_profile(profile, DEFAULT_EVSE_ID);
    ASSERT_THAT(res.status, testing::Eq(ChargingProfileStatusEnum::Accepted));

    auto sut = handler.validate_tx_profile(profile, DEFAULT_EVSE_ID);

    EXPECT_THAT(sut, testing::Eq(ProfileValidationResultEnum::Valid));
}

The first two pass, and the last one fails with TxProfileConflictingStackLevel.

[Pietfried]

The DuplicateProfileValidityPeriod occured because the check for the overapping validity period of profiles used the combination:

stackLevel - chargingProfileKind- evseId

although

K01.FR.06 requires
stackLevel - chargingProfilePurpose- evseId