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

network connection profiles #410

Closed
wants to merge 36 commits into from
Closed

network connection profiles #410

wants to merge 36 commits into from

Conversation

SNSubramanya
Copy link
Contributor

No description provided.

@SNSubramanya SNSubramanya linked an issue Jan 22, 2024 that may be closed by this pull request
Extend Network Connection Profiles #408
Closed
@SNSubramanya SNSubramanya force-pushed the 408-network-connection-profiles branch from 95a6a1a to a5d1c79 Compare January 29, 2024 09:52
@maaikez maaikez force-pushed the 408-network-connection-profiles branch from 1f7d92e to 944e2aa Compare February 19, 2024 16:23
AssemblyJohn
AssemblyJohn reviewed Feb 20, 2024
View reviewed changes
include/ocpp/common/websocket/websocket_base.hpp Outdated
@@ -47,7 +48,7 @@ class WebsocketBase {
WebsocketConnectionOptions connection_options;
std::function<void(const int security_profile)> connected_callback;
std::function<void()> disconnected_callback;
std::function<void(const websocketpp::close::status::value reason)> closed_callback;
std::function<void(const WebsocketCloseReason reason)> closed_callback;
std::function<void(const std::string& message)> message_callback;
websocketpp::lib::shared_ptr<boost::asio::steady_timer> reconnect_timer;
Copy link
Contributor

Choose a reason for hiding this comment

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

@hikinggrass maybe we should also abstract usage to the timer and use the evse timer instead?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes and there is more to be abstracted I think. I did not go through all the websocketpp things there, only the things I needed to be changed. I did not look into detail how websocketpp and the websocket in general works.

Pietfried
Pietfried reviewed Feb 20, 2024
View reviewed changes
include/ocpp/common/websocket/websocket.hpp
@@ -50,7 +54,7 @@ class Websocket {

/// \brief register a \p callback that is called when the websocket connection has been closed and will not attempt
/// to reconnect
void register_closed_callback(const std::function<void(const websocketpp::close::status::value reason)>& callback);
void register_closed_callback(const std::function<void(const WebsocketCloseReason reason)>& callback);
Copy link
Contributor

Choose a reason for hiding this comment

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

We can keep the closed_callback and disconnected_callback for now because they are implicitly used to manage the reconnect behavior, but to reduce complexity of the interface I would prefer to not have the seperated after this refactor is done

@maaikez maaikez force-pushed the 408-network-connection-profiles branch from 48f7846 to 86b7632 Compare March 19, 2024 15:59
@EVerest EVerest deleted a comment from SNSubramanya Mar 19, 2024
@maaikez maaikez force-pushed the 408-network-connection-profiles branch from 3eb08ca to 63ead24 Compare March 19, 2024 16:26
@maaikez
Copy link
Contributor

maaikez commented Mar 19, 2024

There are a lot of changes now. Tested this with websocket plain (not tls) and only ocpp 2.0.1. Is it possible to take a look again if this is the direction we want to go?

Maybe we want to move 'ping' to the connectivity manager as well.

@maaikez maaikez force-pushed the 408-network-connection-profiles branch 4 times, most recently from 84d3a3b to 2657cad Compare March 19, 2024 17:22
SNSubramanya and others added 10 commits March 27, 2024 14:52
@SNSubramanya @maaikez
[wip] add a websocked connected and disconnected callback
f5eae6b 
Signed-off-by: Soumya Subramanya <[email protected]>
Signed-off-by: Maaike <[email protected]>
@SNSubramanya @maaikez
[wip] experiment with promise and future
b94f03c 
Signed-off-by: Soumya Subramanya <[email protected]>

[wip] fixed bug in print statement

Signed-off-by: Soumya Subramanya <[email protected]>
Signed-off-by: Maaike <[email protected]>
@SNSubramanya @maaikez
make websocket callbacks optional
5f03834 
Signed-off-by: Soumya Subramanya <[email protected]>
Signed-off-by: Maaike <[email protected]>
@SNSubramanya @maaikez
[wip] Add a future return for the configure network connection callback
cc6d0a3 
Signed-off-by: Soumya Subramanya <[email protected]>
Signed-off-by: Maaike <[email protected]>
@SNSubramanya @maaikez
[wip] Add a variable to configure the timeout duration
9169440 
Signed-off-by: Soumya Subramanya <[email protected]>
Signed-off-by: Maaike <[email protected]>
@SNSubramanya @maaikez
[wip] Add a check for the timeout value and deferred status
0658697 
Signed-off-by: Soumya Subramanya <[email protected]>
Signed-off-by: Maaike <[email protected]>
@SNSubramanya @maaikez
[wip] Added an on_try_switch function to switch to a specific network…
aeb45d2 
… connection profile

Signed-off-by: Soumya Subramanya <[email protected]>
Signed-off-by: Maaike <[email protected]>
@SNSubramanya @maaikez
[wip] Add definition for on_try_switch_network_profile
5436951 
Signed-off-by: Soumya Subramanya <[email protected]>
Signed-off-by: Maaike <[email protected]>
@SNSubramanya @maaikez
[wip] some more PR changes
2a71918 
Signed-off-by: Soumya Subramanya <[email protected]>
Signed-off-by: Maaike <[email protected]>
@maaikez
websocket_(dis)connected_callback: Connected callback: NetworkConnect…
b6d22c8 
…ionProfile should not be an optional. Add configuration slot as parameter. Disconnected callback: also add configuration slot and NetworkConnectionProfile.

Signed-off-by: Maaike Zijderveld <[email protected]>
Signed-off-by: Maaike <[email protected]>
AssemblyJohn and others added 22 commits March 27, 2024 14:52
@AssemblyJohn @maaikez
Fixed compiling for new interface
9e81cb8 
Signed-off-by: AssemblyJohn <[email protected]>
Signed-off-by: Maaike <[email protected]>
@maaikez
Change configuration slot to be an int32_t instead of string. Add doc…
2eee3ed 
…umentation for network connectivity.

Signed-off-by: Maaike Zijderveld <[email protected]>
Signed-off-by: Maaike <[email protected]>
@maaikez
Implement a bit more of on_try_switch_network_connection_profile.
0dcf551 
Signed-off-by: Maaike Zijderveld <[email protected]>
Signed-off-by: Maaike <[email protected]>
@maaikez
Add connectivity manager.
677c2d0 
Signed-off-by: Maaike <[email protected]>
@maaikez
Add some callbacks in connectivity manager
39269f4 
Signed-off-by: Maaike <[email protected]>
@maaikez
Add main thread to connectivity manager.
6cc22b3 
Signed-off-by: Maaike <[email protected]>
@maaikez
Remove things from ChargePoint that are moved to the ConnectivityMana…
8a5353f 
…ger.

Signed-off-by: Maaike <[email protected]>
@maaikez
Change charge_point so it does not use websocket member anymore. Late…
6230d9f 
…r we will remove it completely.

Signed-off-by: Maaike <[email protected]>
@maaikez
On network disconnect: only disconnect if the network that should be …
5d5e7ef 
…disconnected is indeed connected. Init function: try next configuration priority if the current is not found. Remove some TODO's that are already implemented.

Signed-off-by: Maaike <[email protected]>
@maaikez
Fix bug where disconnected network slot was always disconnecting the …
1fa3d07 
…websocket.

Signed-off-by: Maaike <[email protected]>
@maaikez
Add logging for testing and try to fix some bugs.
28b242d 
Signed-off-by: Maaike <[email protected]>
@maaikez
Move reconnect timer to connectivity manager. formatting.
1dad026 
Signed-off-by: Maaike <[email protected]>
@maaikez
Add websocket failed callback. Add functionality in connectivity mana…
f47488e 
…ger to reconnect with backoff wait time.

Signed-off-by: Maaike <[email protected]>
@maaikez
Strip websocket classes. Add (a lot of!) logging for debugging purposes.
42fb35a 
Signed-off-by: Maaike <[email protected]>
@maaikez
Add documentation. Fix some crashes when websocket is reconnecting.
062367b 
Signed-off-by: Maaike <[email protected]>
@maaikez
Cleanup. Introduce new mutex to prevent pending, active and requested…
fe28ee8 
… network slots in connectivity manager.

Signed-off-by: Maaike <[email protected]>
@maaikez
Fix after rebase
65e4f34 
Signed-off-by: Maaike <[email protected]>
@maaikez
Removed include for websocket_plain and websocket_tls .hpp from webso…
e005b18 
…cket.hpp.

Signed-off-by: Maaike <[email protected]>
@maaikez
Formatting.
83383e8 
Signed-off-by: Maaike <[email protected]>
@maaikez
'Fix' some lint issues and clang format issues.
25c301a 
Signed-off-by: Maaike <[email protected]>
@maaikez
Add some removed code (oops). Move (duplicate) code from chargepoint …
10b5ff5 
…to connectivity manager.

Signed-off-by: Maaike <[email protected]>
@maaikez
Make libwebsockets also work with security profile 1 (so that the url…
9d2e723 
… is not set to wss). Extra check if websocket is already stopped. Extra check if a reconnection is already taking place (in connectivity manager).

Signed-off-by: Maaike <[email protected]>
@maaikez maaikez force-pushed the 408-network-connection-profiles branch from b3a2043 to 9d2e723 Compare March 27, 2024 13:53
marcemmers
marcemmers reviewed Apr 11, 2024
View reviewed changes
lib/ocpp/v201/connectivity_manager.cpp

/// \brief Default timeout for the return value (future) of the `configure_network_connection_profile_callback`
/// function.
constexpr int32_t default_network_config_timeout_seconds = 60;
Copy link
Contributor

Choose a reason for hiding this comment

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

These can (should) be chrono types

marcemmers
marcemmers reviewed Apr 11, 2024
View reviewed changes
lib/ocpp/v201/connectivity_manager.cpp
EVLOG_debug << "Init websocket without config slot, use first available: " << config_slot_int;
} else {
// No network connection profile. Retry connecting after some time, maybe it is set manually.
EVLOG_info << "No network connection profile set. Try again later.";
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we can try this more often then every 300 seconds but not print this message every time?

@WilcodenBesten WilcodenBesten mentioned this pull request Aug 7, 2024
Closed
@WilcodenBesten WilcodenBesten mentioned this pull request Aug 15, 2024
Closed
@maaikez maaikez mentioned this pull request Sep 5, 2024
Merged
2 tasks
@marcemmers marcemmers closed this Sep 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Pietfried Pietfried Pietfried left review comments

@marcemmers marcemmers marcemmers left review comments

@AssemblyJohn AssemblyJohn AssemblyJohn left review comments

@maaikez maaikez maaikez left review comments

@hikinggrass hikinggrass Awaiting requested review from hikinggrass hikinggrass will be requested when the pull request is marked ready for review hikinggrass is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Extend Network Connection Profiles
5 participants
@SNSubramanya @maaikez @AssemblyJohn @marcemmers @Pietfried