K01: Implement Full Validate Profile Method & Required Methods #611
Conversation
|
CC: @shankari |
christopher-davis-afs
force-pushed
the
feature/k01-validate-profile
branch
2 times, most recently
from
800e8c0 to
6cd39a1
Compare
christopher-davis-afs
force-pushed
the
feature/k01-validate-profile
branch
3 times, most recently
from
14c4ab5 to
ee4eac7
Compare
couryrr-afs
force-pushed
the
feature/k01-validate-profile
branch
from
ee4eac7 to
ec8fbee
Compare
christopher-davis-afs
force-pushed
the
feature/k01-validate-profile
branch
from
e8c77be to
d4016cd
Compare
christopher-davis-afs
force-pushed
the
feature/k01-validate-profile
branch
2 times, most recently
from
0541f70 to
8bed65f
Compare
|
@the-bay-kay the high-level description is not quite correct, this PR addresses the following functional requirements:
Please read through my review and review the changes yourself so you get familiar with the process. |
There was a problem hiding this comment.
For ease of review, it would be helpful to have a description of which functions are related to with FR, preferably in the code, so that it is easier for future maintainers to find it while reading through the code.
I have several comments, but they are primarily around improved documentation and design choices. I don't see anything obviously incorrect, and obviously the tests pass. This has already been pending for a while, so I will approve to unblock @marcemmers and anticipate that the comments will be addressed in a future cleanup PR.
Tests for K01.FR.39 were previously mislabeled for K01.FR.06. This commit relabels these tests. Signed-off-by: Christopher Davis <[email protected]>
Signed-off-by: Coury Richards <[email protected]>
Signed-off-by: Coury Richards <[email protected]>
Signed-off-by: Christoph <[email protected]>
Signed-off-by: Christoph <[email protected]>
Signed-off-by: Christoph <[email protected]>
Signed-off-by: Christopher Davis <[email protected]>
…period This was missed in prior commits. Signed-off-by: Christopher Davis <[email protected]>
Signed-off-by: Christopher Davis <[email protected]>
Reworks how we handle and test conforming the validity periods for K01.FR.06. Signed-off-by: Christopher Davis <[email protected]>
…tion Signed-off-by: Christopher Davis <[email protected]>
Signed-off-by: Christopher Davis <[email protected]>
Now we can catch regressions that make valid profiles read as invalid, and vice versa. Signed-off-by: Christopher Davis <[email protected]>
Signed-off-by: Christopher Davis <[email protected]>
Signed-off-by: Christopher Davis <[email protected]>
Signed-off-by: Christopher Davis <[email protected]>
Signed-off-by: Christopher Davis <[email protected]>
Create a static UUID and only generate a new one for tests that rely on having different transaction IDs. Signed-off-by: Christopher Davis <[email protected]>
christopher-davis-afs
force-pushed
the
feature/k01-validate-profile
branch
from
8bed65f to
6b516b6
Compare
Signed-off-by: Christopher Davis <[email protected]>
Signed-off-by: Christopher Davis <[email protected]>
Signed-off-by: Christopher Davis <[email protected]>
Signed-off-by: Christopher Davis <[email protected]>
Signed-off-by: Christopher Davis <[email protected]>
christopher-davis-afs
requested review from
Pietfried,
hikinggrass and
maaikez
as code owners
|
@christopher-davis-afs Just give me a sign that it is ready to merge and I will merge it ;) |
|
It is ready :) |
|
@couryrr-afs @drmrd @wjmp I believe that there were several comments that here that the reviewers wanted to change, but didn't want to hold up the merge for. Now that the merge is done, it would be great to see a cleanup PR address the comments. Also, some of my comments were marked as resolved, but I don't agree with the resolution 😄 |
@shankari Yep — we haven’t forgotten about the code items and can get the comments added to the code base you requested. We have been prioritizing getting the Validate Profile PR merged, the Add Profiles PR ready in support of Alfen’s timelines and some other blocker around that. I thought it best to resolve comments prior to presenting the PR to the community as to avoid any confusion or further delay based on your comment about the review. We will have a PR out soon. |
Describe your changes
This PR finishes fleshing out the profile validation work in
SmartChargingHandler. It finalizes the implementation for a few functional requirements:With this PR, we can now call
validate_profile()and, based on the type of the given profile, run the specific validation methods that are applicable. We also simplify the function signatures of some of the previously added methods to take the EVSE ID.Issue ticket number and link
Checklist before requesting a review