-
Notifications
You must be signed in to change notification settings - Fork 50
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
Defer websocket connect() #889
Conversation
Signed-off-by: Marc Emmers <[email protected]>
…ion for OCPP1.6 ; Removed WEBSOCKET_INIT_DELAY on startup Signed-off-by: Piet Gömpel <[email protected]>
@@ -135,7 +135,7 @@ void ConnectivityManager::connect(std::optional<int32_t> configuration_slot_opt) | |||
// After the websocket gets closed a reconnect will be triggered | |||
this->websocket->disconnect(WebsocketCloseReason::ServiceRestart); | |||
} else { | |||
this->try_connect_websocket(); | |||
this->websocket_timer.timeout([this] { this->try_connect_websocket(); }, std::chrono::seconds(0)); |
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.
@marcemmers I used std::chrono::seconds(0)
instead of WEBSOCKET_INIT_DELAY
because the WEBSOCKET_INIT_DELAY
was only introduced to solve an issue with OCTT tests reconnects as far as I remember.
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.
Yes I don't think there would be a point delaying there. Although I think we should probably revisit misusing a timer for this at all, that would be out of scope for this.
* Introduced thread_mutex protecting access to threads of libwebsockets when the ~WebsocketLibwebsockets destructor was called it attempted to acquire a lock for the connection_mutex and joins the websocket_thread . But the websocket_thread runs the client_loop that also requires the lock when calling on_conn_fail, so the client_loop which could result in a deadlock Signed-off-by: Piet Gömpel <[email protected]>
@marcemmers please have another looks at the recent changes |
Hi @marcemmers , we are looking into it and considering some more changes to the |
Closed in favor of #896 |
Describe your changes
The
start()
function of the charge_point could be blocking until a connection timeout occured when connection the websocket for the first time. This change defers the call to the connect() function so that the start() function does not block anymore.Additionally introduced thread_mutex protecting access to threads of libwebsockets when the ~WebsocketLibwebsockets destructor was called it attempted to acquire a lock for the connection_mutex and joins the websocket_thread . But the websocket_thread runs the client_loop that also requires the lock when calling on_conn_fail, so the client_loop which could result in a deadlock
Issue ticket number and link
Checklist before requesting a review