diff --git a/CHANGELOG.md b/CHANGELOG.md index 824eb74b6..30df6c189 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,7 @@ # Change log +- [#544](https://github.com/mobilityhouse/ocpp/issues/544) Pass `Call.unique_id` to the `on` and `after` routing handlers. - [#559](https://github.com/mobilityhouse/ocpp/issues/559) Update project dependencies as of 22-12-2023 - [#447](https://github.com/mobilityhouse/ocpp/issues/447) Make formatting of enums in py3.11 consistent with earlier Python versions - [#421](https://github.com/mobilityhouse/ocpp/issues/421) Type of v16.datatypes.SampledValue.context is incorrect diff --git a/ocpp/charge_point.py b/ocpp/charge_point.py index 997943de7..4d7cb9676 100644 --- a/ocpp/charge_point.py +++ b/ocpp/charge_point.py @@ -225,9 +225,15 @@ async def _handle_call(self, msg): handler = handlers["_on_action"] except KeyError: _raise_key_error(msg.action, self._ocpp_version) - + handler_signature = inspect.signature(handler) + call_unique_id_required = "call_unique_id" in handler_signature.parameters try: - response = handler(**snake_case_payload) + # call_unique_id should be passed as kwarg only if is defined explicitly + # in the handler signature + if call_unique_id_required: + response = handler(**snake_case_payload, call_unique_id=msg.unique_id) + else: + response = handler(**snake_case_payload) if inspect.isawaitable(response): response = await response except Exception as e: @@ -259,9 +265,16 @@ async def _handle_call(self, msg): try: handler = handlers["_after_action"] + handler_signature = inspect.signature(handler) + call_unique_id_required = "call_unique_id" in handler_signature.parameters + # call_unique_id should be passed as kwarg only if is defined explicitly + # in the handler signature + if call_unique_id_required: + response = handler(**snake_case_payload, call_unique_id=msg.unique_id) + else: + response = handler(**snake_case_payload) # Create task to avoid blocking when making a call inside the # after handler - response = handler(**snake_case_payload) if inspect.isawaitable(response): asyncio.ensure_future(response) except KeyError: diff --git a/tests/test_charge_point.py b/tests/test_charge_point.py index e6b320ef0..897fabc2b 100644 --- a/tests/test_charge_point.py +++ b/tests/test_charge_point.py @@ -3,22 +3,27 @@ import pytest from ocpp.charge_point import camel_to_snake_case, remove_nones, snake_to_camel_case -from ocpp.routing import create_route_map, on +from ocpp.messages import Call +from ocpp.routing import after, create_route_map, on +from ocpp.v16 import ChargePoint as cp_16 from ocpp.v16.call import ( BootNotificationPayload, GetConfigurationPayload, MeterValuesPayload, ) +from ocpp.v16.call_result import ( + BootNotificationPayload as BootNotificationResultPayload, +) from ocpp.v16.datatypes import MeterValue, SampledValue -from ocpp.v16.enums import Action -from ocpp.v20 import ChargePoint as cp +from ocpp.v16.enums import Action, RegistrationStatus +from ocpp.v20 import ChargePoint as cp_20 from ocpp.v201.call import SetNetworkProfilePayload from ocpp.v201.datatypes import NetworkConnectionProfileType from ocpp.v201.enums import OCPPInterfaceType, OCPPTransportType, OCPPVersionType def test_getters_should_not_be_called_during_routemap_setup(): - class ChargePoint(cp): + class ChargePoint(cp_20): @property def foo(self): raise RuntimeError("this will be raised") @@ -31,12 +36,12 @@ def foo(self): def test_multiple_classes_with_same_name_for_handler(): - class ChargerA(cp): + class ChargerA(cp_20): @on(Action.Heartbeat) def heartbeat(self, **kwargs): pass - class ChargerB(cp): + class ChargerB(cp_20): @on(Action.Heartbeat) def heartbeat(self, **kwargs): pass @@ -232,3 +237,101 @@ def test_remove_nones_with_list_of_strings(): assert remove_nones(payload) == { "key": ["ClockAlignedDataInterval", "ConnectionTimeOut"] } + + +@pytest.mark.asyncio +async def test_call_unique_id_added_to_handler_args_correctly(connection): + """ + This test ensures that the `call_unique_id` is getting passed to the + `on` and `after` handlers only if it is explicitly set in the handler signature. + + To cover all possible cases, we define two chargers: + + ChargerA: + `call_unique_id` not required on `on` handler but required on `after` handler. + + ChargerB: + `call_unique_id` required on `on` handler but not required on `after` handler. + + Each handler verifies a set of asserts to verify that the `call_unique_id` + is passed correctly. + To confirm that the handlers are actually being called and hence the asserts + are being ran, we introduce a set of counters that increase each time a specific + handler runs. + """ + charger_a_test_call_unique_id = "charger_a_1234" + charger_b_test_call_unique_id = "charger_b_5678" + payload_a = {"chargePointVendor": "foo_a", "chargePointModel": "bar_a"} + payload_b = {"chargePointVendor": "foo_b", "chargePointModel": "bar_b"} + + class ChargerA(cp_16): + on_boot_notification_call_count = 0 + after_boot_notification_call_count = 0 + + @on(Action.BootNotification) + def on_boot_notification(self, *args, **kwargs): + # call_unique_id should not be passed as arg nor kwarg + assert kwargs == camel_to_snake_case(payload_a) + assert args == () + ChargerA.on_boot_notification_call_count += 1 + return BootNotificationResultPayload( + current_time="foo", interval=1, status=RegistrationStatus.accepted + ) + + @after(Action.BootNotification) + def after_boot_notification(self, call_unique_id, *args, **kwargs): + assert call_unique_id == charger_a_test_call_unique_id + assert kwargs == camel_to_snake_case(payload_a) + # call_unique_id should not be passed as arg + assert args == () + ChargerA.after_boot_notification_call_count += 1 + return BootNotificationResultPayload( + current_time="foo", interval=1, status=RegistrationStatus.accepted + ) + + class ChargerB(cp_16): + on_boot_notification_call_count = 0 + after_boot_notification_call_count = 0 + + @on(Action.BootNotification) + def on_boot_notification(self, call_unique_id, *args, **kwargs): + assert call_unique_id == charger_b_test_call_unique_id + assert kwargs == camel_to_snake_case(payload_b) + # call_unique_id should not be passed as arg + assert args == () + ChargerB.on_boot_notification_call_count += 1 + return BootNotificationResultPayload( + current_time="foo", interval=1, status=RegistrationStatus.accepted + ) + + @after(Action.BootNotification) + def after_boot_notification(self, *args, **kwargs): + # call_unique_id should not be passed as arg nor kwarg + assert kwargs == camel_to_snake_case(payload_b) + assert args == () + ChargerB.after_boot_notification_call_count += 1 + return BootNotificationResultPayload( + current_time="foo", interval=1, status=RegistrationStatus.accepted + ) + + charger_a = ChargerA("charger_a_id", connection) + charger_b = ChargerB("charger_b_id", connection) + + msg_a = Call( + unique_id=charger_a_test_call_unique_id, + action=Action.BootNotification.value, + payload=payload_a, + ) + await charger_a._handle_call(msg_a) + + msg_b = Call( + unique_id=charger_b_test_call_unique_id, + action=Action.BootNotification.value, + payload=payload_b, + ) + await charger_b._handle_call(msg_b) + + assert ChargerA.on_boot_notification_call_count == 1 + assert ChargerA.after_boot_notification_call_count == 1 + assert ChargerB.on_boot_notification_call_count == 1 + assert ChargerB.after_boot_notification_call_count == 1