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

Altered auto-reconnection attempt logic to comply with part 4 of the OCPP2.0.1 standard #221

Merged

Conversation

Jonesywolf
Copy link
Contributor

According to part 4 of the OCPP2.0.1 standard: JSON over WebSockets implementation guide, section 5.3. Reconnecting, there are some configuration variables, specifically:

  • RetryBackOffRepeatTimes
  • RetryBackOffRandomRange
  • RetryBackOffWaitMinimum

which dictate auto reconnection behaviour of the charge point. I have done my best to implement these changes and update the corresponding network test accordingly.

While the websocket code is shared between both OCPP versions, since OCPP1.6J (to my understanding) doesn't specify auto reconnection behaviour, it shouldn't be incompatible.

@Jonesywolf
Copy link
Contributor Author

This is not quite ready yet, will reopen when it's more fully tested.

@Jonesywolf Jonesywolf closed this Sep 12, 2023
@Jonesywolf
Copy link
Contributor Author

I got toxiproxy working, this code should at least pass the pipeline and worked in our implementation.

@Jonesywolf Jonesywolf reopened this Sep 12, 2023
ws/websocket.go Outdated
@@ -973,7 +985,8 @@ func (client *Client) cleanup() {

func (client *Client) handleReconnection() {
log.Info("started automatic reconnection handler")
delay := client.timeoutConfig.ReconnectBackoff
delay := client.timeoutConfig.RetryBackOffWaitMinimum + time.Duration(rand.Intn(client.timeoutConfig.RetryBackOffRandomRange+1))
Copy link
Owner

Choose a reason for hiding this comment

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

First of all thanks a lot for the contribution. I agree with you that the incremental backoff with range can be used for v1.6 as well, since nothing really speaks against it.

Regarding the implementation, I see the following issues:

  1. the back-off random range is being added only once
  2. the range is being added as nanoseconds, which doesn't really add much randomness -> I presume this should be cast to seconds

Design suggestion

Just to quote the specs:

When the Charging Station is reconnecting, after a connection loss, it will use this variable as the maximum value for the random part of the back-off time. It will add a new random value to every increasing back-off time, including the first connection attempt (with this maximum), for the amount of times it will double the previous back-off time. When the maximum number of increments is reached, the Charging Station will keep connecting with the same back-off time.

My interpretation is that for every iteration of the reconnect loop, the new delay should be:

delay *= 2
delay += time.Duration(rand.Intn(client.timeoutConfig.RetryBackOffRandomRange+1)) * time.Seconds

Does this make sense or did you reach a different interpretation?

@Jonesywolf
Copy link
Contributor Author

Jonesywolf commented Sep 21, 2023

Hi Lorenzo, I'm happy to contribute, thank you for making such a fantastic open source library. As far as both your comments go, you're totally right, they were both oversights on my part, I'm glad you caught them. Both should be resolved by commit c5ef78b

Please let me know if you have any more suggestions or find anything else I missed.

@lorenzodonini lorenzodonini merged commit 784d79e into lorenzodonini:master Sep 22, 2023
1 check passed
@lorenzodonini
Copy link
Owner

LG2M, merged 👍

@Jonesywolf Jonesywolf deleted the ocpp2-auto-reconnection branch September 22, 2023 13:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants