From e1741cda8b4c2e437c226813d29d29844e5a3b83 Mon Sep 17 00:00:00 2001 From: hutcheb Date: Tue, 1 Oct 2024 17:38:57 +0800 Subject: [PATCH] feat(plc4py): Made the reading of the data types more robust --- plc4py/plc4py/drivers/umas/UmasConnection.py | 6 +++--- plc4py/plc4py/drivers/umas/UmasDevice.py | 15 +++++++------ plc4py/plc4py/drivers/umas/UmasVariables.py | 21 ++++++++++++++++--- ...masPDUReadUnlocatedVariableNamesRequest.py | 6 +++--- plc4py/plc4py/spi/messages/PlcBrowser.py | 6 ++---- .../main/resources/protocols/umas/umas.mspec | 2 +- 6 files changed, 36 insertions(+), 20 deletions(-) diff --git a/plc4py/plc4py/drivers/umas/UmasConnection.py b/plc4py/plc4py/drivers/umas/UmasConnection.py index 5fb02fe9142..4257b3c3d24 100644 --- a/plc4py/plc4py/drivers/umas/UmasConnection.py +++ b/plc4py/plc4py/drivers/umas/UmasConnection.py @@ -121,7 +121,7 @@ def browse_request_builder(self) -> BrowseRequestBuilder: """ return DefaultBrowseRequestBuilder() - def execute(self, request: PlcRequest) -> Awaitable[PlcResponse]: + async def execute(self, request: PlcRequest) -> PlcResponse: """ Executes a PlcRequest as long as it's already connected :param request: Plc Request to execute @@ -131,10 +131,10 @@ def execute(self, request: PlcRequest) -> Awaitable[PlcResponse]: return self._default_failed_request(PlcResponseCode.NOT_CONNECTED) if isinstance(request, PlcReadRequest): - return self._read(request) + return await self._read(request) if isinstance(request, PlcBrowseRequest): - return self._browse(request) + return await self._browse(request) return self._default_failed_request(PlcResponseCode.NOT_CONNECTED) diff --git a/plc4py/plc4py/drivers/umas/UmasDevice.py b/plc4py/plc4py/drivers/umas/UmasDevice.py index 5e8d5c710ce..828886999e6 100644 --- a/plc4py/plc4py/drivers/umas/UmasDevice.py +++ b/plc4py/plc4py/drivers/umas/UmasDevice.py @@ -131,15 +131,16 @@ async def _update_plc_project_info(self, transport, loop): await self._send_read_memory_block(transport, loop) offset = 0x0000 first_message = True - data_types: List[UmasDatatypeReference] = {} + data_types: List[UmasDatatypeReference] = [] while offset != 0x0000 or first_message: first_message = False ( offset, - data_types, + temp_data_types, ) = await self._send_unlocated_variable_datatype_request( transport, loop, offset ) + data_types.extend(temp_data_types) data_type_children: Dict[str, List[UmasUDTDefinition]] = {} for data_type in data_types: if data_type.class_identifier == 2: @@ -169,9 +170,11 @@ def _generate_variable_tree( ) -> Dict[str, UmasVariable]: return_dict = {} for kea, tag in tags.items(): - return_dict[kea] = UmasVariableBuilder( + temp_variable = UmasVariableBuilder( kea, tag, data_types, data_type_children ).build() + if temp_variable is not None: + return_dict[kea] = temp_variable return return_dict async def _send_plc_ident(self, transport: Transport, loop: AbstractEventLoop): @@ -267,9 +270,9 @@ async def _send_unlocated_variable_datatype_request( request_pdu = UmasPDUReadUnlocatedVariableNamesRequestBuilder( record_type=0xDD03, - block_no=0x0000, + block_no=offset, index=self.index, - offset=offset, + offset=0x0000, hardware_id=self.hardware_id, ).build(0, 0) @@ -288,7 +291,7 @@ async def _send_unlocated_variable_datatype_request( bytearray(data_type_response.block), ByteOrder.LITTLE_ENDIAN ) basic_info = UmasPDUReadDatatypeNamesResponse.static_parse(read_buffer) - return basic_info.next_address, basic_info.records + return basic_info.range, basic_info.records async def _send_unlocated_variable_datatype_format_request( self, diff --git a/plc4py/plc4py/drivers/umas/UmasVariables.py b/plc4py/plc4py/drivers/umas/UmasVariables.py index 6e2d3a04220..823e7dda884 100644 --- a/plc4py/plc4py/drivers/umas/UmasVariables.py +++ b/plc4py/plc4py/drivers/umas/UmasVariables.py @@ -147,7 +147,7 @@ class UmasVariableBuilder: def build(self) -> UmasVariable: variable: UmasVariable = None _ARRAY_REGEX: str = ( - r"^ARRAY\[(?P[0-9]*)..(?P[0-9]*)\] OF (?P[a-zA-z0-9]*)" + r"^ARRAY\[(?P[0-9]*)..(?P[0-9]*)\] OF (?P[a-zA-z0-9_]*)" ) _ARRAY_COMPILED: Pattern[AnyStr] = re.compile(_ARRAY_REGEX) @@ -192,10 +192,25 @@ def build(self) -> UmasVariable: ) elif data_type_reference.class_identifier == 4: match = _ARRAY_COMPILED.match(data_type_reference.value) - data_type = UmasDataType[match.group("data_type")] + data_type = None + if hasattr(UmasDataType, match.group("data_type")): + data_type = UmasDataType[match.group("data_type")].value + else: + for child_data_type_reference in self.data_type_references: + if ( + match.group("data_type") + == child_data_type_reference.value + ): + data_type = child_data_type_reference.data_type + break + if data_type is None: + raise NotImplementedError( + "Unable to read structures of UDT's" + ) + variable = UmasArrayVariable( self.tag_reference.value, - data_type.value, + data_type, self.block, self.tag_reference.offset + self.offset, int(match.group("start_number")), diff --git a/plc4py/plc4py/protocols/umas/readwrite/UmasPDUReadUnlocatedVariableNamesRequest.py b/plc4py/plc4py/protocols/umas/readwrite/UmasPDUReadUnlocatedVariableNamesRequest.py index 2a31ba96a53..8996d19a441 100644 --- a/plc4py/plc4py/protocols/umas/readwrite/UmasPDUReadUnlocatedVariableNamesRequest.py +++ b/plc4py/plc4py/protocols/umas/readwrite/UmasPDUReadUnlocatedVariableNamesRequest.py @@ -71,7 +71,7 @@ def serialize_umas_pduitem_child(self, write_buffer: WriteBuffer): ) # Const Field (blank) - write_buffer.write_unsigned_short(self.BLANK, logical_name="blank") + write_buffer.write_unsigned_byte(self.BLANK, logical_name="blank") write_buffer.pop_context("UmasPDUReadUnlocatedVariableNamesRequest") @@ -98,7 +98,7 @@ def length_in_bits(self) -> int: length_in_bits += 16 # Const Field (blank) - length_in_bits += 16 + length_in_bits += 8 return length_in_bits @@ -153,7 +153,7 @@ def static_parse_builder( byte_length=byte_length, ) - BLANK: int = read_buffer.read_unsigned_short( + BLANK: int = read_buffer.read_unsigned_byte( logical_name="blank", byte_order=ByteOrder.LITTLE_ENDIAN, umas_request_function_key=umas_request_function_key, diff --git a/plc4py/plc4py/spi/messages/PlcBrowser.py b/plc4py/plc4py/spi/messages/PlcBrowser.py index 4cfcf174cb2..6b948c5d8db 100644 --- a/plc4py/plc4py/spi/messages/PlcBrowser.py +++ b/plc4py/plc4py/spi/messages/PlcBrowser.py @@ -68,11 +68,9 @@ async def _browse(self, request: PlcBrowseRequest) -> PlcBrowseResponse: ) # Return the response return response - except Exception: - # If an error occurs during the execution of the browse request, return a response with - # the INTERNAL_ERROR code. This exception is very general and probably should be replaced. + except Exception as e: # TODO:- This exception is very general and probably should be replaced - return PlcBrowseResponse(PlcResponseCode.INTERNAL_ERROR, {}) + raise e def is_browse_supported(self) -> bool: """ diff --git a/protocols/umas/src/main/resources/protocols/umas/umas.mspec b/protocols/umas/src/main/resources/protocols/umas/umas.mspec index 1acd5594039..08d611a86db 100644 --- a/protocols/umas/src/main/resources/protocols/umas/umas.mspec +++ b/protocols/umas/src/main/resources/protocols/umas/umas.mspec @@ -97,7 +97,7 @@ [simple uint 32 hardwareId] [simple uint 16 blockNo] [simple uint 16 offset] - [const uint 16 blank 0x00] + [const uint 8 blank 0x00] ] ['0xFD' UmasPDUErrorResponse [array uint 8 block count 'byteLength - 2']