-
Notifications
You must be signed in to change notification settings - Fork 48
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
K01 - Small cleanups #746
K01 - Small cleanups #746
Conversation
@shankari Please review. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As always, more documentation and explanations would be helpful!
doc/ocpp_201_status.md
Outdated
| K01.FR.40 | ✅ | `Absolute`/`Recurring` profiles without `startSchedule` fields are rejected. | | ||
| K01.FR.41 | ✅ | `Relative` profiles with `startSchedule` fields are rejected. | | ||
| K01.FR.42 | ⛽️K08 | | | ||
| K01.FR.43 | | Open question to OCA | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Link pls
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Giavotto: Is there a link available for this question?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unfortunately we don't have access to the OCA anymore.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm a bit confused by this statement. It sounds like this was a question that was asked to to the OCA. Does this mean that we could ask questions before but we can't ask questions now? Can we ask the community and have them ask the OCA?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The question was asked on the OCA Mailing list. For people with access to OCA Causeway: https://oca.causewaynow.com/wg/OCA-TWG/mail/thread/4254
I copied the question and reponse in Zulip: https://lfenergy.zulipchat.com/#narrow/stream/417676-EVerest.3A-Cloud-communication/topic/Smart.20charging.20delivery.20collaboration/near/457133329
With charging profiles, the charging can only be limited, not guaranteed. So if a profiles limits charging to 1 phase, but the charging station has no physical means to disconnect a number of phases, so always 2 or 3 phases are connected, it has to Reject the charging profile.
75cd267
to
9dc7d86
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The remaining comments are mainly to add additional documentation/links, so I am not going to hold up approval. @wjmp @couryrr-afs please track these requests in a separate issue and resolve them in a subsequent PR.
@shankari I just want to clarify the comment that needs tracked. The only open comment that I see is around the OCA link. These were added from the community members. Did you need anything else tracked? |
Signed-off-by: Gianfranco Berardi <[email protected]>
…t_charging_profile_req Signed-off-by: Christoph <[email protected]> Signed-off-by: Gianfranco Berardi <[email protected]>
…dsCallError Signed-off-by: Christoph <[email protected]> Signed-off-by: Gianfranco Berardi <[email protected]>
Signed-off-by: Christoph <[email protected]> Signed-off-by: Gianfranco Berardi <[email protected]>
…iew. Updated config to true Signed-off-by: Christoph <[email protected]> Signed-off-by: Gianfranco Berardi <[email protected]>
…n requirements updated some comment and test names after community discussion. Signed-off-by: Coury Richards <[email protected]> Signed-off-by: Gianfranco Berardi <[email protected]>
…ication. Signed-off-by: Gianfranco Berardi <[email protected]>
Co-authored-by: Piet Gömpel <[email protected]> Signed-off-by: Gianfranco Berardi <[email protected]>
Signed-off-by: Gianfranco Berardi <[email protected]>
Signed-off-by: Gianfranco Berardi <[email protected]>
Signed-off-by: Gianfranco Berardi <[email protected]>
…ning Signed-off-by: Coury Richards <[email protected]> Signed-off-by: Gianfranco Berardi <[email protected]>
@couryrr-afs quickly spot checking, there is also #746 (comment) and #746 (comment) |
Signed-off-by: Gianfranco Berardi <[email protected]>
config.json was removed. Signed-off-by: Gianfranco Berardi <[email protected]>
804bb2d
to
484afc3
Compare
Signed-off-by: Gianfranco Berardi <[email protected]>
Signed-off-by: Gianfranco Berardi <[email protected]>
@Pietfried Is this PR ready to merge? I don't have write access to the repository, if so. |
Describe your changes
Created a new constructor for ChargePoint which allows dependency injection of the DeviceModel, DatabaseHandler, and MessageQueue. This change should allow for unit tests to be easier to create as the context for the tests can more easily be controlled.
Also implemented K01-FR29 - return an error when Smart Charging is not supported when receiving a ChargePoint::handle_set_charging_profile_req.
Update the status doc to reflect current state.
Checklist before requesting a review