fix: Add missing IEC 61131-3 type mappings to OPC-UA plugin - #93
Conversation
SINT variables were not mapped in map_plc_to_opcua_type(), causing BadInternalError when OPC-UA clients tried to read them. The asyncua library failed to serialize responses because unmapped types defaulted to ua.VariantType.Variant, which couldn't handle raw integer values. Added type mappings for: - SINT (SByte), USINT (Byte) - UINT (UInt16), WORD (UInt16) - UDINT (UInt32), DWORD (UInt32) - ULINT (UInt64), LWORD (UInt64) - LREAL (Double) Also added corresponding value conversion handling in both convert_value_for_opcua() and convert_value_for_plc() functions to support both read and write operations. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR fixes a BadInternalError that occurred when reading SINT variables via OPC-UA clients by adding comprehensive type mappings for IEC 61131-3 data types that were previously missing.
Changes:
- Added type mappings for 9 missing IEC 61131-3 types (SINT, USINT, UINT, WORD, UDINT, DWORD, ULINT, LWORD, LREAL)
- Implemented value conversion logic for both read and write operations for all new types
- Added comprehensive test coverage for the new type mappings and conversions
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| core/src/drivers/plugins/python/opcua/opcua_utils.py | Added type mappings and conversion handlers for 9 missing IEC 61131-3 types in map_plc_to_opcua_type(), convert_value_for_opcua(), and convert_value_for_plc() |
| tests/pytest/plugins/opcua/test_type_conversions.py | Added test cases for new type mappings (SINT, USINT, UINT, WORD, UDINT, DWORD, ULINT, LWORD, LREAL) and SINT value conversion/roundtrip tests |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| elif datatype.upper() in ["UINT", "Uint", "WORD", "Word"]: | ||
| # Ensure proper uint16 type for OPC-UA compatibility | ||
| clamped_value = max(0, min(65535, int(value))) | ||
| return ctypes.c_uint16(clamped_value).value |
There was a problem hiding this comment.
The clamping logic is duplicated across multiple type handlers. Consider extracting the clamping and conversion into a helper function, e.g., _clamp_and_convert(value, min_val, max_val, ctype) to reduce code duplication and improve maintainability.
| elif datatype.upper() in ["UINT", "Uint", "WORD", "Word"]: | ||
| # Ensure proper uint16 type for PLC compatibility | ||
| clamped_value = max(0, min(65535, int(value))) | ||
| return ctypes.c_uint16(clamped_value).value |
There was a problem hiding this comment.
The clamping logic is duplicated across multiple type handlers in this function as well. Consider extracting the clamping and conversion into a helper function to reduce code duplication, consistent with the suggestion for convert_value_for_opcua().
- Remove redundant mixed-case strings from datatype checks since .upper() is already called (e.g., "Sint" is unreachable after "sint".upper() == "SINT") - Replace bare except clauses with specific struct.error catches to avoid catching unintended exceptions like KeyboardInterrupt https://claude.ai/code/session_01FULyWikeb5YVJX8NVX5hax
refactor: Clean up type checking in OPC-UA plugin
Summary
BadInternalErrorwhen reading SINT variables via OPC-UA clients (e.g., UaExpert)Problem
When reading a SINT variable (e.g.,
TON0.STATE) via an OPC-UA client, the server returnedBadInternalErrorwith:This occurred because SINT was not mapped in
map_plc_to_opcua_type(), defaulting toua.VariantType.Variant, which the asyncua library couldn't serialize with raw integer values.Changes
opcua_utils.py:convert_value_for_opcua()andconvert_value_for_plc()test_type_conversions.py:Test plan
🤖 Generated with Claude Code