feat: Add TIME type support for OPC-UA plugin#82
Conversation
Add support for IEC 61131-3 TIME, DATE, TOD, and DT types in the OPC-UA plugin: - Add IEC_TIMESPEC ctypes structure matching C definition (tv_sec, tv_nsec) - Implement TIME type mapping to OPC-UA Int64 (milliseconds) - Implement DATE/DT type mapping to OPC-UA DateTime - Add timespec_to_milliseconds and milliseconds_to_timespec conversion functions - Update convert_value_for_opcua/plc functions to handle TIME types - Add read_timespec_direct and write_timespec_direct memory access functions - Update synchronization to pass datatype hint for TIME handling - Add datatype validation in config model with VALID_DATATYPES constant - Add TIME variable examples to config template TIME values are represented as Int64 milliseconds in OPC-UA, which provides good compatibility with standard OPC-UA clients while maintaining reasonable precision for PLC applications. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add comprehensive unit tests for the new TIME type support: Type conversion tests (test_type_conversions.py): - TIME/TOD/DATE/DT type mappings to OPC-UA types - TIME conversion from tuple to milliseconds - TIME conversion from milliseconds to tuple - TIME roundtrip conversion tests - timespec_to_milliseconds helper function tests - milliseconds_to_timespec helper function tests - TIME_DATATYPES constant tests Memory access tests (test_memory.py): - IEC_TIMESPEC structure tests (size, fields, initialization) - read_timespec_direct function tests - write_timespec_direct function tests - read_memory_direct with TIME datatype hint tests - Roundtrip read/write tests for TIME values All 127 tests pass. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…cua-time-variable
- DATE: Now extracts only the date portion, setting time to 00:00:00 (ignores HH:MM:SS from the IEC_TIMESPEC value) - TOD: Now uses current date (today) + time from IEC_TIMESPEC (ignores YYYY-MM-DD, only uses HH:MM:SS) Changed mapping from Int64 to DateTime for better OPC-UA client compatibility - DT: Unchanged - full DateTime conversion (both date and time) - TIME: Unchanged - Int64 milliseconds representation Updated tests to reflect the new behavior. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
marconetsf
left a comment
There was a problem hiding this comment.
PR Review Summary
Great work on this PR! The implementation is solid, well-tested, and follows the project's coding standards. I've added a few inline comments for minor improvements that can be addressed in a follow-up if desired.
Overall Assessment: ✅ APPROVED
See full review document: docs/pr-reviews/PR_82_REVIEW.md
Strengths
- Well-designed architecture matching IEC 61131-3 specification
- Excellent test coverage (71 new tests)
- Comprehensive development plan document
- Backward compatible API changes
- Good error handling with safe defaults
Minor Issues (non-blocking)
- Duplicate
TIME_DATATYPESconstant in two files - Type hints could be more precise (
tuple->tuple[int, int]) - Silent error handling in TOD conversion could benefit from logging
Minor Issues Found (Non-blocking)These are suggestions for future improvement - not blocking merge. 1. Duplicate constant definition
# Option: Import from opcua_memory instead of defining here
from .opcua_memory import TIME_DATATYPES2. Type hint could be more precise
def milliseconds_to_timespec(ms: int) -> tuple[int, int]:3. Type hint could be more precise
def read_timespec_direct(address: int) -> tuple[int, int]:4. Silent error handling in TOD conversion
except (ValueError, OverflowError) as e:
log_warn(f"Invalid TOD value (hours={hours}), using midnight: {e}")
return datetime(today.year, today.month, today.day, tzinfo=timezone.utc)Full review document available at: |
There was a problem hiding this comment.
Pull request overview
This PR adds comprehensive support for IEC 61131-3 TIME-related data types (TIME, DATE, TOD, DT) to the OPC-UA plugin. TIME/TOD types are represented as Int64 milliseconds in OPC-UA for broad client compatibility, while DATE/DT types map to native OPC-UA DateTime. The implementation includes ctypes structure definitions, memory access functions, type conversions, and extensive test coverage.
Changes:
- Added IEC_TIMESPEC ctypes structure and TIME-specific memory read/write functions
- Implemented TIME type mappings and bidirectional conversion helpers between PLC timespec format and OPC-UA representations
- Updated synchronization logic to handle TIME values via direct memory access and pass datatype hints for proper TIME handling
- Added datatype validation and TIME variable examples to configuration
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/pytest/plugins/opcua/test_type_conversions.py | Added 48 comprehensive unit tests covering TIME type mappings, conversions, roundtrips, and helper functions |
| tests/pytest/plugins/opcua/test_memory.py | Added 23 unit tests for IEC_TIMESPEC structure, TIME memory operations, and datatype hint handling |
| docs/plans/TIME_TYPE_SUPPORT_PLAN.md | Added detailed development and test plan documenting TIME type support implementation |
| core/src/drivers/plugins/python/shared/plugin_config_decode/opcua_config_model.py | Added VALID_DATATYPES constant and datatype validation for TIME types in configuration |
| core/src/drivers/plugins/python/opcua/synchronization.py | Updated to pass datatype hints and handle TIME writes via direct memory access |
| core/src/drivers/plugins/python/opcua/opcua_utils.py | Added TIME_DATATYPES constant, conversion helpers, type mappings, and TIME handling in value conversions |
| core/src/drivers/plugins/python/opcua/opcua_memory.py | Added IEC_TIMESPEC structure, read/write functions, and datatype parameter to read_memory_direct |
| core/src/drivers/plugins/python/opcua/opcua_config_template.json | Added three TIME variable examples demonstrating usage patterns |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Summary
Changes
Core Implementation
IEC_TIMESPECctypes structure,read_timespec_direct(),write_timespec_direct(), updatedread_memory_direct()with datatype hintTIME_DATATYPES, conversion helpers (timespec_to_milliseconds,milliseconds_to_timespec), TIME type mappings, TIME handling in value conversionsConfiguration
VALID_DATATYPESconstant with TIME types, datatype validation in configTests
Design Decisions
Test plan
🤖 Generated with Claude Code