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.

Sign up for GitHub

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

Jump to bottom

Add schedule validation #521

Merged
merged 1 commit into from
Mar 15, 2024
Merged

Add schedule validation #521

merged 1 commit into from
Mar 15, 2024

Conversation

christopher-davis-afs
Copy link
Contributor

@christopher-davis-afs christopher-davis-afs commented Mar 8, 2024 •
edited
Loading

Describe your changes

Depends on #452 Ready now that #452 is merged :)

Implements:

  • K01.FR.19
  • K01.FR.31
  • K01.FR.35
  • K01.FR.40
  • K01.FR.41

Does not implement:

  • K01.FR.20
  • K01.FR.34
  • K01.FR.43
  • K01.FR.45
  • K01.FR.48

These functional requirements rely on functionality in DeviceModel, the EVSE, or message handling that is not currently implemented in Everest.

Checklist before requesting a review

  • I have performed a self-review of my code
  • I have made corresponding changes to the documentation
  • I read the contribution documentation and made sure that my changes meet its requirements

shankari
shankari approved these changes Mar 11, 2024
View reviewed changes
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.

No objections from a structural perspective. I spot checked some of the specifications and they seem fairly straightforward.

@drmrd @christopher-davis-afs are we tracking which of the specs are in various stages (not started, under development, in review, merged). I don't think we have an issue for each specification, and I want to make sure that they don't fall through the cracks.

@christopher-davis-afs christopher-davis-afs force-pushed the couryrr/add-validate-schedule branch from 233984a to 39849f7 Compare March 11, 2024 14:31
@christopher-davis-afs christopher-davis-afs marked this pull request as ready for review March 11, 2024 14:32
@couryrr-afs
Copy link
Contributor

No objections from a structural perspective. I spot checked some of the specifications and they seem fairly straightforward.

@drmrd @christopher-davis-afs are we tracking which of the specs are in various stages (not started, under development, in review, merged). I don't think we have an issue for each specification, and I want to make sure that they don't fall through the cracks.

We are implementing a process to comprehensively track the full requirements of the specs functional blocks and their corresponding implementations status for improved visibility. Our current plan is maintaining a markdown list that shows the overall progress, with each user story containing a subsection of the list as acceptance criteria. When a story is complete the markdown will be updated.

@Pietfried
Copy link
Contributor

Looks good from my side!

Considering:

  • K01.FR.43
  • K01.FR.45
  • K01.FR.48

Could we include the SupplyPhases from the device model for Connector, EVSE and ChargingStation to validate the schedules?

@christopher-davis-afs
Copy link
Contributor Author

Looks good from my side!

Considering:

* K01.FR.43

* K01.FR.45

* K01.FR.48

Could we include the SupplyPhases from the device model for Connector, EVSE and ChargingStation to validate the schedules?

What do you mean by include here?

@Pietfried
Copy link
Contributor

Looks good from my side!
Considering:

* K01.FR.43

* K01.FR.45

* K01.FR.48

Could we include the SupplyPhases from the device model for Connector, EVSE and ChargingStation to validate the schedules?

What do you mean by include here?

The tests could request the SupplyPhases variables from the device model to be able to check the precondition for K01.FR.43, K01.FR.45 and K01.FR.43 and test for the requirement definition

@christopher-davis-afs
Copy link
Contributor Author

The preconditions for those are:

  • K01.FR.43: When a SetChargingProfileRequest with a value for numberPhases is received AND the EVSE is of type AC AND the Charging Station cannot ensure that no more than the received numberPhases will be used...
  • K01.FR.45: When a SetChargingProfileRequest with a value for numberPhases is received AND the EVSE is of type AC AND the received numberPhases is NOT supported by the Charging Station and higher than the numberPhases that are supported by the Charging Station...
  • K01.FR.48: When a SetChargingProfileRequest with a value for phaseToUse is received AND the EVSE is NOT capable of switching the phase connected to the EV, which is indicated by ACPhaseSwitchingSupported not being implemented or defined as false AND the EVSE is NOT going to use the received phaseToUse...

K01.FR.48 requires checking ACPhaseSwitchingSupported on the EVSE via DeviceModel, and as far as we can tell there's not a way to know whether or not an EVSE is of type AC or not in the current Everest codebase. We also were not able to figure out how to get a variable on a specific EVSE via DeviceModel - when we last asked about it we were told that the device model functionality was not fully hooked up in everest since we need access to the charger state machine, which is also not hooked up. So I'm a little unsure what changes to make here given the DeviceModel functionality isn't really fleshed out.

Am I misunderstanding the question? Or did I misunderstand the answer to the DeviceModel question?

@shankari
Copy link

@couryrr-afs wrt #521 (comment) can you please add a link to the markdown to this issue once it has been generated?

@couryrr-afs
Copy link
Contributor

@shankari https://github.com/US-JOET/base-camp/issues/94

@christopher-davis-afs @couryrr-afs
smart_charging: Add validation for profile schedules
0298bc2 
Implements:

* K01.FR.19
* K01.FR.31
* K01.FR.35
* K01.FR.40
* K01.FR.41

Does not implement:

* K01.FR.20
* K01.FR.34
* K01.FR.43
* K01.FR.45
* K01.FR.48

These functional requirements rely on functionality
in DeviceModel, the EVSE, or message handling that
is not currently implemented in Everest.

Co-authored-by: Coury Richards <[email protected]>
Signed-off-by: Christopher Davis <[email protected]>
@christopher-davis-afs christopher-davis-afs force-pushed the couryrr/add-validate-schedule branch from 39849f7 to 0298bc2 Compare March 14, 2024 17:08
@Pietfried Pietfried merged commit 84477ba into EVerest:main Mar 15, 2024
4 checks passed
This was referenced May 2, 2024
Open
Open
@drmrd
Copy link
Collaborator

drmrd commented May 2, 2024

Contributes to #361

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Pietfried Pietfried Pietfried approved these changes

@shankari shankari shankari approved these changes

@hikinggrass hikinggrass Awaiting requested review from hikinggrass hikinggrass is a code owner

@marcemmers marcemmers Awaiting requested review from marcemmers marcemmers is a code owner

@maaikez maaikez Awaiting requested review from maaikez maaikez is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@christopher-davis-afs @couryrr-afs @Pietfried @shankari @drmrd