-
Notifications
You must be signed in to change notification settings - Fork 23
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
Implemented Set Charging Profile API #44
Open
louisg1337
wants to merge
26
commits into
thoughtworks:main
Choose a base branch
from
louisg1337:set_charging_profile
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
… it to work with the current code I have. Also added in a few new structs to handle the request/response of the call, and started working on the handler function to deal with the response
…fixed up a cyclic import bug
…tChargingProfile if we needed to
…ata with lowercase values
shankari
suggested changes
Jun 19, 2024
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.
@louisg1337 My high level feedback is that you should move the [TEST]
log messages to Debug. And maybe label them as [API TRACE]
so we know what they are doing.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Context
Hello! I am on a team that is working on setting up software simulated demos for the charging software EVerest. These demos showcase how EVerest interacts with different CSMSs, especially using the latest protocols. One of the CSMSs that we really enjoy using is MaEVe, but we noticed that there wasn't any support for setting charging profiles just yet. This was one feature that we wanted to use to demonstrate EVerest's capabilities at the CharIn Testival, which just passed a few days ago, so we decided to implement it ourselves.
Implementation
For our use case, all we wanted was an API endpoint that, when called, would send a charging profile to EVerest. We made the API call accessible via
POST /cs/{csId}/setchargingprofile
, and theChargingProfileType
object would be provided in the body of the call.Testing Done
To test this new feature out, I used one of the software simulations demos that was setup to showcase EVerest during CharIn, which can be found here. These demos are meant to be plug and play as long as you have
Docker
anddocker-compose
installed. I'll walk through the steps here on how to run the demos and observe thesetChargingProfile
API request working.everest-demo
repo and run this script to call the newsetChargingProfile
API.We would love to integrate these changes into
maeve-csms
for future use and availability for anyone else whom may need it. Please let me know any changes that may need to be made to integrate this, thank you!