Java-OCA-OCPP icon indicating copy to clipboard operation
Java-OCA-OCPP copied to clipboard

Propagate code and reason from disconnect from server on client

Open emilm opened this issue 3 years ago • 9 comments

The reason for this change is to have different reconnect strategies depending on why you are disconnected. If the OCPP rejected you because of a 404 or something like that, or if you simply lose connection.

My question here is what do do with explicit disconnects from within the code? Right now I pass 0, "disconnect() method called" - but it looks a bit hacky. 2 other alternatives are:

a separate disconnect method without arguments - but then the implementation must handle 2 disconnect() methods. nullable Integer so no error code is passed instead of a magic 0 or -1 or something

emilm avatar Sep 21 '22 19:09 emilm

Yes it feels better to have separate methods. The ocpp server in question gives a 404 but the code is 1002. this is when the charge point does not exist on the server . it's before websocket is even established i think.

It's necessary to have code and reason to have different behaviors based on the disconnection reason . i believe a physical disconnect will give another code etc.

emilm avatar Sep 21 '22 19:09 emilm

I think it's a good idea to only have one disconnected method and more explicit information is a good idea, since it allows implementers to act on the information.

I added a separate commit showing how it would look like with separate method for disconnect without params, but I forced pushed the first commit again after reading your post again! So now it is in it's original state.

emilm avatar Sep 21 '22 22:09 emilm

We should note that this change has breaking changes to some external APIs, namely:

  • ClientEvents.java
  • SessionEvents.java

I don't know how to avoid it. But since it would trigger a major version change on the API, I would either create version 2 branch or welcome any ideas on how to avoid it.

TVolden avatar Sep 22 '22 07:09 TVolden

Would it be better with the commit I did with keeping the parameterless version + add with code / reason? Would it still be considered a breaking change? https://github.com/ChargeTimeEU/Java-OCA-OCPP/commit/0de3f8ac2687c6d191eefefd1cbcbbfaf609f83e

emilm avatar Sep 22 '22 08:09 emilm

Would it be better with the commit I did with keeping the parameterless version + add with code / reason? Would it still be considered a breaking change? 0de3f8a

The fact that they are interfaces will cause the breaking change as long as a new method is added, an existing one is removed or modified. As far as I know, there isn't alot we can do to avoid it

TVolden avatar Sep 22 '22 09:09 TVolden

So what's the verdict on this @TVolden ? :)

emilm avatar Feb 07 '23 18:02 emilm

Hi @emilm, I seem to have created a v2 branch for these changes, you just need to change the PR to target that branch.

TVolden avatar Feb 08 '23 07:02 TVolden

First off, thank you TVolden for making this library. I’m just wondering what’s the status of this PR? These changes would be helpful.


Best regards,

conrad-uraga-power-x avatar Jun 02 '23 02:06 conrad-uraga-power-x

Hi @conrad-uraga-power-x

The PR should target the v2 branch instead of master. That's it, really 🙂

Sincerely Thomas Volden

TVolden avatar Jun 02 '23 11:06 TVolden