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.

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

Reservation changes for 2.0.1 #943

Open
wants to merge 63 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 34 commits
Commits
Show all changes
63 commits
Select commit Hold shift + click to select a range
1abe69e
Start implementation of reservation.
maaikez Sep 17, 2024
606af4f
Add todo's to support evse 0 and start making changes for that.
maaikez Sep 23, 2024
dc3b0cf
Merge branch 'main' into feature/327-use-case-h01-reservation
maaikez Sep 24, 2024
c8f44cc
Merge branch 'main' into feature/327-use-case-h01-reservation
maaikez Sep 25, 2024
c40fe98
Continue working on reservations.
maaikez Sep 25, 2024
228cdd0
Some changes in the reservation interface to allow optional values an…
maaikez Sep 26, 2024
63ca3de
Add connector type when initializing connector in auth module. Fix au…
maaikez Oct 9, 2024
b56eca9
Try to make an algorithm for reservations with no evse but a specific…
maaikez Oct 10, 2024
bc9b476
Reservations algorithm for multiple cars, no evse id and a given conn…
maaikez Oct 11, 2024
3b1170f
Tests now work (but I don't understand completely why yet).
maaikez Oct 12, 2024
81f407d
The algorithm for reservations now seems to work and all tests pass.
maaikez Oct 14, 2024
2abd635
Add possibility to add reservation for specific evse id.
maaikez Oct 15, 2024
35a306f
Check if charging is possible on a specific evse id. Add more tests.
maaikez Oct 15, 2024
84f7208
Cancel reservation. Return the reason why a reservation did not succe…
maaikez Oct 16, 2024
b46b503
Set evse unavailable / faulted and cancel reservations.
maaikez Oct 17, 2024
8172cd0
Test cancel reservation. Test cancelling because of expired reservati…
maaikez Oct 18, 2024
fd13f08
Add some new tests.
maaikez Oct 21, 2024
bfe4ee5
Add some tests and fix bugs found with the tests.
maaikez Oct 23, 2024
f4eaf0d
Refactor AuthHandler so it can handle multiple connectors per evse.
maaikez Oct 24, 2024
3437975
Add documentation to ReservationHandler and reservation tests. Set ev…
maaikez Oct 25, 2024
4e6d076
Send reservation update when a reservation is cancelled. Fix some bug…
maaikez Oct 28, 2024
7d03a7a
Merge branch 'main' into feature/327-use-case-h01-reservation
maaikez Oct 29, 2024
ef6abde
Add key value store to reservation handler. Fix things with evse id /…
maaikez Oct 30, 2024
f4d10a8
Load and store reservations from the kvs. Create stub for kvs interfa…
maaikez Oct 31, 2024
315b62a
Fix / remove todo's. Add debug logging. Remove unnecessary logging.
maaikez Oct 31, 2024
4a0ac28
Change libocpp dependency to get the correct version for the reservat…
maaikez Oct 31, 2024
c1994aa
Merge branch 'main' into feature/327-use-case-h01-reservation
maaikez Oct 31, 2024
c2d2b65
Fix some small issues after self reviewing the code.
maaikez Oct 31, 2024
843fae4
Add connector type to config sil ocpp201 yaml.
maaikez Oct 31, 2024
501d73a
Formatting
maaikez Nov 1, 2024
08a38ee
Typo
maaikez Nov 1, 2024
5543e0a
Formatting.
maaikez Nov 1, 2024
f1e923f
Fix bug with occupied / available while making reservations.
maaikez Nov 4, 2024
f2fcace
Remove logging or change to debug logging.
maaikez Nov 4, 2024
c4f8756
Add logging.
maaikez Nov 4, 2024
7c9d33a
Review comments.
maaikez Nov 5, 2024
5c7dbac
Make it build again.
maaikez Nov 6, 2024
132ca86
Check if there are global reservations that prevent charging and crea…
maaikez Nov 6, 2024
0847d20
Review comments. Add map with evse's to constructor of Reservation Ha…
maaikez Nov 8, 2024
295a7ec
Review comments and codacy warnings. Change libocpp reserve now callb…
maaikez Nov 8, 2024
46695d9
Change function name 'init_connector' to 'init_evse'.
maaikez Nov 11, 2024
ce3781b
Add mutex around shared evse map in authhandler and reservation handler.
maaikez Nov 11, 2024
2d61ce9
Merge branch 'main' into feature/327-use-case-h01-reservation
maaikez Nov 11, 2024
cf54aaa
Formatting.
maaikez Nov 11, 2024
78eacfb
Remove todo's (tickets are made if necessary).
maaikez Nov 11, 2024
cc6c6a9
Types in config must be primitive
maaikez Nov 11, 2024
8ab8859
Add missing bazel dependency to interfaces lib
hikinggrass Nov 11, 2024
3cc70d4
Merge branch 'main' into feature/327-use-case-h01-reservation
maaikez Nov 12, 2024
6df07ce
Review comments.
maaikez Nov 12, 2024
df45d6c
Review comments
maaikez Nov 12, 2024
42d5461
Formatting.
maaikez Nov 12, 2024
c671e80
Merge branch 'main' into feature/327-use-case-h01-reservation
maaikez Nov 12, 2024
925e08f
Fix bug after test.
maaikez Nov 12, 2024
f205906
Merge branch 'main' into feature/327-use-case-h01-reservation
maaikez Nov 13, 2024
5f4f0e6
Fix some bugs found with OCTT testing. Do not send 'accepted' when ma…
maaikez Nov 13, 2024
9ae31b5
Set evse manager connector type default to unknown
maaikez Nov 13, 2024
53aa3a5
Fix bug where reservation was not removed from evse manager when plug…
maaikez Nov 13, 2024
b1e1040
Add reservation to config 201 pnc.
maaikez Nov 13, 2024
e6d2374
Merge branch 'main' into feature/327-use-case-h01-reservation
Pietfried Nov 14, 2024
ad9d05a
Reservation is now optional for OCPP201 module. Review comments.
maaikez Nov 14, 2024
3b19bd0
Merge branch 'main' into feature/327-use-case-h01-reservation
maaikez Nov 14, 2024
08282f2
Reservation Update can also not been sent, which is the case when OCP…
maaikez Nov 14, 2024
cf690d0
Add comment
maaikez Nov 14, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions config/config-sil-ocpp.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ active_modules:
config_module:
connector_id: 1
evse_id: "1"
connector_type: "cCCS1"
Copy link
Contributor

Choose a reason for hiding this comment

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

The default charge_mode value is AC, so type2 would be more appropriate

session_logging: true
session_logging_xml: false
session_logging_path: /tmp/everest-logs
Expand Down Expand Up @@ -51,6 +52,7 @@ active_modules:
config_module:
connector_id: 2
evse_id: "2"
connector_type: "cCCS1"
session_logging: true
session_logging_xml: false
session_logging_path: /tmp
Expand Down
8 changes: 8 additions & 0 deletions config/config-sil-ocpp201.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ active_modules:
config_module:
connector_id: 1
evse_id: "1"
connector_type: "cCCS1"
session_logging: true
session_logging_xml: false
session_logging_path: /tmp
Expand All @@ -45,6 +46,7 @@ active_modules:
config_module:
connector_id: 2
evse_id: "2"
connector_type: "cCCS1"
session_logging: true
session_logging_xml: false
session_logging_path: /tmp
Expand Down Expand Up @@ -130,6 +132,9 @@ active_modules:
connector_zero_sink:
- module_id: grid_connection_point
implementation_id: external_limits
reservation:
- module_id: auth
implementation_id: reservation
persistent_store:
module: PersistentStore
evse_security:
Expand Down Expand Up @@ -157,6 +162,9 @@ active_modules:
implementation_id: evse
- module_id: evse_manager_2
implementation_id: evse
kvs:
- module_id: persistent_store
implementation_id: main
energy_manager:
module: EnergyManager
connections:
Expand Down
2 changes: 1 addition & 1 deletion dependencies.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ libevse-security:
# OCPP
libocpp:
git: https://github.com/EVerest/libocpp.git
git_tag: 6a0010225535599b053e401307d76b58e9e3c7c5
git_tag: feature/327-use-case-h01-reservation
cmake_condition: "EVEREST_DEPENDENCY_ENABLED_LIBOCPP"
# Josev
Josev:
Expand Down
37 changes: 25 additions & 12 deletions interfaces/reservation.yaml
Original file line number Diff line number Diff line change
@@ -1,21 +1,15 @@
description: Interface for reservations
cmds:
reserve_now:
description: Reserves this evse.
description: Reserves an evse.
arguments:
connector_id:
description: >-
The id of the connector to be reserved. A value of 0 means that
the reservation is not for a specific connector
type: integer
reservation:
description: The information about the Reservation to be placed
request:
type: object
$ref: /reservation#/Reservation
$ref: /reservation#/ReserveNowRequest
description: Requests to make a reservation
result:
description: >-
Returns Accepted if reservation was succesfull or specifies error
code.
Returns Accepted if reservation was succesful or specifies error code.
type: string
$ref: /reservation#/ReservationResult
cancel_reservation:
Expand All @@ -29,4 +23,23 @@ cmds:
Returns true if reservation was cancelled. Returns false if there
was no reservation to cancel.
type: boolean
vars: {}
is_reservation_for_token:
Copy link
Contributor

Choose a reason for hiding this comment

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

I would prefer something like exists_reservation here

description: >-
Checks if there is a reservation made for the given connector and token. Will also return true if there
is a reservation with this token for evse id 0.
arguments:
request:
type: object
$ref: /reservation#/ReservationCheck
description: Check if there is a reservation on the given connector for the given token.
result:
description: >-
Returns true if there is a reservation for the given connector id and token or group id token. If there is a
connector id given, it will also return true if a reservation with this token was made for connector id 0.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
connector id given, it will also return true if a reservation with this token was made for connector id 0.
connector id given, it will also return true if a reservation with this token was made for evse id 0.
``` ?

type: boolean
vars:
reservation_update:
description: >-
Update of the reservation.
type: object
$ref: /reservation#/ReservationUpdateStatus
56 changes: 43 additions & 13 deletions modules/Auth/Auth.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@ void Auth::init() {

this->auth_handler = std::make_unique<AuthHandler>(
string_to_selection_algorithm(this->config.selection_algorithm), this->config.connection_timeout,
this->config.prioritize_authorization_over_stopping_transaction, this->config.ignore_connector_faults);
this->config.prioritize_authorization_over_stopping_transaction, this->config.ignore_connector_faults,
this->info.id, (!this->r_kvs.empty() ? this->r_kvs.at(0).get() : nullptr));

for (const auto& token_provider : this->r_token_provider) {
token_provider->subscribe_provided_token([this](ProvidedIdToken provided_token) {
Expand All @@ -30,20 +31,24 @@ void Auth::ready() {

int32_t evse_index = 0;
for (const auto& evse_manager : this->r_evse_manager) {
int32_t connector_id = evse_manager->call_get_evse().id;
this->auth_handler->init_connector(connector_id, evse_index);
const int32_t evse_id = evse_manager->call_get_evse().id;

evse_manager->subscribe_session_event([this, connector_id](SessionEvent session_event) {
this->auth_handler->handle_session_event(connector_id, session_event);
for (const auto& connector : evse_manager->call_get_evse().connectors) {
this->auth_handler->init_connector(
connector.id, evse_index, connector.type.value_or(types::evse_manager::ConnectorTypeEnum::Unknown));
}

evse_manager->subscribe_session_event([this, evse_id](SessionEvent session_event) {
this->auth_handler->handle_session_event(evse_id, session_event);
});

evse_manager->subscribe_error(
"evse_manager/Inoperative",
[this, connector_id](const Everest::error::Error& error) {
this->auth_handler->handle_permanent_fault_raised(connector_id);
[this, evse_id](const Everest::error::Error& error) {
this->auth_handler->handle_permanent_fault_raised(evse_id);
},
[this, connector_id](const Everest::error::Error& error) {
this->auth_handler->handle_permanent_fault_cleared(connector_id);
[this, evse_id](const Everest::error::Error& error) {
this->auth_handler->handle_permanent_fault_cleared(evse_id);
});

evse_index++;
Expand All @@ -53,6 +58,7 @@ void Auth::ready() {
[this](const ProvidedIdToken& token, TokenValidationStatus status) {
this->p_main->publish_token_validation_status({token, status});
});

this->auth_handler->register_notify_evse_callback(
[this](const int evse_index, const ProvidedIdToken& provided_token, const ValidationResult& validation_result) {
this->r_evse_manager.at(evse_index)->call_authorize_response(provided_token, validation_result);
Expand All @@ -70,11 +76,35 @@ void Auth::ready() {
[this](const int32_t evse_index, const StopTransactionRequest& request) {
this->r_evse_manager.at(evse_index)->call_stop_transaction(request);
});
this->auth_handler->register_reserved_callback([this](const int32_t evse_index, const int32_t& reservation_id) {
this->r_evse_manager.at(evse_index)->call_reserve(reservation_id);
});
this->auth_handler->register_reserved_callback(
[this](const std::optional<int32_t> evse_id, const int32_t& reservation_id) {
// Only call the evse manager to store the reservation if it is done for a specific evse.
if (evse_id.has_value()) {
EVLOG_info << "Call reserved callback for evse id " << evse_id.value();

this->r_evse_manager.at(evse_id.value() - 1)->call_reserve(reservation_id);
Copy link
Contributor

Choose a reason for hiding this comment

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

call_reserve returns a bool, we should check for the result. Im not sure, but maybe this also allows to move a little bit of the business logic to check whether a reservation can be placed (e.g. in case of faulted, inoperative) to the EvseManager. If I look at handle_reserve / reserve a few checks are done there

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm yes that's a good one. Since the ReservationHandler has a copy of this states, it should just always be possible. But ok, if the evse manager returns false here, should we cancel the reservation in that case then? Because I think at this point the reservation has already been made.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I just left a todo there for now.

Copy link
Contributor

Choose a reason for hiding this comment

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

The EvseManager shall stay autonoumus with the decision if a reservation can be made, so we should be able to handle the response at some point

Copy link
Contributor Author

@maaikez maaikez Nov 12, 2024

Choose a reason for hiding this comment

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

So should the flow be:

  • check in the reservation handler if the reservation can be made, but do not make it yet
  • if it can be made, call 'reserve' on the evse manager
  • if evse manager returns 'true', we make the reservation in the reservation handler

?

Is that all going well when a new car just arrives in the meantime?

Or maybe we just have to create the reservation and if the evse manager returns false, cancel it again? Because we have kind of a strange situation if the reservation is not yet made, the evse manager returns true on the 'reserve' call, we make the reservation but it is not possible anymore (because a car just arrived).

BTW, the evse manager also has nothing to say about reservations that are not tight to a specific evse. So who is responsible for the reservations here then?

}
});
this->auth_handler->register_reservation_cancelled_callback(
[this](const int32_t evse_index) { this->r_evse_manager.at(evse_index)->call_cancel_reservation(); });
[this](const std::optional<int32_t> evse_id, const int32_t reservation_id, const ReservationEndReason reason) {
// Only call the evse manager to cancel the reservation if it was for a specific evse
if (evse_id.has_value()) {
EVLOG_debug << "Call evse manager to cancel the reservation with evse id " << evse_id.value();
this->r_evse_manager.at(evse_id.value() - 1)->call_cancel_reservation();
}

ReservationUpdateStatus status;
status.reservation_id = reservation_id;
if (reason == ReservationEndReason::Expired) {
status.reservation_status = Reservation_status::Expired;
} else if (reason == ReservationEndReason::Cancelled) {
status.reservation_status = Reservation_status::Removed;
} else {
// On reservation used: do not publish a reservation update!!
return;
}
this->p_reservation->publish_reservation_update(status);
});
}

void Auth::set_connection_timeout(int& connection_timeout) {
Expand Down
9 changes: 7 additions & 2 deletions modules/Auth/Auth.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
#include <generated/interfaces/auth_token_provider/Interface.hpp>
#include <generated/interfaces/auth_token_validator/Interface.hpp>
#include <generated/interfaces/evse_manager/Interface.hpp>
#include <generated/interfaces/kvs/Interface.hpp>

// ev@4bf81b14-a215-475c-a1d3-0a484ae48918:v1
// insert your custom include headers here
Expand Down Expand Up @@ -45,20 +46,24 @@ class Auth : public Everest::ModuleBase {
std::unique_ptr<reservationImplBase> p_reservation,
std::vector<std::unique_ptr<auth_token_providerIntf>> r_token_provider,
std::vector<std::unique_ptr<auth_token_validatorIntf>> r_token_validator,
std::vector<std::unique_ptr<evse_managerIntf>> r_evse_manager, Conf& config) :
std::vector<std::unique_ptr<evse_managerIntf>> r_evse_manager, std::vector<std::unique_ptr<kvsIntf>> r_kvs,
Conf& config) :
ModuleBase(info),
p_main(std::move(p_main)),
p_reservation(std::move(p_reservation)),
r_token_provider(std::move(r_token_provider)),
r_token_validator(std::move(r_token_validator)),
r_evse_manager(std::move(r_evse_manager)),
config(config){};
r_kvs(std::move(r_kvs)),
config(config) {
}

const std::unique_ptr<authImplBase> p_main;
const std::unique_ptr<reservationImplBase> p_reservation;
const std::vector<std::unique_ptr<auth_token_providerIntf>> r_token_provider;
const std::vector<std::unique_ptr<auth_token_validatorIntf>> r_token_validator;
const std::vector<std::unique_ptr<evse_managerIntf>> r_evse_manager;
const std::vector<std::unique_ptr<kvsIntf>> r_kvs;
const Conf& config;

// ev@1fce4c5e-0ab8-41bb-90f7-14277703d2ac:v1
Expand Down
1 change: 1 addition & 0 deletions modules/Auth/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ target_link_libraries(${MODULE_NAME}
date::date
date::date-tz
everest::timer
nlohmann_json::nlohmann_json
)
# ev@bcc62523-e22b-41d7-ba2f-825b493a3c97:v1

Expand Down
Loading
Loading