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 source to enable/disable commands/events #505

Merged
merged 7 commits into from
Jun 3, 2024
Merged

Add source to enable/disable commands/events #505

merged 7 commits into from
Jun 3, 2024

Conversation

corneliusclaussen
Copy link
Contributor

@corneliusclaussen corneliusclaussen commented Jan 23, 2024 •
edited
Loading 

Add new enable_disable commands to EvseManager

It replaces the old enable/disable commands and adds a source and
priority. See README of the API module for more details.

Compile time breaking change on the evse_manager interface.
All enable/disable calls need to be switched over to the new
enable_disable call, for everest-core this is done in this PR.

In some uses cases it is useful if e.g. a LocalAPI source disables
a connector that only the same local source can enable again.
Think of a service technician who has an app to disable the connector
locally. Then it should be prevented that it is re-enabled by the CSMS
until the service technician enables it again via the same app.
Using priorities this is quite flexible and a lot of use-cases can
be implemented without a change in EVerest code.

The API module also has a new enable_disable command. It also has
compatibility implementations for the old enable/disable commands.

@corneliusclaussen corneliusclaussen force-pushed the feature/enable_disable_source branch 2 times, most recently from 84a4278 to 847d96f Compare January 23, 2024 08:41
@corneliusclaussen corneliusclaussen marked this pull request as draft January 24, 2024 13:26
@corneliusclaussen corneliusclaussen force-pushed the feature/enable_disable_source branch 5 times, most recently from 88c8d2a to bfb8e30 Compare March 21, 2024 11:42
@corneliusclaussen corneliusclaussen added post-release Tag that this PR should not go into the current merge window for the upcoming release and removed post-release Tag that this PR should not go into the current merge window for the upcoming release labels Mar 22, 2024
@corneliusclaussen corneliusclaussen added the post-release Tag that this PR should not go into the current merge window for the upcoming release label Mar 22, 2024
TMilburnPodPoint
TMilburnPodPoint reviewed Mar 26, 2024
View reviewed changes
modules/API/README.md Outdated Show resolved Hide resolved
modules/API/README.md Outdated Show resolved Hide resolved
modules/OCPP/OCPP.cpp Show resolved Hide resolved
modules/EvseManager/Charger.cpp Show resolved Hide resolved
@james-ctc james-ctc marked this pull request as ready for review April 23, 2024 12:05
@corneliusclaussen
Copy link
Contributor Author

@james-ctc Anything missing here still or are we good?

@james-ctc
Copy link
Contributor

All good to review and merge

@james-ctc james-ctc force-pushed the feature/enable_disable_source branch from a364314 to 8023cb6 Compare May 16, 2024 13:37
@james-ctc james-ctc mentioned this pull request May 16, 2024
Closed
3 tasks
@james-ctc james-ctc force-pushed the feature/enable_disable_source branch from 317b3e4 to 7c387e1 Compare May 17, 2024 08:41
@Pietfried Pietfried self-assigned this May 23, 2024
Pietfried
Pietfried requested changes May 23, 2024
View reviewed changes
interfaces/evse_manager.yaml Outdated Show resolved Hide resolved
modules/API/API.cpp Outdated Show resolved Hide resolved
modules/OCPP/README.md Outdated Show resolved Hide resolved
@Pietfried Pietfried force-pushed the feature/enable_disable_source branch from 7c387e1 to 070c72f Compare May 29, 2024 11:47
corneliusclaussen and others added 2 commits May 30, 2024 09:57
@corneliusclaussen @Pietfried
Add new enable_disable commands to EvseManager
12f58e8 
It replaces the old enable/disable commands and adds a source and
priority. See README of the API module for more details.

Compile time breaking change on the evse_manager interface.
All enable/disable calls need to be switched over to the new
enable_disable call, for everest-core this is done in this PR.

In some uses cases it is useful if e.g. a LocalAPI source disables
a connector that only the same local source can enable again.
Think of a service technician who has an app to disable the connector
locally. Then it should be prevented that it is re-enabled by the CSMS
until the service technician enables it again via the same app.
Using priorities this is quite flexible and a lot of use-cases can
be implemented without a change in EVerest code.

The API module also has a new enable_disable command. It also has
compatibility implementations for the old enable/disable commands.

Signed-off-by: Cornelius Claussen <[email protected]>

fix: merge issues
fix: updated README

Signed-off-by: James Chapman <[email protected]>
@james-ctc @Pietfried
fix: merge error + missing blank line
cff6f37 
Signed-off-by: James Chapman <[email protected]>
Pietfried and others added 3 commits May 30, 2024 09:57
@Pietfried Pietfried force-pushed the feature/enable_disable_source branch from b0b0f7b to dbc8527 Compare May 30, 2024 07:58
@Pietfried
Copy link
Contributor

@SebaLukas your review is also required for this one.

@Pietfried Pietfried requested a review from james-ctc May 30, 2024 09:03
mhei
mhei reviewed May 31, 2024
View reviewed changes
types/evse_manager.yaml
description: Priority of this entry. The highest priority is 0.
type: integer
minimum: 0
maximum: 10000
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason to use 10000 here? I'd stick to usual byte range boundaries here, e.g. 255 - should be far enough IMHO.

james-ctc
james-ctc approved these changes May 31, 2024
View reviewed changes
Copy link
Contributor

@james-ctc james-ctc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The changes look okay to me

SebaLukas
SebaLukas approved these changes Jun 3, 2024
View reviewed changes
Copy link
Contributor

@SebaLukas SebaLukas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@Pietfried Pietfried merged commit 571f3d4 into main Jun 3, 2024
4 of 5 checks passed
@Pietfried Pietfried deleted the feature/enable_disable_source branch June 3, 2024 11:27
mhei added a commit to mhei/remotechargeport that referenced this pull request Jun 5, 2024
@mhei
Adapt SatelliteAgent/SatelliteController to change evse_manager inter…
fa90713 
…face

EVerest release 2024.5.0 introduced a change in the evse_manager interface:
see EVerest/everest-core#505

So a small adaption is needed in both modules to cover the
new/replaced command(s).

Signed-off-by: Michael Heimpold <[email protected]>
mhei added a commit to mhei/remotechargeport that referenced this pull request Jun 10, 2024
@mhei
Adapt SatelliteAgent/SatelliteController to change evse_manager inter…
f1e00bc 
…face

EVerest release 2024.5.0 introduced a change in the evse_manager interface:
see EVerest/everest-core#505

So a small adaption is needed in both modules to cover the
new/replaced command(s).

Signed-off-by: Michael Heimpold <[email protected]>
@Pietfried Pietfried mentioned this pull request Jun 11, 2024
Closed
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@brett-swann brett-swann brett-swann left review comments

@TMilburnPodPoint TMilburnPodPoint TMilburnPodPoint left review comments

@mhei mhei mhei left review comments

@james-ctc james-ctc james-ctc approved these changes

@SebaLukas SebaLukas SebaLukas approved these changes

@Pietfried Pietfried Pietfried approved these changes

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

Assignees

@Pietfried Pietfried

Labels
post-release Tag that this PR should not go into the current merge window for the upcoming release
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@corneliusclaussen @james-ctc @Pietfried @mhei @SebaLukas @TMilburnPodPoint @brett-swann