From 5dd0ccb9dc3524a0230d2d3c551d184c83847ffd Mon Sep 17 00:00:00 2001 From: Erik Jaegervall Date: Fri, 8 Nov 2024 14:30:22 +0100 Subject: [PATCH] Make sure v2 used only when possible --- kuksa-client/kuksa_client/grpc/__init__.py | 28 ++++++- kuksa-client/tests/test_grpc.py | 88 +++++++++++++++++++++- 2 files changed, 110 insertions(+), 6 deletions(-) diff --git a/kuksa-client/kuksa_client/grpc/__init__.py b/kuksa-client/kuksa_client/grpc/__init__.py index 9ae1adc..305d01c 100644 --- a/kuksa-client/kuksa_client/grpc/__init__.py +++ b/kuksa-client/kuksa_client/grpc/__init__.py @@ -830,6 +830,18 @@ def _prepare_publish_value_request( value_type = paths_with_required_type.get(update.entry.path) if value_type is not None: update.entry.value_type = value_type + + for field in update.fields: + if field != Field.VALUE: + raise VSSClientError( + error={ + "code": grpc.StatusCode.INVALID_ARGUMENT.value[0], + "reason": grpc.StatusCode.INVALID_ARGUMENT.value[1], + "message": "Cannot use v2 to publish fields other than value", + }, + errors=[], + ) + req = val_v2.PublishValueRequest( signal_id=types_v2.SignalID(path=update.entry.path), data_point=update.entry.value.v2_to_message(update.entry.value_type), @@ -863,6 +875,18 @@ def _prepare_subscribev2_request( paths = [] for entry in entries: paths.append(entry.path) + + for field in entry.fields: + if field != Field.VALUE: + raise VSSClientError( + error={ + "code": grpc.StatusCode.INVALID_ARGUMENT.value[0], + "reason": grpc.StatusCode.INVALID_ARGUMENT.value[1], + "message": "Cannot use v2 if specifiying fields other than value", + }, + errors=[], + ) + req = val_v2.SubscribeRequest(signal_paths=paths) logger.debug("%s: %s", type(req).__name__, req) return req @@ -1220,7 +1244,7 @@ def get(self, entries: Iterable[EntryRequest], **rpc_kwargs) -> List[DataEntry]: @check_connected def set( - self, updates: Collection[EntryUpdate], try_v2: bool = True, **rpc_kwargs + self, updates: Collection[EntryUpdate], try_v2: bool = False, **rpc_kwargs ) -> None: """ Parameters: @@ -1273,7 +1297,7 @@ def set( @check_connected def subscribe( - self, entries: Iterable[SubscribeEntry], try_v2: bool = True, **rpc_kwargs + self, entries: Iterable[SubscribeEntry], try_v2: bool = False, **rpc_kwargs ) -> Iterator[List[EntryUpdate]]: """ Parameters: diff --git a/kuksa-client/tests/test_grpc.py b/kuksa-client/tests/test_grpc.py index 806f787..1b82eb4 100644 --- a/kuksa-client/tests/test_grpc.py +++ b/kuksa-client/tests/test_grpc.py @@ -1313,6 +1313,51 @@ async def test_set_some_updates_v2( ): assert actual_request == expected_request + @pytest.mark.usefixtures("val_server") + async def test_set_some_updates_v2_target( + self, unused_tcp_port, val_servicer_v2, val_servicer_v1 + ): + """ + Similar test to above, but trying to update target values using v2 + which is not allowed + """ + val_servicer_v1.Get.return_value = val_v1.GetResponse( + entries=( + types_v1.DataEntry( + path="Vehicle.Speed", + metadata=types_v1.Metadata(data_type=types_v1.DATA_TYPE_FLOAT), + ), + types_v1.DataEntry( + path="Vehicle.ADAS.ABS.IsActive", + metadata=types_v1.Metadata(data_type=types_v1.DATA_TYPE_BOOLEAN), + ), + ) + ) + _updates = [ + EntryUpdate( + DataEntry("Vehicle.Speed", value=Datapoint(value=42.0)), + (Field.ACTUATOR_TARGET,), + ), + EntryUpdate( + DataEntry( + "Vehicle.ADAS.ABS.IsActive", + value=Datapoint(value=False), + ), + (Field.ACTUATOR_TARGET,), + ), + ] + + async with VSSClient( + "127.0.0.1", unused_tcp_port, ensure_startup_connection=False + ) as client: + with pytest.raises(VSSClientError): + await client.set( + updates=_updates, + try_v2=True, + ) + assert val_servicer_v1.Get.call_count == 1 + assert val_servicer_v2.PublishValue.call_count == 0 + @pytest.mark.usefixtures("val_server") async def test_set_no_updates_provided( self, unused_tcp_port, val_servicer_v1, val_servicer_v2 @@ -1588,16 +1633,17 @@ async def test_subscribe_some_entries_v1( entry for entry in ( # generator is intentional (Iterable) EntryRequest( - "Vehicle.Speed", View.CURRENT_VALUE, (Field.VALUE,) + # View is ignored, so we can provide any value + "Vehicle.Speed", View.FIELDS, (Field.VALUE,) ), EntryRequest( "Vehicle.ADAS.ABS.IsActive", - View.TARGET_VALUE, + View.UNSPECIFIED, (Field.ACTUATOR_TARGET,), ), EntryRequest( "Vehicle.Chassis.Height", - View.METADATA, + View.UNSPECIFIED, (Field.METADATA_DATA_TYPE,), ), ) @@ -1731,7 +1777,8 @@ async def test_subscribe_some_entries_v2( ), EntryRequest( "Vehicle.ADAS.ABS.IsActive", - View.CURRENT_VALUE, + # Specified View is ignored so we can use anyone :-) + View.METADATA, (Field.VALUE,), ), ) @@ -1799,6 +1846,39 @@ async def test_subscribe_some_entries_v2( ], ] + @pytest.mark.usefixtures("val_server") + async def test_subscribe_some_entries_v2_target( + self, mocker, unused_tcp_port, val_servicer_v2 + ): + """ + Similar to above but trying to subscribe to target values which is not possible using v2 + """ + async with VSSClient( + "127.0.0.1", unused_tcp_port, ensure_startup_connection=False + ) as client: + actual_responses = [] + + with pytest.raises(VSSClientError): + async for updates in client.subscribe( + entries=( + entry + for entry in ( # generator is intentional (Iterable) + EntryRequest( + "Vehicle.Speed", View.TARGET_VALUE, (Field.ACTUATOR_TARGET,) + ), + EntryRequest( + "Vehicle.ADAS.ABS.IsActive", + View.TARGET_VALUE, + (Field.ACTUATOR_TARGET,), + ), + ) + ), + try_v2=True, + ): + actual_responses.append(updates) + + assert not actual_responses + @pytest.mark.usefixtures("val_server") async def test_subscribe_no_entries_requested( self, mocker, unused_tcp_port, val_servicer_v1, val_servicer_v2