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

Raise exception if response from call() is CallError #124

Merged
merged 7 commits into from
Oct 20, 2020

Conversation

tmh-azinhal
Copy link
Contributor

@tmh-azinhal tmh-azinhal commented Oct 7, 2020

This PR references issue #104 .

If the response is a CallError, an exception will be raised instead of being silenced.
To maintain backwards compatibility a suppress=True keyword argument was added in Chargepoint.call.

Copy link
Contributor

@OrangeTux OrangeTux left a comment

Choose a reason for hiding this comment

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

You can add a test? I'll come back later to

Copy link
Collaborator

@tropxy tropxy left a comment

Choose a reason for hiding this comment

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

A test for this is missing, then it is good to go!

@OrangeTux
Copy link
Contributor

I promised some advice when it comes to writing tests for the piece of code you introduced.

In [tests/v16/test_v16_charge_point.py] you can add the tests. For the test you can reuse the base_central_system as is done in other tests in that file.

This test require a call, that is the request, and a call error. The latter is the response. Because of limitations of testing you first need prepare the response. Create an instance of ocpp.messages.CallError, turn it into json and pass it to base_central_system.route_message(). route_message() will put the response on an internal response queue.

Now you can create the request. You need to instantiate a payload from call.v16.call and 'send' that to the other end of the connection using base_central_system.call(). call() will use the response queue to look for response to its request.

Note that it's important that the request and response have the same unique id. Otherwise a response can't be matched to a request.

I hope this is enough information. If not, let me know.

Copy link
Contributor

@OrangeTux OrangeTux left a comment

Choose a reason for hiding this comment

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

The lint job on CircleCI compains. Can you make the job happy?

@tmh-azinhal tmh-azinhal requested a review from tropxy October 20, 2020 11:21
Copy link
Contributor

@OrangeTux OrangeTux left a comment

Choose a reason for hiding this comment

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

Thanks!

@tmh-azinhal tmh-azinhal merged commit 1d86f22 into master Oct 20, 2020
@tropxy tropxy linked an issue Oct 21, 2020 that may be closed by this pull request
@jainmohit2001 jainmohit2001 deleted the 104-handle-callerrors branch October 1, 2024 19:56
ajmirsky pushed a commit to ajmirsky/ocpp that referenced this pull request Nov 26, 2024
* If response is a CallError, an exception will be raised instead of being silenced.
To maintain backwards compatibility a suppress=True keyword argument was added in ChargePoint.call.

* Raise exception if response from call() is CallError

* Add tests for handling CallError

Co-authored-by: Auke Willem Oosterhoff <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Implement way to allow users to handle CallErrors
3 participants