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

Asynchronous execution #215

Merged
merged 4 commits into from
Sep 30, 2022
Merged

Conversation

dimaa6
Copy link

@dimaa6 dimaa6 commented Sep 26, 2022

This pull request resolves #172 . It introduces ability to skip executing request by server or client immediately, and allows to complete it at later time by invoking new methods of IClient and IServer interfaces:

IClient:

boolean asyncCompleteRequest(String uniqueId, Confirmation confirmation) throws UnsupportedFeatureException, OccurenceConstraintException;
IServer:

boolean asyncCompleteRequest(UUID sessionIndex, String uniqueId, Confirmation confirmation)
      throws NotConnectedException, UnsupportedFeatureException, OccurenceConstraintException;

Changes are not breaking, existing code will continue to work without any need for modification. Usage of new methods and asynchronous mode are fully optional. The following diagram provides an example of using the asynchronous mode:

async_diagram

It also resolves #86 by allowing client to call asyncCompleteRequest in handleTriggerMessageRequest before sending a message requested by TriggerMessageRequest, and finally return null from handleTriggerMessageRequest.

One serious change is that in order to know OCPP message ID of the request in its handler, it has been included into every class which implemented Request interface. I enhanced Request interface with getOcppMessageId and setOcppMessageId methods, and implemented them in abstract class RequestWithId which implements Request. Then every class which formerly implemented Request interface now extends RequestWithId. Now handler implementation can simple call getOcppMessageId on the instance of Request it got. I have taken measures to not include ocppMessageId into serialized message, by introducing Exclude annotation.

@dimaa6 dimaa6 marked this pull request as ready for review September 26, 2022 11:21
@TVolden
Copy link
Member

TVolden commented Sep 27, 2022

Hi @dimaa6,

First of all, thank you for your contribution.
I'll try to review the code this evening and get back to you.

Sincerely,
Thomas Volden

Copy link
Member

@TVolden TVolden left a comment

Choose a reason for hiding this comment

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

Looks fine

}

@Override
public boolean completePendingPromise(String id, Confirmation confirmation) throws UnsupportedFeatureException, OccurenceConstraintException {
Copy link
Member

Choose a reason for hiding this comment

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

The method only returns true or throws an exception. Should probably just make it void. This would also conform to the CQS principle.

Copy link
Author

Choose a reason for hiding this comment

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

It may return false if it did not find pending promise:

synchronized (pendingPromises) {
promiseAction = pendingPromises.get(id);
if (promiseAction == null) return false;
// remove promise from store
pendingPromises.remove(id);
}

@TVolden TVolden merged commit 9fe3b79 into ChargeTimeEU:master Sep 30, 2022
@TVolden
Copy link
Member

TVolden commented Sep 30, 2022

Hi @dimaa6,

I have merged it into the main. Thank you for the contribution, it really means a lot to me.

Sincerely,
Thomas Volden

@mf-fujimori
Copy link

Hi @dimaa6
May I ask a question?

I believe that when the server sends a message, a uniqueId is assigned to the request.

The server needs the uniqueId when it calls "asyncCompleteRequest".

Where is "setOcppMessageId" called when the message is sent?

@dimaa6
Copy link
Author

dimaa6 commented Nov 17, 2023

Hi @dimaa6 May I ask a question?

I believe that when the server sends a message, a uniqueId is assigned to the request.

The server needs the uniqueId when it calls "asyncCompleteRequest".

Where is "setOcppMessageId" called when the message is sent?

You mean this line?

https://github.com/ChargeTimeEU/Java-OCA-OCPP/blob/master/ocpp-common/src/main/java/eu/chargetime/ocpp/Session.java#L218

@mf-fujimori
Copy link

Thank you for your response.

https://github.com/ChargeTimeEU/Java-OCA-OCPP/blob/master/ocpp-common/src/main/java/eu/chargetime/ocpp/Session.java#L218

I believe this is the process upon receipt.

An ID is generated when a message is sent.
I would like to know where this is the ID that is set in Request.
https://github.com/ChargeTimeEU/Java-OCA-OCPP/blob/master/ocpp-common/src/main/java/eu/chargetime/ocpp/Server.java#L219

@dimaa6
Copy link
Author

dimaa6 commented Nov 17, 2023

Thank you for your response.

https://github.com/ChargeTimeEU/Java-OCA-OCPP/blob/master/ocpp-common/src/main/java/eu/chargetime/ocpp/Session.java#L218

I believe this is the process upon receipt.

An ID is generated when a message is sent. I would like to know where this is the ID that is set in Request. https://github.com/ChargeTimeEU/Java-OCA-OCPP/blob/master/ocpp-common/src/main/java/eu/chargetime/ocpp/Server.java#L219

Yes, ID is generated when the request is sent, you have identified the line correctly. But I still don't understand the question. Let me try to provide the information I have. Session still handles the received message, code on server and client side is the same. And setOcppMessageId is called on receiving side, not the sending side. Then client code which handles the message will have the ID and will be able to call asyncCompleteRequest. Does this answer your question?

@mf-fujimori
Copy link

I am sorry that my question is not clear.
What I am asking is if it is possible to use "asyncCompleteRequest" to send a message from the server and cancel it if no response is returned.
I want to remove it because the library will continue to have Promise if no response is returned.

@dimaa6
Copy link
Author

dimaa6 commented Nov 28, 2023

I am sorry that my question is not clear. What I am asking is if it is possible to use "asyncCompleteRequest" to send a message from the server and cancel it if no response is returned. I want to remove it because the library will continue to have Promise if no response is returned.

You may create a pull request with new method which will remove promise by ID, similarly to asyncCompleteRequest but without actually completing it.

@matssom
Copy link

matssom commented Dec 18, 2023

I might be understanding wrong, but in the example bellow (from OCPP 1.6 example) it seems like returning null means unsupported feature, not "I'll handle response later". Is this an outdated example or am I misunderstanding?

@Override
public RemoteStartTransactionConfirmation handleRemoteStartTransactionRequest(RemoteStartTransactionRequest request) {

    System.out.println(request);
    // ... handle event

    return null; // returning null means unsupported feature
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants