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

A routing method decorated with another decorator on top of @on or @after, will not be included in the routables #156

Open
tropxy opened this issue Nov 14, 2020 · 1 comment
Assignees
Labels
bug Something isn't working

Comments

@tropxy
Copy link
Collaborator

tropxy commented Nov 14, 2020

OCPP Version: 0.8.1
Python Version: 3.X

If we have an handler for a BootNotification like this:

@on(Action.BootNotification)
def on_boot_notitication(self, charge_point_vendor: str, charge_point_model: str, **kwargs):
    return call_result.BootNotificationPayload(
        current_time=datetime.utcnow().isoformat(),
        interval=10,
        status=RegistrationStatus.accepted
    )

Things work as expected. But if we have a simple decorator like this one:

def say_hello(f):
    def inner(*args, **kwargs):
        print("hello")
        return f(*args, **kwargs)
    return inner

and we apply it to the BootNotification handler:

@on(Action.BootNotification)
@say_hello
def on_boot_notitication(self, charge_point_vendor: str, charge_point_model: str, **kwargs):
    return call_result.BootNotificationPayload(
        current_time=datetime.utcnow().isoformat(),
        interval=10,
        status=RegistrationStatus.accepted
    )

The handler is not registered in the routables and we get the error:

INFO:ocpp:thesimulatedone: receive message [2, "39a937eb-aa77-414e-9d50-870bca5d08d3", "BootNotification", {"chargePointModel": "Generic", "chargePointVendor": "Virtual Charge Point"}]
ERROR:websockets.server:Error in connection handler
Traceback (most recent call last):
  File "/Users/andre/.virtualenvs/tmh-ocpp/lib/python3.8/site-packages/ocpp/charge_point.py", line 162, in _handle_call
    handlers = self.route_map[msg.action]
KeyError: 'BootNotification'

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/Users/andre/.virtualenvs/tmh-ocpp/lib/python3.8/site-packages/websockets/server.py", line 191, in handler
    await self.ws_handler(self, path)
  File "central_system.py", line 78, in on_connect
    await cp.start()
  File "/Users/andre/.virtualenvs/tmh-ocpp/lib/python3.8/site-packages/ocpp/charge_point.py", line 126, in start
    await self.route_message(message)
  File "/Users/andre/.virtualenvs/tmh-ocpp/lib/python3.8/site-packages/ocpp/charge_point.py", line 144, in route_message
    await self._handle_call(msg)
  File "/Users/andre/.virtualenvs/tmh-ocpp/lib/python3.8/site-packages/ocpp/charge_point.py", line 164, in _handle_call
    raise NotImplementedError(f"No handler for '{msg.action}' "
ocpp.exceptions.NotImplementedError: NotImplementedError: No handler for 'BootNotification' registered., {}

This happens because the code in here:

if func.__name__ not in routables:

uses the function name to establish a route, but the say_hello decorator changes the function name to 'inner', in this case.
A quick fix on the client side is to use the functools and decorate the inner method with it, like:

import functools

def say_hello(f):
    @functools.wraps(f)
    def inner(*args, **kwargs):
        print("hello")
        return f(*args, **kwargs)
    return inner

But ideally, the lib should be shielded against other decorators actions

@tropxy tropxy added the bug Something isn't working label Nov 14, 2020
@OrangeTux OrangeTux self-assigned this Jan 5, 2021
@OrangeTux
Copy link
Collaborator

I'm unsure how to fix this on this library. Or even issue a warning to the user.

Consider the following code:

def other(f):
    def inner(*args, **kwargs):
        return f(args, kwargs)

    return inner

class ChargePoint:
    @on(Action.Heartbeat)
    @other
    def on_heartbeat(self):
        pass

Note that decorators are executed from bottom to top. Also note that decorators are syntactic sugar. The above could be rewritten as: on(other)(on_heartbeat)

As you can see on() receives other(). And in it's current implementation on() will register other in a global list of routables. Ans it sets the attributes _on_action and _skip_schema_validation on this method instead of setting them on on_heartbeat.

create_route_map() iterates over that global lists of routables. It verifies if an instance of ChargePoint has an attribute that matches the routables. So in our case something like getattr(ChargePoint(), 'other') is done. That look up will fail.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Development

No branches or pull requests

2 participants