-
Notifications
You must be signed in to change notification settings - Fork 326
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
Allow library users to send requests abstracting over specific request type #586
base: master
Are you sure you want to change the base?
Conversation
Thanks for your PR. And your use case is something we could support. I would suggest a slightly different implementation, though. Instead of allowing adding a method that accepts class ChargePoint:
async def send_call(self, call: Call, suppress=True, unique_id=None):
... Note that I don't like |
I had the same naming problem. I like So if the
Would it be OK to still allow callers to pass a Call object with unique_id set to None and let the library generate the unique ID in that case? In my use case I don't need control over what the unique ID is and it's enough to have requests and responses linked through async returns. I think that's why I went with separate action and payload first. The ugly thing about separate action and payload parameters to me is that the call and return become asymmetric: you call the method with separate parameters of simple types, but it will return you a CallResult object. The ugly thing about having one Call object parameter is that you'd have to set its unique_id field to None to let the library manage the unique_id. |
@OrangeTux I made some changes and I hope it's good to go like this. If you have more ideas about the name and interface for |
@OrangeTux @Jared-Newell-Mobility I would like to get this merged. Is there anything I can do to make it happen? |
I second the merge of that feature. I need it for OCPP 2 stack testing purpose. What is hindering code owners review and merge? |
HI @reinierl, the idea looks promising. Can you please update the PR with master and remove conflicts? |
altijd hetzelfde met die Nederlanders in de EV-oplaad-IT...
…on to generic_call
Done! |
Hmm, I couldn't quickly find a way to run the tests myself so I just went for it. I'll look into what's failing tomorrow probably. |
Sometimes you want to let other code decide which request is to be sent, and just pass stuff on into the OCPP WebSocket connection. In such cases having to different separate methods for the different actions creates a lot of boilerplate, especially with OCPP 2.0.1 which has many more possible actions.
This merge request adds a
generic_call
method that allows a library user to give a JSON payload and action name, without having to represent the payload as a special object or having to call a specific method for the action.Note that I'm experienced in OCPP and in coding in general but I'm a noob when it comes to Python. Comments on how to improve my code are very welcome.