From 3e7d4277828d8a9c9b7d683a8bdd54de2bb8bc14 Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Sun, 7 Dec 2025 20:03:50 +0000 Subject: [PATCH 01/17] Optimize mutex usage to minimize hold time and prevent priority inversion This commit implements three key optimizations for the mutex protecting image table access in Python plugins: 1. Priority Inheritance for Mutex (C core) - Modified plugin_driver_create() to use pthread_mutexattr_setprotocol() with PTHREAD_PRIO_INHERIT - Prevents priority inversion when lower-priority plugin threads hold the mutex while the real-time PLC scan cycle thread is waiting 2. Move Conversion Logic Outside Critical Section (modbus_master_memory.py) - Added new optimized functions that separate data conversion from buffer access: * convert_modbus_data_to_iec_values() - pre-convert before mutex * write_preconverted_iec_values() - write under mutex * read_raw_iec_values() - read under mutex * convert_raw_iec_to_modbus() - convert after mutex release - Legacy functions kept for backward compatibility 3. Batch Write Preparations (modbus_master_plugin.py) - READ operations: Pre-convert all Modbus data to IEC values before acquiring mutex, then write all pre-converted values under single mutex acquisition - WRITE operations: Collect all due write points, read all raw IEC values under single mutex acquisition, then convert and perform Modbus writes outside the mutex These optimizations reduce mutex hold time by ensuring only actual buffer access operations occur within the critical section, while CPU-intensive conversion work happens outside. Co-Authored-By: Thiago Alves --- core/src/drivers/plugin_driver.c | 98 ++-- .../modbus_master/modbus_master_memory.py | 478 ++++++++++++++++-- .../modbus_master/modbus_master_plugin.py | 127 +++-- 3 files changed, 591 insertions(+), 112 deletions(-) diff --git a/core/src/drivers/plugin_driver.c b/core/src/drivers/plugin_driver.c index 64065165..760fbd12 100644 --- a/core/src/drivers/plugin_driver.c +++ b/core/src/drivers/plugin_driver.c @@ -5,11 +5,11 @@ #include "../plc_app/utils/log.h" #include "plugin_config.h" #include "plugin_driver.h" +#include #include #include #include #include -#include // External buffer declarations from image_tables.c extern IEC_BOOL *bool_input[BUFFER_SIZE][8]; @@ -41,13 +41,33 @@ plugin_driver_t *plugin_driver_create(void) return NULL; } - // Initialize mutex - if (pthread_mutex_init(&driver->buffer_mutex, NULL) != 0) + // Initialize mutex with priority inheritance to prevent priority inversion + // This ensures that when a lower-priority plugin thread holds the mutex, + // it temporarily inherits the priority of any higher-priority thread + // (like the PLC scan cycle thread) waiting for the mutex. + pthread_mutexattr_t mutex_attr; + if (pthread_mutexattr_init(&mutex_attr) != 0) + { + free(driver); + return NULL; + } + + if (pthread_mutexattr_setprotocol(&mutex_attr, PTHREAD_PRIO_INHERIT) != 0) + { + pthread_mutexattr_destroy(&mutex_attr); + free(driver); + return NULL; + } + + if (pthread_mutex_init(&driver->buffer_mutex, &mutex_attr) != 0) { + pthread_mutexattr_destroy(&mutex_attr); free(driver); return NULL; } + pthread_mutexattr_destroy(&mutex_attr); + return driver; } @@ -105,34 +125,37 @@ int plugin_driver_load_config(plugin_driver_t *driver, const char *config_file) // Check if config file exists, if not copy from default if (access(config_file, F_OK) != 0) { - printf("[PLUGIN]: Config file %s not found, copying from plugins_default.conf\n", config_file); - + printf("[PLUGIN]: Config file %s not found, copying from plugins_default.conf\n", + config_file); + // Check if default config exists if (access("plugins_default.conf", F_OK) != 0) { printf("[PLUGIN]: Default config file plugins_default.conf not found\n"); return -1; } - + // Copy default config to target config file FILE *src = fopen("plugins_default.conf", "r"); FILE *dst = fopen(config_file, "w"); - + if (!src || !dst) { printf("[PLUGIN]: Failed to copy default config\n"); - if (src) fclose(src); - if (dst) fclose(dst); + if (src) + fclose(src); + if (dst) + fclose(dst); return -1; } - + char buffer[1024]; size_t bytes; while ((bytes = fread(buffer, 1, sizeof(buffer), src)) > 0) { fwrite(buffer, 1, bytes, dst); } - + fclose(src); fclose(dst); printf("[PLUGIN]: Successfully copied default config to %s\n", config_file); @@ -146,11 +169,12 @@ int plugin_driver_load_config(plugin_driver_t *driver, const char *config_file) } driver->plugin_count = config_count; - has_python_plugin = 0; + has_python_plugin = 0; for (int w = 0; w < config_count; w++) { memcpy(&driver->plugins[w].config, &configs[w], sizeof(plugin_config_t)); - if (configs[w].type == PLUGIN_TYPE_PYTHON) { + if (configs[w].type == PLUGIN_TYPE_PYTHON) + { has_python_plugin = 1; } } @@ -170,7 +194,9 @@ int plugin_driver_load_config(plugin_driver_t *driver, const char *config_file) plugin_manager_destroy(plugin->manager); return -1; } - } else if (plugin->config.type == PLUGIN_TYPE_NATIVE) { + } + else if (plugin->config.type == PLUGIN_TYPE_NATIVE) + { if (native_plugin_get_symbols(plugin) != 0) { fprintf(stderr, "Failed to get native plugin symbols for: %s\n", @@ -195,14 +221,14 @@ int plugin_driver_init(plugin_driver_t *driver) for (int i = 0; i < driver->plugin_count; i++) { plugin_instance_t *plugin = &driver->plugins[i]; - + // Skip disabled plugins if (!plugin->config.enabled) { printf("[PLUGIN]: Skipping disabled plugin: %s\n", plugin->config.name); continue; } - + if (plugin->config.type == PLUGIN_TYPE_PYTHON && plugin->python_plugin && plugin->python_plugin->pFuncInit) { @@ -218,7 +244,7 @@ int plugin_driver_init(plugin_driver_t *driver) // Call the Python init function with proper capsule PyObject *result = PyObject_CallFunctionObjArgs(plugin->python_plugin->pFuncInit, args, NULL); - + // Store the capsule reference for the lifetime of the plugin plugin->python_plugin->args_capsule = args; @@ -236,7 +262,8 @@ int plugin_driver_init(plugin_driver_t *driver) { // Generate structured args for native plugin plugin_runtime_args_t *args = - (plugin_runtime_args_t *)generate_structured_args_with_driver(PLUGIN_TYPE_NATIVE, driver, i); + (plugin_runtime_args_t *)generate_structured_args_with_driver(PLUGIN_TYPE_NATIVE, + driver, i); if (!args) { fprintf(stderr, "Failed to generate runtime args for native plugin: %s\n", @@ -276,8 +303,8 @@ int plugin_driver_start(plugin_driver_t *driver) return 0; } - - if (has_python_plugin) { + if (has_python_plugin) + { main_tstate = PyEval_SaveThread(); gstate = PyGILState_Ensure(); } @@ -285,14 +312,14 @@ int plugin_driver_start(plugin_driver_t *driver) for (int i = 0; i < driver->plugin_count; i++) { plugin_instance_t *plugin = &driver->plugins[i]; - + // Skip disabled plugins if (!plugin->config.enabled) { printf("[PLUGIN]: Skipping disabled plugin during start: %s\n", plugin->config.name); continue; } - + switch (plugin->config.type) { case PLUGIN_TYPE_PYTHON: @@ -347,7 +374,8 @@ int plugin_driver_start(plugin_driver_t *driver) break; } } - if (has_python_plugin) { + if (has_python_plugin) + { PyGILState_Release(gstate); } return 0; @@ -475,7 +503,8 @@ void plugin_driver_destroy(plugin_driver_t *driver) return; } - if (has_python_plugin) { + if (has_python_plugin) + { gstate = PyGILState_Ensure(); } @@ -499,10 +528,12 @@ void plugin_driver_destroy(plugin_driver_t *driver) if (plugin->native_plugin->cleanup) { plugin->native_plugin->cleanup(); - printf("[PLUGIN]: Native plugin %s cleaned up successfully.\n", plugin->config.name); + printf("[PLUGIN]: Native plugin %s cleaned up successfully.\n", + plugin->config.name); } // Close the shared library handle - if (plugin->native_plugin->handle) { + if (plugin->native_plugin->handle) + { dlclose(plugin->native_plugin->handle); plugin->native_plugin->handle = NULL; } @@ -512,7 +543,8 @@ void plugin_driver_destroy(plugin_driver_t *driver) } } - if (has_python_plugin) { + if (has_python_plugin) + { PyGILState_Release(gstate); PyEval_RestoreThread(main_tstate); Py_FinalizeEx(); @@ -860,28 +892,32 @@ int native_plugin_get_symbols(plugin_instance_t *plugin) native_bundle->start = (plugin_start_loop_func_t)dlsym(handle, "start_loop"); if (!native_bundle->start) { - fprintf(stderr, "Warning: 'start_loop' function not found in native plugin '%s' (optional)\n", + fprintf(stderr, + "Warning: 'start_loop' function not found in native plugin '%s' (optional)\n", plugin->config.path); } native_bundle->stop = (plugin_stop_loop_func_t)dlsym(handle, "stop_loop"); if (!native_bundle->stop) { - fprintf(stderr, "Warning: 'stop_loop' function not found in native plugin '%s' (optional)\n", + fprintf(stderr, + "Warning: 'stop_loop' function not found in native plugin '%s' (optional)\n", plugin->config.path); } native_bundle->cycle_start = (plugin_cycle_start_func_t)dlsym(handle, "cycle_start"); if (!native_bundle->cycle_start) { - fprintf(stderr, "Warning: 'cycle_start' function not found in native plugin '%s' (optional)\n", + fprintf(stderr, + "Warning: 'cycle_start' function not found in native plugin '%s' (optional)\n", plugin->config.path); } native_bundle->cycle_end = (plugin_cycle_end_func_t)dlsym(handle, "cycle_end"); if (!native_bundle->cycle_end) { - fprintf(stderr, "Warning: 'cycle_end' function not found in native plugin '%s' (optional)\n", + fprintf(stderr, + "Warning: 'cycle_end' function not found in native plugin '%s' (optional)\n", plugin->config.path); } diff --git a/core/src/drivers/plugins/python/modbus_master/modbus_master_memory.py b/core/src/drivers/plugins/python/modbus_master/modbus_master_memory.py index 8a3177ae..67a2d9c4 100644 --- a/core/src/drivers/plugins/python/modbus_master/modbus_master_memory.py +++ b/core/src/drivers/plugins/python/modbus_master/modbus_master_memory.py @@ -1,6 +1,19 @@ -"""Modbus Master plugin memory access utilities.""" +"""Modbus Master plugin memory access utilities. -from typing import Optional, Dict, Any, List +This module provides utilities for accessing IEC buffers from the Modbus Master plugin. +It is designed to minimize mutex hold time by separating data conversion from buffer access: + +- For READ operations (Modbus -> IEC): Convert Modbus data to IEC values BEFORE acquiring + the mutex, then write the pre-converted values to buffers under the mutex. + +- For WRITE operations (IEC -> Modbus): Read raw IEC values under the mutex, then convert + to Modbus format AFTER releasing the mutex. + +This pattern ensures the mutex is only held during actual buffer access, not during +CPU-intensive conversion operations. +""" + +from typing import List, Optional, Tuple try: # Try relative imports first (when used as package) @@ -12,15 +25,15 @@ # Import utility functions to avoid circular imports try: from .modbus_master_utils import ( - get_modbus_registers_count_for_iec_size, + convert_iec_value_to_modbus_registers, convert_modbus_registers_to_iec_value, - convert_iec_value_to_modbus_registers + get_modbus_registers_count_for_iec_size, ) except ImportError: from modbus_master_utils import ( - get_modbus_registers_count_for_iec_size, + convert_iec_value_to_modbus_registers, convert_modbus_registers_to_iec_value, - convert_iec_value_to_modbus_registers + get_modbus_registers_count_for_iec_size, ) @@ -40,7 +53,7 @@ def get_sba_access_details(iec_addr, is_write_op: bool = False) -> Optional[Buff size = iec_addr.size # Determine if this is a boolean operation - is_boolean = (size == "X") + is_boolean = size == "X" # Calculate buffer_idx based on size and index_bytes if size == "X": # Boolean - 1 bit @@ -74,7 +87,7 @@ def get_sba_access_details(iec_addr, is_write_op: bool = False) -> Optional[Buff elif area == "Q": buffer_type_str = "bool_output" elif area == "M": - print(f"Memory area 'M' not supported for boolean operations") + print("Memory area 'M' not supported for boolean operations") return None else: print(f"Unknown area for boolean: {area}") @@ -125,7 +138,7 @@ def get_sba_access_details(iec_addr, is_write_op: bool = False) -> Optional[Buff buffer_idx=buffer_idx, bit_idx=bit_idx, element_size_bytes=element_size_bytes, - is_boolean=is_boolean + is_boolean=is_boolean, ) except Exception as e: @@ -133,7 +146,355 @@ def get_sba_access_details(iec_addr, is_write_op: bool = False) -> Optional[Buff return None -def update_iec_buffer_from_modbus_data(sba, iec_addr, modbus_data: list, length: int): +# ============================================================================= +# OPTIMIZED FUNCTIONS FOR MINIMAL MUTEX HOLD TIME +# ============================================================================= +# These functions separate data conversion from buffer access to minimize +# the time the mutex is held. Use these instead of the legacy functions +# when mutex hold time is critical. +# ============================================================================= + + +def convert_modbus_data_to_iec_values( # pylint: disable=too-many-locals + iec_addr, modbus_data: list, length: int +) -> Tuple[Optional[List], Optional[BufferAccessDetails]]: + """ + Pre-converts Modbus data to IEC values BEFORE acquiring the mutex. + + This function performs all CPU-intensive conversion work outside the + critical section, returning pre-converted values that can be quickly + written to buffers under the mutex. + + Args: + iec_addr: IECAddress object + modbus_data: List of values from Modbus (booleans for coils/inputs, + integers for registers) + length: Number of IEC elements to convert + + Returns: + Tuple of (converted_values, buffer_details) or (None, None) if failed. + converted_values is a list of tuples: (buffer_idx, bit_idx_or_none, value) + """ + try: + details = get_sba_access_details(iec_addr, is_write_op=True) + if not details: + print(f"(FAIL) Failed to get SBA access details for {iec_addr}") + return None, None + + base_buffer_idx = details.buffer_idx + base_bit_idx = details.bit_idx + is_boolean = details.is_boolean + iec_size = iec_addr.size + + converted_values = [] + + for i in range(length): + if is_boolean: + if i >= len(modbus_data): + break + + current_data = modbus_data[i] + + if base_bit_idx is not None: + current_bit_idx = base_bit_idx + i + current_buffer_idx = base_buffer_idx + (current_bit_idx // 8) + actual_bit_idx = current_bit_idx % 8 + else: + current_buffer_idx = base_buffer_idx + actual_bit_idx = i + + converted_values.append((current_buffer_idx, actual_bit_idx, current_data)) + + else: + registers_per_element = get_modbus_registers_count_for_iec_size(iec_size) + start_reg_idx = i * registers_per_element + end_reg_idx = start_reg_idx + registers_per_element + + if end_reg_idx > len(modbus_data): + break + + element_registers = modbus_data[start_reg_idx:end_reg_idx] + + try: + if iec_size in ["B", "W"]: + current_data = convert_modbus_registers_to_iec_value( + element_registers, iec_size + ) + elif iec_size in ["D", "L"]: + current_data = convert_modbus_registers_to_iec_value( + element_registers, iec_size, use_big_endian=False + ) + else: + print(f"Unsupported IEC size: {iec_size}") + continue + except ValueError as e: + print(f"(FAIL) Error converting registers to IEC value: {e}") + continue + + current_buffer_idx = base_buffer_idx + i + converted_values.append((current_buffer_idx, None, current_data)) + + return converted_values, details + + except Exception as e: + print(f"(FAIL) Error in convert_modbus_data_to_iec_values: {e}") + return None, None + + +def write_preconverted_iec_values( + sba, converted_values: List[Tuple], details: BufferAccessDetails +) -> bool: + """ + Writes pre-converted IEC values to buffers. Call this UNDER the mutex. + + This function performs only the minimal buffer write operations, + as all conversion has already been done by convert_modbus_data_to_iec_values(). + + Args: + sba: SafeBufferAccess instance + converted_values: List of tuples (buffer_idx, bit_idx_or_none, value) + from convert_modbus_data_to_iec_values() + details: BufferAccessDetails from convert_modbus_data_to_iec_values() + + Returns: + True if all writes succeeded, False otherwise + """ + try: + buffer_type = details.buffer_type_str + is_boolean = details.is_boolean + all_success = True + + for buffer_idx, bit_idx, value in converted_values: + if is_boolean: + if buffer_type == "bool_input": + success, msg = sba.write_bool_input( + buffer_idx, bit_idx, value, thread_safe=False + ) + elif buffer_type == "bool_output": + success, msg = sba.write_bool_output( + buffer_idx, bit_idx, value, thread_safe=False + ) + else: + print(f"Unexpected boolean buffer type: {buffer_type}") + all_success = False + continue + + if not success: + print( + f"(FAIL) Failed to write boolean at buffer {buffer_idx}, " + f"bit {bit_idx}: {msg}" + ) + all_success = False + + else: + if buffer_type == "byte_input": + success, msg = sba.write_byte_input(buffer_idx, value, thread_safe=False) + elif buffer_type == "byte_output": + success, msg = sba.write_byte_output(buffer_idx, value, thread_safe=False) + elif buffer_type == "int_input": + success, msg = sba.write_int_input(buffer_idx, value, thread_safe=False) + elif buffer_type == "int_output": + success, msg = sba.write_int_output(buffer_idx, value, thread_safe=False) + elif buffer_type == "int_memory": + success, msg = sba.write_int_memory(buffer_idx, value, thread_safe=False) + elif buffer_type == "dint_input": + success, msg = sba.write_dint_input(buffer_idx, value, thread_safe=False) + elif buffer_type == "dint_output": + success, msg = sba.write_dint_output(buffer_idx, value, thread_safe=False) + elif buffer_type == "dint_memory": + success, msg = sba.write_dint_memory(buffer_idx, value, thread_safe=False) + elif buffer_type == "lint_input": + success, msg = sba.write_lint_input(buffer_idx, value, thread_safe=False) + elif buffer_type == "lint_output": + success, msg = sba.write_lint_output(buffer_idx, value, thread_safe=False) + elif buffer_type == "lint_memory": + success, msg = sba.write_lint_memory(buffer_idx, value, thread_safe=False) + else: + print(f"Unknown buffer type: {buffer_type}") + all_success = False + continue + + if not success: + print(f"(FAIL) Failed to write {buffer_type} at index {buffer_idx}: {msg}") + all_success = False + + return all_success + + except Exception as e: + print(f"(FAIL) Error in write_preconverted_iec_values: {e}") + return False + + +def read_raw_iec_values( # pylint: disable=too-many-locals + sba, iec_addr, length: int +) -> Tuple[Optional[List], Optional[BufferAccessDetails], Optional[str]]: + """ + Reads raw IEC values from buffers. Call this UNDER the mutex. + + This function performs only the minimal buffer read operations. + Conversion to Modbus format should be done AFTER releasing the mutex + using convert_raw_iec_to_modbus(). + + Args: + sba: SafeBufferAccess instance + iec_addr: IECAddress object + length: Number of IEC elements to read + + Returns: + Tuple of (raw_values, buffer_details, iec_size) or (None, None, None) if failed. + raw_values is a list of raw IEC values (booleans or integers). + """ + try: + details = get_sba_access_details(iec_addr, is_write_op=False) + if not details: + print(f"(FAIL) Failed to get SBA access details for {iec_addr}") + return None, None, None + + buffer_type = details.buffer_type_str + base_buffer_idx = details.buffer_idx + base_bit_idx = details.bit_idx + is_boolean = details.is_boolean + iec_size = iec_addr.size + + raw_values = [] + + for i in range(length): + if is_boolean: + if base_bit_idx is not None: + current_bit_idx = base_bit_idx + i + current_buffer_idx = base_buffer_idx + (current_bit_idx // 8) + actual_bit_idx = current_bit_idx % 8 + else: + current_buffer_idx = base_buffer_idx + actual_bit_idx = i + + if buffer_type == "bool_input": + value, msg = sba.read_bool_input( + current_buffer_idx, actual_bit_idx, thread_safe=False + ) + elif buffer_type == "bool_output": + value, msg = sba.read_bool_output( + current_buffer_idx, actual_bit_idx, thread_safe=False + ) + else: + print(f"Unexpected boolean buffer type: {buffer_type}") + return None, None, None + + if msg != "Success": + print( + f"(FAIL) Failed to read boolean at buffer {current_buffer_idx}, " + f"bit {actual_bit_idx}: {msg}" + ) + return None, None, None + + raw_values.append(value) + + else: + current_buffer_idx = base_buffer_idx + i + + if buffer_type == "byte_input": + value, msg = sba.read_byte_input(current_buffer_idx, thread_safe=False) + elif buffer_type == "byte_output": + value, msg = sba.read_byte_output(current_buffer_idx, thread_safe=False) + elif buffer_type == "int_input": + value, msg = sba.read_int_input(current_buffer_idx, thread_safe=False) + elif buffer_type == "int_output": + value, msg = sba.read_int_output(current_buffer_idx, thread_safe=False) + elif buffer_type == "int_memory": + value, msg = sba.read_int_memory(current_buffer_idx, thread_safe=False) + elif buffer_type == "dint_input": + value, msg = sba.read_dint_input(current_buffer_idx, thread_safe=False) + elif buffer_type == "dint_output": + value, msg = sba.read_dint_output(current_buffer_idx, thread_safe=False) + elif buffer_type == "dint_memory": + value, msg = sba.read_dint_memory(current_buffer_idx, thread_safe=False) + elif buffer_type == "lint_input": + value, msg = sba.read_lint_input(current_buffer_idx, thread_safe=False) + elif buffer_type == "lint_output": + value, msg = sba.read_lint_output(current_buffer_idx, thread_safe=False) + elif buffer_type == "lint_memory": + value, msg = sba.read_lint_memory(current_buffer_idx, thread_safe=False) + else: + print(f"Unknown buffer type: {buffer_type}") + return None, None, None + + if msg != "Success": + print( + f"(FAIL) Failed to read {buffer_type} at index {current_buffer_idx}: {msg}" + ) + return None, None, None + + raw_values.append(value) + + return raw_values, details, iec_size + + except Exception as e: + print(f"(FAIL) Error in read_raw_iec_values: {e}") + return None, None, None + + +def convert_raw_iec_to_modbus( + raw_values: List, details: BufferAccessDetails, iec_size: str +) -> Optional[List]: + """ + Converts raw IEC values to Modbus format. Call this AFTER releasing the mutex. + + This function performs all CPU-intensive conversion work outside the + critical section. + + Args: + raw_values: List of raw IEC values from read_raw_iec_values() + details: BufferAccessDetails from read_raw_iec_values() + iec_size: IEC size string ('X', 'B', 'W', 'D', 'L') + + Returns: + List of values ready for Modbus write, or None if failed + """ + try: + is_boolean = details.is_boolean + + if is_boolean: + # Boolean values are already in the correct format for Modbus + return list(raw_values) + + # Convert non-boolean IEC values to Modbus registers + modbus_values = [] + for value in raw_values: + try: + if iec_size in ["B", "W"]: + element_registers = convert_iec_value_to_modbus_registers(value, iec_size) + elif iec_size in ["D", "L"]: + element_registers = convert_iec_value_to_modbus_registers( + value, iec_size, use_big_endian=False + ) + else: + print(f"Unsupported IEC size: {iec_size}") + return None + + modbus_values.extend(element_registers) + + except ValueError as e: + print(f"(FAIL) Error converting IEC value to registers: {e}") + return None + + return modbus_values + + except Exception as e: + print(f"(FAIL) Error in convert_raw_iec_to_modbus: {e}") + return None + + +# ============================================================================= +# LEGACY FUNCTIONS (kept for backward compatibility) +# ============================================================================= +# These functions perform conversion inside the mutex-protected section. +# For new code, prefer using the optimized functions above. +# ============================================================================= + + +def update_iec_buffer_from_modbus_data( # pylint: disable=too-many-locals + sba, iec_addr, modbus_data: list, length: int +): """ Updates IEC buffers with data read from Modbus. Assumes mutex is already acquired. @@ -176,17 +537,22 @@ def update_iec_buffer_from_modbus_data(sba, iec_addr, modbus_data: list, length: # Write boolean value if buffer_type == "bool_input": - success, msg = sba.write_bool_input(current_buffer_idx, actual_bit_idx, - current_data, thread_safe=False) + success, msg = sba.write_bool_input( + current_buffer_idx, actual_bit_idx, current_data, thread_safe=False + ) elif buffer_type == "bool_output": - success, msg = sba.write_bool_output(current_buffer_idx, actual_bit_idx, - current_data, thread_safe=False) + success, msg = sba.write_bool_output( + current_buffer_idx, actual_bit_idx, current_data, thread_safe=False + ) else: print(f"Unexpected boolean buffer type: {buffer_type}") continue if not success: - print(f"(FAIL) Failed to write boolean at buffer {current_buffer_idx}, bit {actual_bit_idx}: {msg}") + print( + f"(FAIL) Failed to write boolean at buffer " + f"{current_buffer_idx}, bit {actual_bit_idx}: {msg}" + ) else: # For non-boolean operations, handle register conversion @@ -204,10 +570,14 @@ def update_iec_buffer_from_modbus_data(sba, iec_addr, modbus_data: list, length: try: if iec_size in ["B", "W"]: # For B and W, direct conversion (no multi-register) - current_data = convert_modbus_registers_to_iec_value(element_registers, iec_size) + current_data = convert_modbus_registers_to_iec_value( + element_registers, iec_size + ) elif iec_size in ["D", "L"]: # For D and L, combine multiple registers - current_data = convert_modbus_registers_to_iec_value(element_registers, iec_size, use_big_endian=False) + current_data = convert_modbus_registers_to_iec_value( + element_registers, iec_size, use_big_endian=False + ) else: print(f"Unsupported IEC size: {iec_size}") continue @@ -219,39 +589,65 @@ def update_iec_buffer_from_modbus_data(sba, iec_addr, modbus_data: list, length: # Write the value using the appropriate method if buffer_type == "byte_input": - success, msg = sba.write_byte_input(current_buffer_idx, current_data, thread_safe=False) + success, msg = sba.write_byte_input( + current_buffer_idx, current_data, thread_safe=False + ) elif buffer_type == "byte_output": - success, msg = sba.write_byte_output(current_buffer_idx, current_data, thread_safe=False) + success, msg = sba.write_byte_output( + current_buffer_idx, current_data, thread_safe=False + ) elif buffer_type == "int_input": - success, msg = sba.write_int_input(current_buffer_idx, current_data, thread_safe=False) + success, msg = sba.write_int_input( + current_buffer_idx, current_data, thread_safe=False + ) elif buffer_type == "int_output": - success, msg = sba.write_int_output(current_buffer_idx, current_data, thread_safe=False) + success, msg = sba.write_int_output( + current_buffer_idx, current_data, thread_safe=False + ) elif buffer_type == "int_memory": - success, msg = sba.write_int_memory(current_buffer_idx, current_data, thread_safe=False) + success, msg = sba.write_int_memory( + current_buffer_idx, current_data, thread_safe=False + ) elif buffer_type == "dint_input": - success, msg = sba.write_dint_input(current_buffer_idx, current_data, thread_safe=False) + success, msg = sba.write_dint_input( + current_buffer_idx, current_data, thread_safe=False + ) elif buffer_type == "dint_output": - success, msg = sba.write_dint_output(current_buffer_idx, current_data, thread_safe=False) + success, msg = sba.write_dint_output( + current_buffer_idx, current_data, thread_safe=False + ) elif buffer_type == "dint_memory": - success, msg = sba.write_dint_memory(current_buffer_idx, current_data, thread_safe=False) + success, msg = sba.write_dint_memory( + current_buffer_idx, current_data, thread_safe=False + ) elif buffer_type == "lint_input": - success, msg = sba.write_lint_input(current_buffer_idx, current_data, thread_safe=False) + success, msg = sba.write_lint_input( + current_buffer_idx, current_data, thread_safe=False + ) elif buffer_type == "lint_output": - success, msg = sba.write_lint_output(current_buffer_idx, current_data, thread_safe=False) + success, msg = sba.write_lint_output( + current_buffer_idx, current_data, thread_safe=False + ) elif buffer_type == "lint_memory": - success, msg = sba.write_lint_memory(current_buffer_idx, current_data, thread_safe=False) + success, msg = sba.write_lint_memory( + current_buffer_idx, current_data, thread_safe=False + ) else: print(f"Unknown buffer type: {buffer_type}") continue if not success: - print(f"(FAIL) Failed to write {buffer_type} at index {current_buffer_idx}: {msg}") + print( + f"(FAIL) Failed to write {buffer_type} at index {current_buffer_idx}: {msg}" + ) except Exception as e: print(f"(FAIL) Error updating IEC buffer: {e}") -def read_data_for_modbus_write(sba, iec_addr, length: int) -> Optional[list]: +def read_data_for_modbus_write( # pylint: disable=too-many-locals + sba, iec_addr, length: int +) -> Optional[list]: """ Reads data from IEC buffers for Modbus write operations. Assumes mutex is already acquired. @@ -292,15 +688,22 @@ def read_data_for_modbus_write(sba, iec_addr, length: int) -> Optional[list]: # Read boolean value if buffer_type == "bool_input": - value, msg = sba.read_bool_input(current_buffer_idx, actual_bit_idx, thread_safe=False) + value, msg = sba.read_bool_input( + current_buffer_idx, actual_bit_idx, thread_safe=False + ) elif buffer_type == "bool_output": - value, msg = sba.read_bool_output(current_buffer_idx, actual_bit_idx, thread_safe=False) + value, msg = sba.read_bool_output( + current_buffer_idx, actual_bit_idx, thread_safe=False + ) else: print(f"Unexpected boolean buffer type: {buffer_type}") return None if msg != "Success": - print(f"(FAIL) Failed to read boolean at buffer {current_buffer_idx}, bit {actual_bit_idx}: {msg}") + print( + f"(FAIL) Failed to read boolean at buffer " + f"{current_buffer_idx}, bit {actual_bit_idx}: {msg}" + ) return None values.append(value) @@ -337,7 +740,9 @@ def read_data_for_modbus_write(sba, iec_addr, length: int) -> Optional[list]: return None if msg != "Success": - print(f"(FAIL) Failed to read {buffer_type} at index {current_buffer_idx}: {msg}") + print( + f"(FAIL) Failed to read {buffer_type} at index {current_buffer_idx}: {msg}" + ) return None # Convert IEC value to Modbus registers @@ -347,7 +752,9 @@ def read_data_for_modbus_write(sba, iec_addr, length: int) -> Optional[list]: element_registers = convert_iec_value_to_modbus_registers(value, iec_size) elif iec_size in ["D", "L"]: # For D and L, split into multiple registers - element_registers = convert_iec_value_to_modbus_registers(value, iec_size, use_big_endian=False) + element_registers = convert_iec_value_to_modbus_registers( + value, iec_size, use_big_endian=False + ) else: print(f"Unsupported IEC size: {iec_size}") return None @@ -364,4 +771,3 @@ def read_data_for_modbus_write(sba, iec_addr, length: int) -> Optional[list]: except Exception as e: print(f"(FAIL) Error reading data for Modbus write: {e}") return None - diff --git a/core/src/drivers/plugins/python/modbus_master/modbus_master_plugin.py b/core/src/drivers/plugins/python/modbus_master/modbus_master_plugin.py index 2c24c457..484306fa 100644 --- a/core/src/drivers/plugins/python/modbus_master/modbus_master_plugin.py +++ b/core/src/drivers/plugins/python/modbus_master/modbus_master_plugin.py @@ -30,9 +30,11 @@ try: # Try relative imports first (when used as package) from .modbus_master_connection import ModbusConnectionManager - from .modbus_master_memory import ( - read_data_for_modbus_write, - update_iec_buffer_from_modbus_data, + from .modbus_master_memory import ( # Optimized functions for minimal mutex hold time + convert_modbus_data_to_iec_values, + convert_raw_iec_to_modbus, + read_raw_iec_values, + write_preconverted_iec_values, ) from .modbus_master_utils import ( calculate_gcd_of_cycle_times, @@ -42,9 +44,11 @@ except ImportError: # Fallback to absolute imports (when run standalone) from modbus_master_connection import ModbusConnectionManager - from modbus_master_memory import ( - read_data_for_modbus_write, - update_iec_buffer_from_modbus_data, + from modbus_master_memory import ( # Optimized functions for minimal mutex hold time + convert_modbus_data_to_iec_values, + convert_raw_iec_to_modbus, + read_raw_iec_values, + write_preconverted_iec_values, ) from modbus_master_utils import ( calculate_gcd_of_cycle_times, @@ -215,60 +219,102 @@ def run(self): # pylint: disable=too-many-locals self.connection_manager.mark_disconnected() # Batch update IEC buffers with single mutex acquisition + # OPTIMIZATION: Pre-convert all Modbus data to IEC values BEFORE + # acquiring the mutex to minimize mutex hold time if read_results_to_update: - lock_acquired, lock_msg = self.sba.acquire_mutex() - if lock_acquired: - try: - for iec_addr, modbus_data, length in read_results_to_update: - update_iec_buffer_from_modbus_data( - self.sba, iec_addr, modbus_data, length - ) - finally: - self.sba.release_mutex() - else: - print( - f"[{self.name}] (FAIL) Failed to acquire mutex " - f"for read updates: {lock_msg}" + # Phase 1: Pre-convert all data outside the mutex + preconverted_updates = [] + for iec_addr, modbus_data, length in read_results_to_update: + converted_values, details = convert_modbus_data_to_iec_values( + iec_addr, modbus_data, length ) + if converted_values is not None and details is not None: + preconverted_updates.append((converted_values, details)) + + # Phase 2: Write pre-converted values under the mutex + if preconverted_updates: + lock_acquired, lock_msg = self.sba.acquire_mutex() + if lock_acquired: + try: + for converted_values, details in preconverted_updates: + write_preconverted_iec_values( + self.sba, converted_values, details + ) + finally: + self.sba.release_mutex() + else: + print( + f"[{self.name}] (FAIL) Failed to acquire mutex " + f"for read updates: {lock_msg}" + ) # 2. WRITE OPERATIONS - Process only I/O points that are due for polling this cycle + # OPTIMIZATION: Batch all write preparations under a single mutex acquisition + # to minimize mutex hold time. Read all raw IEC values at once, then convert + # and write to Modbus outside the mutex. + + # Phase 1: Collect all write points that are due this cycle + write_points_due = [] for point in io_points: if self._stop_event.is_set(): break - # Skip if this point doesn't need to be polled this cycle if point.fc not in [5, 6, 15, 16]: # Write functions continue - # Check if point should be polled this cycle point_cycle_multiple = point.cycle_time_ms // self.gcd_cycle_time_ms if (cycle_counter % point_cycle_multiple) != 0: continue try: - # Parse Modbus offset address = parse_modbus_offset(point.offset) + write_points_due.append((point, address)) + except ValueError as ve: + print( + f"[{self.name}] (FAIL) Invalid offset " + f"'{point.offset}' for FC {point.fc}: {ve}" + ) - # Read data from IEC buffers (with mutex) - lock_acquired, lock_msg = self.sba.acquire_mutex() - if not lock_acquired: - print( - f"[{self.name}] (FAIL) Failed to acquire mutex " - f"for write prep (FC {point.fc}, " - f"offset {point.offset}): {lock_msg}" - ) - continue - + # Phase 2: Read all raw IEC values under a single mutex acquisition + raw_iec_data = [] # List of (point, address, raw_values, details, iec_size) + if write_points_due: + lock_acquired, lock_msg = self.sba.acquire_mutex() + if lock_acquired: try: - values_to_write = read_data_for_modbus_write( - self.sba, point.iec_location, point.length - ) + for point, address in write_points_due: + raw_values, details, iec_size = read_raw_iec_values( + self.sba, point.iec_location, point.length + ) + if raw_values is not None: + raw_iec_data.append( + (point, address, raw_values, details, iec_size) + ) + else: + print( + f"[{self.name}] (FAIL) Failed to read raw IEC data " + f"for write (FC {point.fc}, offset {point.offset})" + ) finally: self.sba.release_mutex() + else: + print( + f"[{self.name}] (FAIL) Failed to acquire mutex " + f"for batch write prep: {lock_msg}" + ) + + # Phase 3: Convert raw IEC values to Modbus format (outside mutex) + # and perform Modbus writes + for point, address, raw_values, details, iec_size in raw_iec_data: + if self._stop_event.is_set(): + break + + try: + # Convert raw IEC values to Modbus format (outside mutex) + values_to_write = convert_raw_iec_to_modbus(raw_values, details, iec_size) if values_to_write is None: print( - f"[{self.name}] (FAIL) Failed to read data " + f"[{self.name}] (FAIL) Failed to convert data " f"for Modbus write (FC {point.fc}, " f"offset {point.offset})" ) @@ -315,34 +361,25 @@ def run(self): # pylint: disable=too-many-locals f"[{self.name}] (FAIL) Modbus write error " f"(FC {point.fc}, addr {address}): {response}" ) - # Mark as disconnected to force reconnection on next cycle self.connection_manager.mark_disconnected() elif response.isError(): print( f"[{self.name}] (FAIL) Modbus write failed " f"(FC {point.fc}, addr {address}): {response}" ) - # Mark as disconnected to force reconnection on next cycle self.connection_manager.mark_disconnected() - except ValueError as ve: - print( - f"[{self.name}] (FAIL) Invalid offset " - f"'{point.offset}' for FC {point.fc}: {ve}" - ) except ConnectionException as ce: print( f"[{self.name}] (FAIL) Connection error writing " f"FC {point.fc}, offset {point.offset}: {ce}" ) - # Mark as disconnected to force reconnection self.connection_manager.mark_disconnected() except Exception as e: print( f"[{self.name}] (FAIL) Error writing " f"FC {point.fc}, offset {point.offset}: {e}" ) - # For other errors also mark disconnected as precaution self.connection_manager.mark_disconnected() # 3. CYCLE TIMING - Sleep for GCD cycle time From 0d8218575525c5978ea618751615a75435284f73 Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Tue, 9 Dec 2025 00:08:03 +0000 Subject: [PATCH 02/17] Add logging for failed data conversions in read operations Addresses Copilot review comment: Log when convert_modbus_data_to_iec_values() returns None to aid debugging. Uses a concise message with IEC address and length without dumping the full modbus_data list to avoid log noise. Co-Authored-By: Thiago Alves --- .../plugins/python/modbus_master/modbus_master_plugin.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/core/src/drivers/plugins/python/modbus_master/modbus_master_plugin.py b/core/src/drivers/plugins/python/modbus_master/modbus_master_plugin.py index 484306fa..3c07f47a 100644 --- a/core/src/drivers/plugins/python/modbus_master/modbus_master_plugin.py +++ b/core/src/drivers/plugins/python/modbus_master/modbus_master_plugin.py @@ -230,6 +230,11 @@ def run(self): # pylint: disable=too-many-locals ) if converted_values is not None and details is not None: preconverted_updates.append((converted_values, details)) + else: + print( + f"[{self.name}] (FAIL) Data conversion failed " + f"for IEC address {iec_addr}, length={length}" + ) # Phase 2: Write pre-converted values under the mutex if preconverted_updates: From 5bbaa33a74697cb1a3216b526c3a655be8b547f7 Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Tue, 9 Dec 2025 20:57:22 +0000 Subject: [PATCH 03/17] Add missing write_bool_input method to SafeBufferAccess Co-Authored-By: Thiago Alves --- .../python/shared/safe_buffer_access_refactored.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/core/src/drivers/plugins/python/shared/safe_buffer_access_refactored.py b/core/src/drivers/plugins/python/shared/safe_buffer_access_refactored.py index e9b4a72b..e9abefd1 100644 --- a/core/src/drivers/plugins/python/shared/safe_buffer_access_refactored.py +++ b/core/src/drivers/plugins/python/shared/safe_buffer_access_refactored.py @@ -110,6 +110,14 @@ def read_bool_output( """Read a boolean output value.""" return self.buffer_accessor.read_buffer("bool_output", buffer_idx, bit_idx, thread_safe) + def write_bool_input( + self, buffer_idx: int, bit_idx: int, value: bool, thread_safe: bool = True + ) -> Tuple[bool, str]: + """Write a boolean input value (used by Modbus master to update PLC inputs).""" + return self.buffer_accessor.write_buffer( + "bool_input", buffer_idx, value, bit_idx, thread_safe + ) + def write_bool_output( self, buffer_idx: int, bit_idx: int, value: bool, thread_safe: bool = True ) -> Tuple[bool, str]: From 2b1a81b05d5690fe5df998a20a97228c3313bbd7 Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Wed, 10 Dec 2025 01:22:56 +0000 Subject: [PATCH 04/17] Add Python Function Blocks support This commit adds runtime support for Python Function Blocks, enabling PLC programs to include function blocks written in Python that communicate with the runtime via shared memory. Changes: - Add core/src/plc_app/include/iec_python.h: Header declaring Python FB loader functions (create_shm_name, python_block_loader) - Add core/src/plc_app/python_loader.c: Implementation ported from v3, adapted to use v4's logging API (log_info, log_error) - Update scripts/compile.sh: Include Python header in generated code compilation and link python_loader with pthread and rt libraries The Python FB loader creates shared memory regions for input/output data exchange and spawns Python processes that run the user's function block code. This matches the behavior of OpenPLC Runtime v3. Co-Authored-By: Thiago Alves --- core/src/plc_app/include/iec_python.h | 74 ++++++++++ core/src/plc_app/python_loader.c | 196 ++++++++++++++++++++++++++ scripts/compile.sh | 23 +-- 3 files changed, 284 insertions(+), 9 deletions(-) create mode 100644 core/src/plc_app/include/iec_python.h create mode 100644 core/src/plc_app/python_loader.c diff --git a/core/src/plc_app/include/iec_python.h b/core/src/plc_app/include/iec_python.h new file mode 100644 index 00000000..7a2ae06d --- /dev/null +++ b/core/src/plc_app/include/iec_python.h @@ -0,0 +1,74 @@ +//----------------------------------------------------------------------------- +// Copyright 2025 Thiago Alves +// This file is part of the OpenPLC Runtime. +// +// OpenPLC is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// OpenPLC is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with OpenPLC. If not, see . +//------ +// +// This header declares the Python Function Block loader functions. +// These functions are used by the generated PLC code to load and execute +// Python function blocks via shared memory communication. +// +// Thiago Alves, Dec 2025 +//----------------------------------------------------------------------------- + +#ifndef IEC_PYTHON_H +#define IEC_PYTHON_H + +#include +#include + +#ifdef __cplusplus +extern "C" +{ +#endif + + /** + * @brief Create a unique shared memory name + * + * Creates a unique name for shared memory regions using mkstemp. + * The name is suitable for use with shm_open(). + * + * @param buf Buffer to store the generated name + * @param size Size of the buffer + * @return 0 on success, -1 on failure + */ + int create_shm_name(char *buf, size_t size); + + /** + * @brief Load and start a Python function block + * + * Writes the Python script to disk, creates shared memory regions for + * input/output data exchange, and spawns a Python process to execute + * the function block. + * + * @param script_name Path where the Python script will be written + * @param script_content The Python script content (with format specifiers for pid and shm_name) + * @param shm_name Base name for shared memory regions + * @param shm_in_size Size of the input shared memory region + * @param shm_out_size Size of the output shared memory region + * @param shm_in_ptr Pointer to store the mapped input shared memory address + * @param shm_out_ptr Pointer to store the mapped output shared memory address + * @param pid PLC process ID (passed to Python script for monitoring) + * @return 0 on success, -1 on failure + */ + int python_block_loader(const char *script_name, const char *script_content, char *shm_name, + size_t shm_in_size, size_t shm_out_size, void **shm_in_ptr, + void **shm_out_ptr, pid_t pid); + +#ifdef __cplusplus +} +#endif + +#endif /* IEC_PYTHON_H */ diff --git a/core/src/plc_app/python_loader.c b/core/src/plc_app/python_loader.c new file mode 100644 index 00000000..e1db7b6f --- /dev/null +++ b/core/src/plc_app/python_loader.c @@ -0,0 +1,196 @@ +//----------------------------------------------------------------------------- +// Copyright 2025 Thiago Alves +// This file is part of the OpenPLC Runtime. +// +// OpenPLC is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// OpenPLC is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with OpenPLC. If not, see . +//------ +// +// This file is responsible for loading function blocks written in Python. +// Python function blocks communicate with the PLC runtime via shared memory. +// +// Thiago Alves, Dec 2025 +//----------------------------------------------------------------------------- + +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include "include/iec_python.h" +#include "utils/log.h" + +/** + * @brief Thread function that runs the Python script and logs its output + * + * This function is spawned as a detached thread to run the Python interpreter + * and capture its stdout/stderr output for logging. + * + * @param arg The command string to execute (will be freed by this function) + * @return NULL + */ +static void *runner_thread(void *arg) +{ + const char *cmd = (const char *)arg; + FILE *fp = popen(cmd, "r"); + if (fp == NULL) + { + log_error("[Python] Failed to start process: %s", cmd); + free((void *)cmd); + return NULL; + } + + char buffer[512]; + while (fgets(buffer, sizeof(buffer), fp) != NULL) + { + // Remove trailing newline if present + size_t len = strlen(buffer); + if (len > 0 && buffer[len - 1] == '\n') + { + buffer[len - 1] = '\0'; + } + log_info("[Python] %s", buffer); + } + + pclose(fp); + free((void *)cmd); + return NULL; +} + +int create_shm_name(char *buf, size_t size) +{ + char shm_mask[] = "/tmp/shmXXXXXXXXXXXX"; + int fd = mkstemp(shm_mask); + if (fd == -1) + { + log_error("[Python loader] mkstemp failed: %s", strerror(errno)); + return -1; + } + close(fd); + + snprintf(buf, size, "/%s", strrchr(shm_mask, '/') + 1); + unlink(shm_mask); + + return 0; +} + +int python_block_loader(const char *script_name, const char *script_content, char *shm_name, + size_t shm_in_size, size_t shm_out_size, void **shm_in_ptr, + void **shm_out_ptr, pid_t pid) +{ + char shm_in_name[256]; + char shm_out_name[256]; + + // Write the Python script to disk + FILE *fp = fopen(script_name, "w"); + if (!fp) + { + log_error("[Python loader] Failed to write Python script: %s", strerror(errno)); + return -1; + } + chmod(script_name, 0640); + + log_info("[Python loader] Random shared memory location: %s", shm_name); + + snprintf(shm_in_name, sizeof(shm_in_name), "%s_in", shm_name); + snprintf(shm_out_name, sizeof(shm_out_name), "%s_out", shm_name); + + // Write script content with format specifiers replaced + fprintf(fp, script_content, pid, shm_name, shm_name); + fflush(fp); + fsync(fileno(fp)); + fclose(fp); + + // Map shared memory for inputs + int shm_in_fd = shm_open(shm_in_name, O_CREAT | O_RDWR, 0660); + if (shm_in_fd < 0) + { + log_error("[Python loader] shm_open (input) error: %s", strerror(errno)); + return -1; + } + if (ftruncate(shm_in_fd, shm_in_size) == -1) + { + log_error("[Python loader] ftruncate (input) error: %s", strerror(errno)); + close(shm_in_fd); + return -1; + } + *shm_in_ptr = mmap(NULL, shm_in_size, PROT_READ | PROT_WRITE, MAP_SHARED, shm_in_fd, 0); + if (*shm_in_ptr == MAP_FAILED) + { + log_error("[Python loader] mmap (input) error: %s", strerror(errno)); + close(shm_in_fd); + return -1; + } + + // Map shared memory for outputs + int shm_out_fd = shm_open(shm_out_name, O_CREAT | O_RDWR, 0660); + if (shm_out_fd < 0) + { + log_error("[Python loader] shm_open (output) error: %s", strerror(errno)); + close(shm_in_fd); + munmap(*shm_in_ptr, shm_in_size); + shm_unlink(shm_in_name); + return -1; + } + if (ftruncate(shm_out_fd, shm_out_size) == -1) + { + log_error("[Python loader] ftruncate (output) error: %s", strerror(errno)); + close(shm_out_fd); + close(shm_in_fd); + munmap(*shm_in_ptr, shm_in_size); + shm_unlink(shm_in_name); + return -1; + } + *shm_out_ptr = mmap(NULL, shm_out_size, PROT_READ | PROT_WRITE, MAP_SHARED, shm_out_fd, 0); + if (*shm_out_ptr == MAP_FAILED) + { + log_error("[Python loader] mmap (output) error: %s", strerror(errno)); + close(shm_out_fd); + close(shm_in_fd); + munmap(*shm_in_ptr, shm_in_size); + shm_unlink(shm_in_name); + return -1; + } + + // Close file descriptors (mapping remains valid) + close(shm_in_fd); + close(shm_out_fd); + + // Prepare command to run Python script + char *cmd = malloc(512); + if (cmd == NULL) + { + log_error("[Python loader] malloc failed for cmd buffer"); + return -1; + } + snprintf(cmd, 512, "python3 -u %s 2>&1", script_name); + + // Spawn thread to run Python process + pthread_t tid; + if (pthread_create(&tid, NULL, runner_thread, cmd) != 0) + { + log_error("[Python loader] pthread_create failed: %s", strerror(errno)); + free(cmd); + return -1; + } + pthread_detach(tid); + + log_info("[Python loader] Started Python function block: %s", script_name); + + return 0; +} diff --git a/scripts/compile.sh b/scripts/compile.sh index 60317d42..ef732e24 100755 --- a/scripts/compile.sh +++ b/scripts/compile.sh @@ -6,12 +6,14 @@ ROOT="core/generated" LIB_PATH="$ROOT/lib" SRC_PATH="$ROOT" BUILD_PATH="build" +PYTHON_INCLUDE_PATH="core/src/plc_app/include" +PYTHON_LOADER_SRC="core/src/plc_app/python_loader.c" FLAGS="-w -O3 -fPIC" check_required_files() { local missing_files=() - + if [ ! -f "$SRC_PATH/Config0.c" ]; then missing_files+=("$SRC_PATH/Config0.c") fi @@ -27,7 +29,7 @@ check_required_files() { if [ ! -d "$LIB_PATH" ]; then missing_files+=("$LIB_PATH (directory)") fi - + if [ ${#missing_files[@]} -ne 0 ]; then echo "[ERROR] Missing required source files:" >&2 printf ' %s\n' "${missing_files[@]}" >&2 @@ -46,17 +48,20 @@ fi # Compile objects into build/ echo "[INFO] Compiling Config0.c..." -gcc $FLAGS -I "$LIB_PATH" -c "$SRC_PATH/Config0.c" -o "$BUILD_PATH/Config0.o" +gcc $FLAGS -I "$LIB_PATH" -I "$PYTHON_INCLUDE_PATH" -include iec_python.h -c "$SRC_PATH/Config0.c" -o "$BUILD_PATH/Config0.o" echo "[INFO] Compiling Res0.c..." -gcc $FLAGS -I "$LIB_PATH" -c "$SRC_PATH/Res0.c" -o "$BUILD_PATH/Res0.o" +gcc $FLAGS -I "$LIB_PATH" -I "$PYTHON_INCLUDE_PATH" -include iec_python.h -c "$SRC_PATH/Res0.c" -o "$BUILD_PATH/Res0.o" echo "[INFO] Compiling debug.c..." -gcc $FLAGS -I "$LIB_PATH" -c "$SRC_PATH/debug.c" -o "$BUILD_PATH/debug.o" +gcc $FLAGS -I "$LIB_PATH" -c "$SRC_PATH/debug.c" -o "$BUILD_PATH/debug.o" echo "[INFO] Compiling glueVars.c..." -gcc $FLAGS -I "$LIB_PATH" -c "$SRC_PATH/glueVars.c" -o "$BUILD_PATH/glueVars.o" +gcc $FLAGS -I "$LIB_PATH" -c "$SRC_PATH/glueVars.c" -o "$BUILD_PATH/glueVars.o" echo "[INFO] Compiling c_blocks_code.cpp..." -g++ $FLAGS -I "$LIB_PATH" -c "$SRC_PATH/c_blocks_code.cpp" -o "$BUILD_PATH/c_blocks_code.o" +g++ $FLAGS -I "$LIB_PATH" -c "$SRC_PATH/c_blocks_code.cpp" -o "$BUILD_PATH/c_blocks_code.o" +echo "[INFO] Compiling python_loader.c..." +gcc $FLAGS -I "core/src/plc_app" -c "$PYTHON_LOADER_SRC" -o "$BUILD_PATH/python_loader.o" # Link shared library into build/ -echo "[INFO] Compiling shared library..." +echo "[INFO] Linking shared library..." g++ $FLAGS -shared -o "$BUILD_PATH/new_libplc.so" "$BUILD_PATH/Config0.o" \ - "$BUILD_PATH/Res0.o" "$BUILD_PATH/debug.o" "$BUILD_PATH/glueVars.o" "$BUILD_PATH/c_blocks_code.o" + "$BUILD_PATH/Res0.o" "$BUILD_PATH/debug.o" "$BUILD_PATH/glueVars.o" \ + "$BUILD_PATH/c_blocks_code.o" "$BUILD_PATH/python_loader.o" -lpthread -lrt From c1a76884848b345d76b977003526ef2285e83704 Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Wed, 10 Dec 2025 13:01:25 +0000 Subject: [PATCH 05/17] Fix logging accessibility: use function pointers instead of direct calls The python_loader.c was calling log_info() and log_error() directly, but these functions are defined in plc_main, not in libplc.so. Since plc_main is not linked with -rdynamic, these symbols would not be resolved at runtime. This commit implements Option B (function pointers/callbacks) to fix the issue: - Add static function pointers for logging in python_loader.c - Add fallback logging to stderr when loggers are not set - Add python_loader_set_loggers() function to inject logging callbacks - Update iec_python.h with the setter function declaration - Update symbols_init() in image_tables.c to wire up logging callbacks after loading libplc.so This approach matches the existing plugin system pattern where logging functions are passed via callbacks rather than relying on symbol export. Co-Authored-By: Thiago Alves --- core/src/plc_app/image_tables.c | 11 ++++ core/src/plc_app/include/iec_python.h | 13 ++++ core/src/plc_app/python_loader.c | 88 ++++++++++++++++++++++----- 3 files changed, 97 insertions(+), 15 deletions(-) diff --git a/core/src/plc_app/image_tables.c b/core/src/plc_app/image_tables.c index 14a41bcc..d67834df 100644 --- a/core/src/plc_app/image_tables.c +++ b/core/src/plc_app/image_tables.c @@ -2,6 +2,7 @@ #include #include "image_tables.h" +#include "include/iec_python.h" #include "log.h" #include "utils.h" @@ -102,6 +103,16 @@ int symbols_init(PluginManager *pm) dint_input, dint_output, lint_input, lint_output, int_memory, dint_memory, lint_memory); + // Initialize Python loader logging callbacks (optional - only present if Python FBs are used) + void (*ext_python_loader_set_loggers)(void (*)(const char *, ...), void (*)(const char *, ...)); + *(void **)(&ext_python_loader_set_loggers) = + plugin_manager_get_func(pm, void (*)(unsigned long), "python_loader_set_loggers"); + if (ext_python_loader_set_loggers) + { + ext_python_loader_set_loggers(log_info, log_error); + log_info("Python loader logging callbacks initialized"); + } + return 0; } diff --git a/core/src/plc_app/include/iec_python.h b/core/src/plc_app/include/iec_python.h index 7a2ae06d..8afca3bb 100644 --- a/core/src/plc_app/include/iec_python.h +++ b/core/src/plc_app/include/iec_python.h @@ -67,6 +67,19 @@ extern "C" size_t shm_in_size, size_t shm_out_size, void **shm_in_ptr, void **shm_out_ptr, pid_t pid); + /** + * @brief Set logging function pointers for the Python loader + * + * This function must be called after loading libplc.so to inject the + * runtime's logging functions. Without this, logging will fall back + * to stderr output. + * + * @param log_info_func Pointer to the log_info function + * @param log_error_func Pointer to the log_error function + */ + void python_loader_set_loggers(void (*log_info_func)(const char *, ...), + void (*log_error_func)(const char *, ...)); + #ifdef __cplusplus } #endif diff --git a/core/src/plc_app/python_loader.c b/core/src/plc_app/python_loader.c index e1db7b6f..dc625751 100644 --- a/core/src/plc_app/python_loader.c +++ b/core/src/plc_app/python_loader.c @@ -19,12 +19,17 @@ // This file is responsible for loading function blocks written in Python. // Python function blocks communicate with the PLC runtime via shared memory. // +// Logging is done via function pointers that are set by the runtime after +// loading libplc.so. This avoids symbol resolution issues between the +// shared library and the main executable. +// // Thiago Alves, Dec 2025 //----------------------------------------------------------------------------- #include #include #include +#include #include #include #include @@ -33,7 +38,60 @@ #include #include "include/iec_python.h" -#include "utils/log.h" + +// Function pointers for logging - set by python_loader_set_loggers() +static void (*py_log_info)(const char *fmt, ...) = NULL; +static void (*py_log_error)(const char *fmt, ...) = NULL; + +// Fallback logging to stderr when loggers not set +static void fallback_log(const char *level, const char *fmt, va_list args) +{ + fprintf(stderr, "[%s] ", level); + vfprintf(stderr, fmt, args); + fprintf(stderr, "\n"); +} + +static void py_log_info_fallback(const char *fmt, ...) +{ + va_list args; + va_start(args, fmt); + fallback_log("INFO", fmt, args); + va_end(args); +} + +static void py_log_error_fallback(const char *fmt, ...) +{ + va_list args; + va_start(args, fmt); + fallback_log("ERROR", fmt, args); + va_end(args); +} + +// Wrapper macros for logging +#define LOG_INFO(...) \ + do \ + { \ + if (py_log_info) \ + py_log_info(__VA_ARGS__); \ + else \ + py_log_info_fallback(__VA_ARGS__); \ + } while (0) + +#define LOG_ERROR(...) \ + do \ + { \ + if (py_log_error) \ + py_log_error(__VA_ARGS__); \ + else \ + py_log_error_fallback(__VA_ARGS__); \ + } while (0) + +void python_loader_set_loggers(void (*log_info_func)(const char *, ...), + void (*log_error_func)(const char *, ...)) +{ + py_log_info = log_info_func; + py_log_error = log_error_func; +} /** * @brief Thread function that runs the Python script and logs its output @@ -50,7 +108,7 @@ static void *runner_thread(void *arg) FILE *fp = popen(cmd, "r"); if (fp == NULL) { - log_error("[Python] Failed to start process: %s", cmd); + LOG_ERROR("[Python] Failed to start process: %s", cmd); free((void *)cmd); return NULL; } @@ -64,7 +122,7 @@ static void *runner_thread(void *arg) { buffer[len - 1] = '\0'; } - log_info("[Python] %s", buffer); + LOG_INFO("[Python] %s", buffer); } pclose(fp); @@ -78,7 +136,7 @@ int create_shm_name(char *buf, size_t size) int fd = mkstemp(shm_mask); if (fd == -1) { - log_error("[Python loader] mkstemp failed: %s", strerror(errno)); + LOG_ERROR("[Python loader] mkstemp failed: %s", strerror(errno)); return -1; } close(fd); @@ -100,12 +158,12 @@ int python_block_loader(const char *script_name, const char *script_content, cha FILE *fp = fopen(script_name, "w"); if (!fp) { - log_error("[Python loader] Failed to write Python script: %s", strerror(errno)); + LOG_ERROR("[Python loader] Failed to write Python script: %s", strerror(errno)); return -1; } chmod(script_name, 0640); - log_info("[Python loader] Random shared memory location: %s", shm_name); + LOG_INFO("[Python loader] Random shared memory location: %s", shm_name); snprintf(shm_in_name, sizeof(shm_in_name), "%s_in", shm_name); snprintf(shm_out_name, sizeof(shm_out_name), "%s_out", shm_name); @@ -120,19 +178,19 @@ int python_block_loader(const char *script_name, const char *script_content, cha int shm_in_fd = shm_open(shm_in_name, O_CREAT | O_RDWR, 0660); if (shm_in_fd < 0) { - log_error("[Python loader] shm_open (input) error: %s", strerror(errno)); + LOG_ERROR("[Python loader] shm_open (input) error: %s", strerror(errno)); return -1; } if (ftruncate(shm_in_fd, shm_in_size) == -1) { - log_error("[Python loader] ftruncate (input) error: %s", strerror(errno)); + LOG_ERROR("[Python loader] ftruncate (input) error: %s", strerror(errno)); close(shm_in_fd); return -1; } *shm_in_ptr = mmap(NULL, shm_in_size, PROT_READ | PROT_WRITE, MAP_SHARED, shm_in_fd, 0); if (*shm_in_ptr == MAP_FAILED) { - log_error("[Python loader] mmap (input) error: %s", strerror(errno)); + LOG_ERROR("[Python loader] mmap (input) error: %s", strerror(errno)); close(shm_in_fd); return -1; } @@ -141,7 +199,7 @@ int python_block_loader(const char *script_name, const char *script_content, cha int shm_out_fd = shm_open(shm_out_name, O_CREAT | O_RDWR, 0660); if (shm_out_fd < 0) { - log_error("[Python loader] shm_open (output) error: %s", strerror(errno)); + LOG_ERROR("[Python loader] shm_open (output) error: %s", strerror(errno)); close(shm_in_fd); munmap(*shm_in_ptr, shm_in_size); shm_unlink(shm_in_name); @@ -149,7 +207,7 @@ int python_block_loader(const char *script_name, const char *script_content, cha } if (ftruncate(shm_out_fd, shm_out_size) == -1) { - log_error("[Python loader] ftruncate (output) error: %s", strerror(errno)); + LOG_ERROR("[Python loader] ftruncate (output) error: %s", strerror(errno)); close(shm_out_fd); close(shm_in_fd); munmap(*shm_in_ptr, shm_in_size); @@ -159,7 +217,7 @@ int python_block_loader(const char *script_name, const char *script_content, cha *shm_out_ptr = mmap(NULL, shm_out_size, PROT_READ | PROT_WRITE, MAP_SHARED, shm_out_fd, 0); if (*shm_out_ptr == MAP_FAILED) { - log_error("[Python loader] mmap (output) error: %s", strerror(errno)); + LOG_ERROR("[Python loader] mmap (output) error: %s", strerror(errno)); close(shm_out_fd); close(shm_in_fd); munmap(*shm_in_ptr, shm_in_size); @@ -175,7 +233,7 @@ int python_block_loader(const char *script_name, const char *script_content, cha char *cmd = malloc(512); if (cmd == NULL) { - log_error("[Python loader] malloc failed for cmd buffer"); + LOG_ERROR("[Python loader] malloc failed for cmd buffer"); return -1; } snprintf(cmd, 512, "python3 -u %s 2>&1", script_name); @@ -184,13 +242,13 @@ int python_block_loader(const char *script_name, const char *script_content, cha pthread_t tid; if (pthread_create(&tid, NULL, runner_thread, cmd) != 0) { - log_error("[Python loader] pthread_create failed: %s", strerror(errno)); + LOG_ERROR("[Python loader] pthread_create failed: %s", strerror(errno)); free(cmd); return -1; } pthread_detach(tid); - log_info("[Python loader] Started Python function block: %s", script_name); + LOG_INFO("[Python loader] Started Python function block: %s", script_name); return 0; } From f72489eef0441eb5880b75f22ead868f814f43ec Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Wed, 10 Dec 2025 17:45:03 +0000 Subject: [PATCH 06/17] Simplify python_loader logging: remove fallback mechanism Per review feedback, since python_loader.c is always compiled into libplc.so and symbols_init() always runs before any Python FB code executes, the logging callbacks will always be set. Therefore, the fallback logging to stderr is unnecessary. Changes: - Remove fallback_log, py_log_info_fallback, py_log_error_fallback functions - Remove NULL checks in LOG_INFO/LOG_ERROR macros - Simplify macros to directly call the function pointers - Remove stdarg.h include (no longer needed) Co-Authored-By: Thiago Alves --- core/src/plc_app/python_loader.c | 51 ++++---------------------------- 1 file changed, 6 insertions(+), 45 deletions(-) diff --git a/core/src/plc_app/python_loader.c b/core/src/plc_app/python_loader.c index dc625751..1883a7b5 100644 --- a/core/src/plc_app/python_loader.c +++ b/core/src/plc_app/python_loader.c @@ -29,7 +29,6 @@ #include #include #include -#include #include #include #include @@ -40,51 +39,13 @@ #include "include/iec_python.h" // Function pointers for logging - set by python_loader_set_loggers() -static void (*py_log_info)(const char *fmt, ...) = NULL; -static void (*py_log_error)(const char *fmt, ...) = NULL; +// These are always initialized by symbols_init() before any Python FB code runs +static void (*py_log_info)(const char *fmt, ...); +static void (*py_log_error)(const char *fmt, ...); -// Fallback logging to stderr when loggers not set -static void fallback_log(const char *level, const char *fmt, va_list args) -{ - fprintf(stderr, "[%s] ", level); - vfprintf(stderr, fmt, args); - fprintf(stderr, "\n"); -} - -static void py_log_info_fallback(const char *fmt, ...) -{ - va_list args; - va_start(args, fmt); - fallback_log("INFO", fmt, args); - va_end(args); -} - -static void py_log_error_fallback(const char *fmt, ...) -{ - va_list args; - va_start(args, fmt); - fallback_log("ERROR", fmt, args); - va_end(args); -} - -// Wrapper macros for logging -#define LOG_INFO(...) \ - do \ - { \ - if (py_log_info) \ - py_log_info(__VA_ARGS__); \ - else \ - py_log_info_fallback(__VA_ARGS__); \ - } while (0) - -#define LOG_ERROR(...) \ - do \ - { \ - if (py_log_error) \ - py_log_error(__VA_ARGS__); \ - else \ - py_log_error_fallback(__VA_ARGS__); \ - } while (0) +// Simple wrapper macros for logging +#define LOG_INFO(...) py_log_info(__VA_ARGS__) +#define LOG_ERROR(...) py_log_error(__VA_ARGS__) void python_loader_set_loggers(void (*log_info_func)(const char *, ...), void (*log_error_func)(const char *, ...)) From 2b7c536500e2eaebdb6f5325e37fd3db630282d7 Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Thu, 11 Dec 2025 16:46:04 +0000 Subject: [PATCH 07/17] Add memory locking for deterministic PLC execution - Add lock_memory() function that calls mlockall(MCL_CURRENT | MCL_FUTURE) - Call lock_memory() after set_realtime_priority() in plc_cycle_thread() - Add sys/mman.h include for mlockall support This prevents the kernel from swapping out memory pages during PLC execution, eliminating unpredictable latency spikes caused by page faults in the scan cycle. Requires container to have memlock ulimit set to unlimited (--ulimit memlock=-1). Co-Authored-By: Thiago Alves --- core/src/plc_app/plc_state_manager.c | 3 ++- core/src/plc_app/utils/utils.c | 14 ++++++++++++++ core/src/plc_app/utils/utils.h | 9 +++++++++ 3 files changed, 25 insertions(+), 1 deletion(-) diff --git a/core/src/plc_app/plc_state_manager.c b/core/src/plc_app/plc_state_manager.c index d43b4837..3982be6a 100644 --- a/core/src/plc_app/plc_state_manager.c +++ b/core/src/plc_app/plc_state_manager.c @@ -23,8 +23,9 @@ void *plc_cycle_thread(void *arg) { PluginManager *pm = (PluginManager *)arg; - // Initialize PLC + // Initialize PLC with real-time optimizations set_realtime_priority(); + lock_memory(); symbols_init(pm); ext_config_init__(); ext_glueVars(); diff --git a/core/src/plc_app/utils/utils.c b/core/src/plc_app/utils/utils.c index 9ea41d69..d01586cc 100644 --- a/core/src/plc_app/utils/utils.c +++ b/core/src/plc_app/utils/utils.c @@ -2,6 +2,7 @@ #include #include #include +#include #include unsigned long long *ext_common_ticktime__ = NULL; @@ -55,6 +56,19 @@ void set_realtime_priority(void) } } +// Lock all memory pages to prevent page faults during PLC execution +void lock_memory(void) +{ + if (mlockall(MCL_CURRENT | MCL_FUTURE) != 0) + { + log_error("mlockall failed: %s", strerror(errno)); + } + else + { + log_info("Memory locked successfully (MCL_CURRENT | MCL_FUTURE)"); + } +} + size_t parse_hex_string(const char *hex_string, uint8_t *data) { size_t count = 0; diff --git a/core/src/plc_app/utils/utils.h b/core/src/plc_app/utils/utils.h index 5e04384a..c3c8e88a 100644 --- a/core/src/plc_app/utils/utils.h +++ b/core/src/plc_app/utils/utils.h @@ -3,6 +3,7 @@ #include #include +#include #include #include @@ -42,6 +43,14 @@ void timespec_diff(struct timespec *a, struct timespec *b, */ void set_realtime_priority(void); +/** + * @brief Lock all current and future memory pages to prevent page faults + * + * This prevents the kernel from swapping out memory pages during PLC execution, + * which could cause unpredictable latency spikes in the scan cycle. + */ +void lock_memory(void); + /** * @brief Parse a hex string into a byte array * From a1dd3bdafa24aa6604d8d2d0791d4f790677a61a Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Wed, 17 Dec 2025 12:57:25 +0000 Subject: [PATCH 08/17] Move timing stats from logs to status response - Remove print_stats_thread that was polluting logs every 5 seconds - Add STATS command to unix socket for fetching timing statistics - Add thread-safe mutex protection for plc_timing_stats access - Add get_timing_stats_snapshot() for safe concurrent reads - Add format_timing_stats_response() to format stats as JSON - Add stats_plc() method to RuntimeManager - Modify handle_status() to include timing_stats in response - Maintain backward compatibility: status field unchanged for old editors - Fix missing return statement in stop_plc exception handler - Disable R0902 and R1732 pylint warnings in pre-commit config Co-Authored-By: Thiago Alves --- .pre-commit-config.yaml | 2 +- core/src/plc_app/plc_main.c | 46 +---------- core/src/plc_app/scan_cycle_manager.c | 110 +++++++++++++++++++++----- core/src/plc_app/scan_cycle_manager.h | 11 ++- core/src/plc_app/unix_socket.c | 6 ++ webserver/app.py | 38 ++++++++- webserver/runtimemanager.py | 36 +++++---- 7 files changed, 163 insertions(+), 86 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 7ba3f4de..a4d74cba 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -5,7 +5,7 @@ repos: - id: pylint # args: [--fail-on=F,E] args: [ - "--disable=C0114,C0411,C0115,C0116,C0412,E0401,W0718,R1702,R0911,R0912,R0915,R1705,W0404,W0603,W0613,W0511", + "--disable=C0114,C0411,C0115,C0116,C0412,E0401,W0718,R1702,R0911,R0912,R0915,R1705,W0404,W0603,W0613,W0511,R0902,R1732", ] # example: disable missing-docstring additional_dependencies: [flask, flask-sqlalchemy, flask_jwt_extended, werkzeug] diff --git a/core/src/plc_app/plc_main.c b/core/src/plc_app/plc_main.c index 6fcaac7f..646e1ebf 100644 --- a/core/src/plc_app/plc_main.c +++ b/core/src/plc_app/plc_main.c @@ -21,8 +21,7 @@ extern PLCState plc_state; volatile sig_atomic_t keep_running = 1; -extern plc_timing_stats_t plc_timing_stats; -plugin_driver_t *plugin_driver = NULL; +plugin_driver_t *plugin_driver = NULL; extern bool print_logs; void handle_sigint(int sig) @@ -31,40 +30,6 @@ void handle_sigint(int sig) keep_running = 0; } -void *print_stats_thread(void *arg) -{ - (void)arg; - while (keep_running) - { - /* - if (bool_output[0][0]) - { - log_debug("bool_output[0][0]: %d", *bool_output[0][0]); - } - else - { - log_debug("bool_output[0][0] is NULL"); - } - */ - - log_info("Scan Count: %lu", plc_timing_stats.scan_count); - log_info("Scan Time - Min: %ld us, Max: %ld us, Avg: %ld us", - plc_timing_stats.scan_time_min, plc_timing_stats.scan_time_max, - plc_timing_stats.scan_time_avg); - log_info("Cycle Time - Min: %lu us, Max: %lu us, Avg: %ld us", - plc_timing_stats.cycle_time_min, plc_timing_stats.cycle_time_max, - plc_timing_stats.cycle_time_avg); - log_info("Cycle Latency - Min: %ld us, Max: %ld us, Avg: %ld us", - plc_timing_stats.cycle_latency_min, plc_timing_stats.cycle_latency_max, - plc_timing_stats.cycle_latency_avg); - log_info("Overruns: %lu", plc_timing_stats.overruns); - - // Print every 5 seconds - sleep(5); - } - return NULL; -} - int main(int argc, char *argv[]) { // Check for --print-logs argument @@ -110,14 +75,6 @@ int main(int argc, char *argv[]) return -1; } - // Launch status printing thread - pthread_t stats_thread; - if (pthread_create(&stats_thread, NULL, print_stats_thread, NULL) != 0) - { - log_error("Failed to create stats thread"); - return -1; - } - // Start PLC if (plc_set_state(PLC_STATE_RUNNING) != true) { @@ -158,6 +115,5 @@ int main(int argc, char *argv[]) // Cleanup log_info("Shutting down..."); plc_state_manager_cleanup(); - pthread_join(stats_thread, NULL); return 0; } diff --git a/core/src/plc_app/scan_cycle_manager.c b/core/src/plc_app/scan_cycle_manager.c index e52c8aef..db8bb88e 100644 --- a/core/src/plc_app/scan_cycle_manager.c +++ b/core/src/plc_app/scan_cycle_manager.c @@ -1,23 +1,23 @@ -#include +#include #include #include +#include +#include #include "scan_cycle_manager.h" #include "utils/utils.h" -static uint64_t expected_start_us = 0; -static uint64_t last_start_us = 0; +static uint64_t expected_start_us = 0; +static uint64_t last_start_us = 0; +static pthread_mutex_t stats_mutex = PTHREAD_MUTEX_INITIALIZER; -plc_timing_stats_t plc_timing_stats = -{ - .scan_time_min = INT64_MAX, - .cycle_latency_min = INT64_MAX, - .cycle_time_avg = 0, - .cycle_time_min = INT64_MAX, - .cycle_latency_avg = 0, - .scan_count = 0, - .overruns = 0 -}; +plc_timing_stats_t plc_timing_stats = {.scan_time_min = INT64_MAX, + .cycle_latency_min = INT64_MAX, + .cycle_time_avg = 0, + .cycle_time_min = INT64_MAX, + .cycle_latency_avg = 0, + .scan_count = 0, + .overruns = 0}; static uint64_t ts_now_us(void) { @@ -26,18 +26,20 @@ static uint64_t ts_now_us(void) return (uint64_t)ts.tv_sec * 1000000ull + ts.tv_nsec / 1000; } - -void scan_cycle_time_start() +void scan_cycle_time_start() { uint64_t now_us = ts_now_us(); + pthread_mutex_lock(&stats_mutex); + if (plc_timing_stats.scan_count == 0) { // Ignore full calculations for the first cycle expected_start_us = now_us + *ext_common_ticktime__ / 1000; // Convert ns to us - last_start_us = now_us; + last_start_us = now_us; plc_timing_stats.scan_count++; + pthread_mutex_unlock(&stats_mutex); return; } @@ -51,7 +53,8 @@ void scan_cycle_time_start() { plc_timing_stats.cycle_time_max = cycle_time_us; } - plc_timing_stats.cycle_time_avg += (cycle_time_us - plc_timing_stats.cycle_time_avg) / plc_timing_stats.scan_count; + plc_timing_stats.cycle_time_avg += + (cycle_time_us - plc_timing_stats.cycle_time_avg) / plc_timing_stats.scan_count; // Calculate cycle latency int64_t latency_us = (int64_t)(now_us - expected_start_us); @@ -63,18 +66,23 @@ void scan_cycle_time_start() { plc_timing_stats.cycle_latency_max = latency_us; } - plc_timing_stats.cycle_latency_avg += (latency_us - plc_timing_stats.cycle_latency_avg) / plc_timing_stats.scan_count; + plc_timing_stats.cycle_latency_avg += + (latency_us - plc_timing_stats.cycle_latency_avg) / plc_timing_stats.scan_count; last_start_us = now_us; expected_start_us += *ext_common_ticktime__ / 1000; // Convert ns to us plc_timing_stats.scan_count++; + + pthread_mutex_unlock(&stats_mutex); } -void scan_cycle_time_end() +void scan_cycle_time_end() { uint64_t now_us = ts_now_us(); + pthread_mutex_lock(&stats_mutex); + // Calculate scan time int64_t scan_time_us = now_us - last_start_us; if (scan_time_us < plc_timing_stats.scan_time_min) @@ -85,11 +93,71 @@ void scan_cycle_time_end() { plc_timing_stats.scan_time_max = scan_time_us; } - plc_timing_stats.scan_time_avg += (scan_time_us - plc_timing_stats.scan_time_avg) / plc_timing_stats.scan_count; + plc_timing_stats.scan_time_avg += + (scan_time_us - plc_timing_stats.scan_time_avg) / plc_timing_stats.scan_count; // Check for overrun if (now_us > expected_start_us) { plc_timing_stats.overruns++; } -} \ No newline at end of file + + pthread_mutex_unlock(&stats_mutex); +} + +bool get_timing_stats_snapshot(plc_timing_stats_t *snapshot) +{ + if (snapshot == NULL) + { + return false; + } + + pthread_mutex_lock(&stats_mutex); + memcpy(snapshot, &plc_timing_stats, sizeof(plc_timing_stats_t)); + pthread_mutex_unlock(&stats_mutex); + + return snapshot->scan_count > 0; +} + +int format_timing_stats_response(char *buffer, size_t buffer_size) +{ + plc_timing_stats_t snapshot; + bool valid = get_timing_stats_snapshot(&snapshot); + + if (!valid) + { + return snprintf(buffer, buffer_size, + "STATS:{" + "\"scan_count\":0," + "\"scan_time_min\":null," + "\"scan_time_max\":null," + "\"scan_time_avg\":null," + "\"cycle_time_min\":null," + "\"cycle_time_max\":null," + "\"cycle_time_avg\":null," + "\"cycle_latency_min\":null," + "\"cycle_latency_max\":null," + "\"cycle_latency_avg\":null," + "\"overruns\":0" + "}\n"); + } + + return snprintf(buffer, buffer_size, + "STATS:{" + "\"scan_count\":%ld," + "\"scan_time_min\":%ld," + "\"scan_time_max\":%ld," + "\"scan_time_avg\":%ld," + "\"cycle_time_min\":%ld," + "\"cycle_time_max\":%ld," + "\"cycle_time_avg\":%ld," + "\"cycle_latency_min\":%ld," + "\"cycle_latency_max\":%ld," + "\"cycle_latency_avg\":%ld," + "\"overruns\":%ld" + "}\n", + snapshot.scan_count, snapshot.scan_time_min, snapshot.scan_time_max, + snapshot.scan_time_avg, snapshot.cycle_time_min, snapshot.cycle_time_max, + snapshot.cycle_time_avg, snapshot.cycle_latency_min, snapshot.cycle_latency_max, + snapshot.cycle_latency_avg, snapshot.overruns); +} diff --git a/core/src/plc_app/scan_cycle_manager.h b/core/src/plc_app/scan_cycle_manager.h index 5d78b791..c624fe2e 100644 --- a/core/src/plc_app/scan_cycle_manager.h +++ b/core/src/plc_app/scan_cycle_manager.h @@ -1,6 +1,7 @@ #ifndef SCAN_CYCLE_MANAGER_H #define SCAN_CYCLE_MANAGER_H +#include #include typedef struct @@ -24,4 +25,12 @@ typedef struct void scan_cycle_time_start(); void scan_cycle_time_end(); -#endif // SCAN_CYCLE_MANAGER_H \ No newline at end of file +// Thread-safe function to get a snapshot of timing stats +// Returns true if stats are valid (scan_count > 0), false otherwise +bool get_timing_stats_snapshot(plc_timing_stats_t *snapshot); + +// Format timing stats as a response string for the STATS command +// Returns the number of characters written (excluding null terminator) +int format_timing_stats_response(char *buffer, size_t buffer_size); + +#endif // SCAN_CYCLE_MANAGER_H diff --git a/core/src/plc_app/unix_socket.c b/core/src/plc_app/unix_socket.c index 8505c031..e9b00968 100644 --- a/core/src/plc_app/unix_socket.c +++ b/core/src/plc_app/unix_socket.c @@ -11,6 +11,7 @@ #include "debug_handler.h" #include "plc_state_manager.h" +#include "scan_cycle_manager.h" #include "unix_socket.h" #include "utils/log.h" #include "utils/utils.h" @@ -94,6 +95,11 @@ void handle_unix_socket_commands(const char *command, char *response, size_t res log_error("Received START command but PLC is already RUNNING"); } } + else if (strcmp(command, "STATS") == 0) + { + log_debug("Received STATS command"); + format_timing_stats_response(response, response_size); + } else if (strncmp(command, "DEBUG:", 6) == 0) { log_debug("Received DEBUG command"); diff --git a/webserver/app.py b/webserver/app.py index 95ec2db4..8d48707f 100644 --- a/webserver/app.py +++ b/webserver/app.py @@ -1,14 +1,17 @@ +import json import os import shutil import ssl import threading from pathlib import Path -from typing import Callable, Final +from typing import Callable, Final, Optional import flask import flask_login + from webserver.credentials import CertGen from webserver.debug_websocket import init_debug_websocket +from webserver.logger import get_logger from webserver.plcapp_management import ( MAX_FILE_SIZE, BuildStatus, @@ -26,7 +29,6 @@ restapi_bp, ) from webserver.runtimemanager import RuntimeManager -from webserver.logger import get_logger, LogParser logger, _ = get_logger("logger", use_buffer=True) @@ -80,11 +82,41 @@ def handle_compilation_status(data: dict) -> dict: } +def parse_timing_stats(stats_response: Optional[str]) -> Optional[dict]: + """ + Parse the STATS response from the runtime. + Expected format: STATS:{json_object} + Returns the parsed JSON object or None if parsing fails. + """ + if stats_response is None: + return None + + # Remove the STATS: prefix + if stats_response.startswith("STATS:"): + json_str = stats_response[6:].strip() + else: + return None + + try: + return json.loads(json_str) + except json.JSONDecodeError: + return None + + def handle_status(data: dict) -> dict: response = runtime_manager.status_plc() if response is None: return {"status": "No response from runtime"} - return {"status": response} + + result: dict = {"status": response} + + # Fetch timing stats and include them in the response + stats_response = runtime_manager.stats_plc() + timing_stats = parse_timing_stats(stats_response) + if timing_stats is not None: + result["timing_stats"] = timing_stats + + return result def handle_ping(data: dict) -> dict: diff --git a/webserver/runtimemanager.py b/webserver/runtimemanager.py index 979cb7b3..bcee5a32 100644 --- a/webserver/runtimemanager.py +++ b/webserver/runtimemanager.py @@ -5,9 +5,10 @@ import time import psutil -from webserver.unixserver import UnixLogServer + +from webserver.logger import get_logger from webserver.unixclient import SyncUnixClient -from webserver.logger import get_logger, LogParser +from webserver.unixserver import UnixLogServer logger, buffer = get_logger("logger", use_buffer=True) @@ -31,17 +32,13 @@ def find_running_process(self): for proc in psutil.process_iter(["pid", "exe", "cmdline"]): try: # First try to match by executable path (most reliable) - if proc.info["exe"] and os.path.samefile( - proc.info["exe"], self.runtime_path - ): + if proc.info["exe"] and os.path.samefile(proc.info["exe"], self.runtime_path): return proc # Alternatively, match by command line (fallback) cmdline = proc.info.get("cmdline") if cmdline and isinstance(cmdline, (list, tuple)) and len(cmdline) > 0: - cmdline_str = " ".join( - str(arg) for arg in cmdline if arg is not None - ) + cmdline_str = " ".join(str(arg) for arg in cmdline if arg is not None) if self.runtime_path in cmdline_str: return proc @@ -110,9 +107,7 @@ def start(self): # Start runtime process if not already running running_process = self.find_running_process() if running_process: - logger.info( - "Found existing PLC runtime process with PID %d", running_process.pid - ) + logger.info("Found existing PLC runtime process with PID %d", running_process.pid) self.process = running_process self._safe_start_log_server() self._safe_connect_runtime_socket() @@ -139,10 +134,7 @@ def is_runtime_alive(self): if self.process is None: return False if isinstance(self.process, psutil.Process): - if ( - self.process.is_running() - and self.process.status() != psutil.STATUS_ZOMBIE - ): + if self.process.is_running() and self.process.status() != psutil.STATUS_ZOMBIE: return True elif isinstance(self.process, subprocess.Popen): if self.process.poll() is None: @@ -255,6 +247,7 @@ def stop_plc(self): return "STOP:ERROR\n" except Exception as e: logger.error("Failed to stop PLC runtime (unexpected): %s", e) + return "STOP:ERROR\n" def status_plc(self): """ @@ -268,3 +261,16 @@ def status_plc(self): except Exception as e: logger.error("Failed to get PLC status (unexpected): %s", e) return "STATUS:ERROR\n" + + def stats_plc(self): + """ + Send STATS command to get timing statistics + """ + try: + return self.runtime_socket.send_and_receive("STATS\n") + except (OSError, socket.error) as e: + logger.error("Failed to get PLC stats: %s", e) + return None + except Exception as e: + logger.error("Failed to get PLC stats (unexpected): %s", e) + return None From 8a73e0f4d5e834e9064caf92bc1594998c93e7dc Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Wed, 17 Dec 2025 13:10:33 +0000 Subject: [PATCH 09/17] Fix int64_t format specifiers for cross-platform compatibility Use PRId64 from inttypes.h instead of %ld for portable int64_t formatting. This fixes build failures on 32-bit ARM platforms where int64_t is 'long long int' rather than 'long int'. Co-Authored-By: Thiago Alves --- core/src/plc_app/scan_cycle_manager.c | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/core/src/plc_app/scan_cycle_manager.c b/core/src/plc_app/scan_cycle_manager.c index db8bb88e..78ef6534 100644 --- a/core/src/plc_app/scan_cycle_manager.c +++ b/core/src/plc_app/scan_cycle_manager.c @@ -1,3 +1,4 @@ +#include #include #include #include @@ -144,18 +145,17 @@ int format_timing_stats_response(char *buffer, size_t buffer_size) return snprintf(buffer, buffer_size, "STATS:{" - "\"scan_count\":%ld," - "\"scan_time_min\":%ld," - "\"scan_time_max\":%ld," - "\"scan_time_avg\":%ld," - "\"cycle_time_min\":%ld," - "\"cycle_time_max\":%ld," - "\"cycle_time_avg\":%ld," - "\"cycle_latency_min\":%ld," - "\"cycle_latency_max\":%ld," - "\"cycle_latency_avg\":%ld," - "\"overruns\":%ld" - "}\n", + "\"scan_count\":%" PRId64 "," + "\"scan_time_min\":%" PRId64 "," + "\"scan_time_max\":%" PRId64 "," + "\"scan_time_avg\":%" PRId64 "," + "\"cycle_time_min\":%" PRId64 "," + "\"cycle_time_max\":%" PRId64 "," + "\"cycle_time_avg\":%" PRId64 "," + "\"cycle_latency_min\":%" PRId64 "," + "\"cycle_latency_max\":%" PRId64 "," + "\"cycle_latency_avg\":%" PRId64 "," + "\"overruns\":%" PRId64 "}\n", snapshot.scan_count, snapshot.scan_time_min, snapshot.scan_time_max, snapshot.scan_time_avg, snapshot.cycle_time_min, snapshot.cycle_time_max, snapshot.cycle_time_avg, snapshot.cycle_latency_min, snapshot.cycle_latency_max, From 0201de50ab3cd0ee0fec460711b642101dc292f1 Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Wed, 17 Dec 2025 22:46:14 +0000 Subject: [PATCH 10/17] Add X-OpenPLC-Runtime-Version header to all API responses This adds an after_request hook to the REST API blueprint that includes the X-OpenPLC-Runtime-Version header with value 'v4' in all API responses. This enables the OpenPLC Editor to detect which runtime version it is connecting to and show an error if there is a version mismatch between the selected target and the actual runtime. Co-Authored-By: Thiago Alves --- webserver/restapi.py | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/webserver/restapi.py b/webserver/restapi.py index 179ad448..c77b040f 100644 --- a/webserver/restapi.py +++ b/webserver/restapi.py @@ -1,7 +1,6 @@ import os from typing import Callable, Optional -import webserver.config from flask import Blueprint, Flask, jsonify, request from flask_jwt_extended import ( JWTManager, @@ -12,7 +11,9 @@ ) from flask_sqlalchemy import SQLAlchemy from werkzeug.security import check_password_hash, generate_password_hash -from webserver.logger import get_logger, LogParser + +import webserver.config +from webserver.logger import get_logger logger, buffer = get_logger("logger", use_buffer=True) @@ -28,6 +29,15 @@ restapi_bp = Blueprint("restapi_blueprint", __name__) _handler_callback_get: Optional[Callable[[str, dict], dict]] = None _handler_callback_post: Optional[Callable[[str, dict], dict]] = None + + +@restapi_bp.after_request +def add_runtime_version_header(response): + """Add runtime version header to all API responses for version detection.""" + response.headers["X-OpenPLC-Runtime-Version"] = "v4" + return response + + jwt = JWTManager(app_restapi) db = SQLAlchemy(app_restapi) @@ -45,6 +55,7 @@ def check_if_token_revoked(jwt_header, jwt_payload): logger.error("Error revoking JWT: %s", e) return False + class User(db.Model): # type: ignore[name-defined] __tablename__ = "users" @@ -58,9 +69,7 @@ class User(db.Model): # type: ignore[name-defined] def set_password(self, password: str) -> str: password = password + app_restapi.config["PEPPER"] - self.password_hash = generate_password_hash( - password, method=self.derivation_method - ) + self.password_hash = generate_password_hash(password, method=self.derivation_method) return self.password_hash def check_password(self, password: str) -> bool: @@ -154,7 +163,7 @@ def get_users_info(): # ) try: users_exist = User.query.first() is not None - except Exception as e: + except Exception: # logger.error("Error checking for users: %s", e) return jsonify({"msg": "User retrieval error"}), 500 From 1598b8dd67e828f755e075e7065f0c0ee177dd2a Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Fri, 19 Dec 2025 16:50:47 +0000 Subject: [PATCH 11/17] Add include_stats parameter to status endpoint and reduce log verbosity - Add optional include_stats parameter to /api/status endpoint (default false) - Only fetch timing stats when include_stats=true, avoiding mutex acquisition - Remove verbose command logging from unix_socket.c (only log unrecognized commands) - This reduces log flooding and avoids unnecessary mutex contention on the critical PLC scan cycle when stats are not needed Co-Authored-By: Thiago Alves --- core/src/plc_app/unix_socket.c | 8 -------- webserver/app.py | 14 +++++++++----- 2 files changed, 9 insertions(+), 13 deletions(-) diff --git a/core/src/plc_app/unix_socket.c b/core/src/plc_app/unix_socket.c index e9b00968..7c8663b5 100644 --- a/core/src/plc_app/unix_socket.c +++ b/core/src/plc_app/unix_socket.c @@ -45,12 +45,10 @@ void handle_unix_socket_commands(const char *command, char *response, size_t res { if (strcmp(command, "PING") == 0) { - log_debug("Received PING command"); strncpy(response, "PING:OK\n", response_size); } else if (strcmp(command, "STATUS") == 0) { - log_debug("Received STATUS command"); PLCState current_state = plc_get_state(); if (current_state == PLC_STATE_INIT) @@ -68,7 +66,6 @@ void handle_unix_socket_commands(const char *command, char *response, size_t res } else if (strcmp(command, "STOP") == 0) { - log_debug("Received STOP command"); if (plc_set_state(PLC_STATE_STOPPED)) strncpy(response, "STOP:OK\n", response_size); else @@ -76,7 +73,6 @@ void handle_unix_socket_commands(const char *command, char *response, size_t res } else if (strcmp(command, "START") == 0) { - log_debug("Received START command"); PLCState current_state = plc_get_state(); if (current_state != PLC_STATE_RUNNING) { @@ -97,12 +93,10 @@ void handle_unix_socket_commands(const char *command, char *response, size_t res } else if (strcmp(command, "STATS") == 0) { - log_debug("Received STATS command"); format_timing_stats_response(response, response_size); } else if (strncmp(command, "DEBUG:", 6) == 0) { - log_debug("Received DEBUG command"); uint8_t debug_data[4096] = {0}; size_t data_length = parse_hex_string(&command[6], debug_data); if (data_length > 0) @@ -181,8 +175,6 @@ void *unix_socket_thread(void *arg) ssize_t bytes_read = read_line(client_fd, command_buffer, COMMAND_BUFFER_SIZE); if (bytes_read > 0) { - log_debug("Received command: %s", command_buffer); - // Handle the command char response[MAX_RESPONSE_SIZE] = {0}; handle_unix_socket_commands(command_buffer, response, MAX_RESPONSE_SIZE); diff --git a/webserver/app.py b/webserver/app.py index 8d48707f..3948d387 100644 --- a/webserver/app.py +++ b/webserver/app.py @@ -110,11 +110,15 @@ def handle_status(data: dict) -> dict: result: dict = {"status": response} - # Fetch timing stats and include them in the response - stats_response = runtime_manager.stats_plc() - timing_stats = parse_timing_stats(stats_response) - if timing_stats is not None: - result["timing_stats"] = timing_stats + # Only fetch timing stats if explicitly requested via include_stats parameter. + # This avoids acquiring the stats mutex on every status poll, which could + # introduce latency to the critical PLC scan cycle. + include_stats = data.get("include_stats", "").lower() == "true" + if include_stats: + stats_response = runtime_manager.stats_plc() + timing_stats = parse_timing_stats(stats_response) + if timing_stats is not None: + result["timing_stats"] = timing_stats return result From da4c60fa9c40d9a197f223d3584e8968cb270361 Mon Sep 17 00:00:00 2001 From: Marcone Tenorio Date: Fri, 19 Dec 2025 20:33:34 +0100 Subject: [PATCH 12/17] Refactor plugin driver configuration handling and improve plugin restart logic --- core/src/drivers/plugin_driver.c | 102 ++++++++++++++++++++------- core/src/drivers/plugin_driver.h | 1 + core/src/plc_app/plc_state_manager.c | 32 +++++---- 3 files changed, 99 insertions(+), 36 deletions(-) diff --git a/core/src/drivers/plugin_driver.c b/core/src/drivers/plugin_driver.c index 760fbd12..f986c6df 100644 --- a/core/src/drivers/plugin_driver.c +++ b/core/src/drivers/plugin_driver.c @@ -115,7 +115,7 @@ static PyObject *create_python_runtime_args_capsule(plugin_runtime_args_t *args) return capsule; } -int plugin_driver_load_config(plugin_driver_t *driver, const char *config_file) +int plugin_driver_update_config(plugin_driver_t *driver, const char *config_file) { if (!driver || !config_file) { @@ -178,6 +178,17 @@ int plugin_driver_load_config(plugin_driver_t *driver, const char *config_file) has_python_plugin = 1; } } + return 0; +} + +int plugin_driver_load_config(plugin_driver_t *driver, const char *config_file) +{ + if (!driver || !config_file) + { + return -1; + } + + plugin_driver_update_config(driver, config_file); // Now retrieve the function symbols and initialize // struct plugin_instance_t para cada plugin. @@ -191,7 +202,6 @@ int plugin_driver_load_config(plugin_driver_t *driver, const char *config_file) { fprintf(stderr, "Failed to get Python plugin symbols for: %s\n", plugin->config.path); - plugin_manager_destroy(plugin->manager); return -1; } } @@ -217,6 +227,16 @@ int plugin_driver_init(plugin_driver_t *driver) return -1; } + PyGILState_STATE local_gstate; + bool gil_acquired = false; + + // Acquire GIL if we have Python plugins + if (has_python_plugin) + { + local_gstate = PyGILState_Ensure(); + gil_acquired = true; + } + // #chamdo a função init de cada plugin aqui for (int i = 0; i < driver->plugin_count; i++) { @@ -239,6 +259,10 @@ int plugin_driver_init(plugin_driver_t *driver) { fprintf(stderr, "Failed to generate runtime args for plugin: %s\n", plugin->config.name); + if (gil_acquired) + { + PyGILState_Release(local_gstate); + } return -1; } // Call the Python init function with proper capsule @@ -253,6 +277,10 @@ int plugin_driver_init(plugin_driver_t *driver) PyErr_Print(); fprintf(stderr, "Python init function failed for plugin: %s\n", plugin->config.name); + if (gil_acquired) + { + PyGILState_Release(local_gstate); + } return -1; } Py_DECREF(result); @@ -286,6 +314,11 @@ int plugin_driver_init(plugin_driver_t *driver) } } + if (gil_acquired) + { + PyGILState_Release(local_gstate); + } + return 0; } @@ -305,8 +338,8 @@ int plugin_driver_start(plugin_driver_t *driver) if (has_python_plugin) { - main_tstate = PyEval_SaveThread(); gstate = PyGILState_Ensure(); + main_tstate = PyEval_SaveThread(); } for (int i = 0; i < driver->plugin_count; i++) @@ -328,6 +361,8 @@ int plugin_driver_start(plugin_driver_t *driver) // NOTE: The thread is created python-side if (plugin->python_plugin && plugin->python_plugin->pFuncStart) { + // Acquire GIL for this specific Python call + PyGILState_STATE local_gil = PyGILState_Ensure(); PyObject *res = PyObject_CallNoArgs(plugin->python_plugin->pFuncStart); if (!res) { @@ -342,6 +377,7 @@ int plugin_driver_start(plugin_driver_t *driver) Py_DECREF( res); // There's no problem in calling DECREF here because it only // handles the returned object from start_loop, not the function itself + PyGILState_Release(local_gil); plugin->running = 1; } @@ -374,10 +410,8 @@ int plugin_driver_start(plugin_driver_t *driver) break; } } - if (has_python_plugin) - { - PyGILState_Release(gstate); - } + // Don't call PyGILState_Release here since we used PyEval_SaveThread + // The GIL will be restored in plugin_driver_destroy return 0; } @@ -395,6 +429,15 @@ int plugin_driver_stop(plugin_driver_t *driver) return 0; } + PyGILState_STATE local_gstate; + bool gil_acquired = false; + + if (has_python_plugin) + { + local_gstate = PyGILState_Ensure(); + gil_acquired = true; + } + // Signal all plugins to stop for (int i = 0; i < driver->plugin_count; i++) { @@ -404,6 +447,11 @@ int plugin_driver_stop(plugin_driver_t *driver) driver->plugins[i].running) { plugin_instance_t *plugin = &driver->plugins[i]; + if (plugin->config.enabled == 0) + { + printf("[PLUGIN]: Plugin %s is disabled, skipping stop.\n", plugin->config.name); + continue; + } PyObject *res = PyObject_CallNoArgs(driver->plugins[i].python_plugin->pFuncStop); if (!res) @@ -416,7 +464,7 @@ int plugin_driver_stop(plugin_driver_t *driver) printf("[PLUGIN]: Plugin %s stopped successfully.\n", plugin->config.name); } Py_DECREF(res); - + printf("[PLUGIN]: Plugin %s stopped...\n", driver->plugins[i].config.name); plugin->running = 0; } @@ -428,11 +476,14 @@ int plugin_driver_stop(plugin_driver_t *driver) printf("[PLUGIN]: Native plugin %s stopped successfully.\n", plugin->config.name); plugin->running = 0; } - - printf("[PLUGIN]: Plugin %s stopped...\n", driver->plugins[i].config.name); // Plugin manager only handles destruction, not stopping } + if (gil_acquired) + { + PyGILState_Release(local_gstate); + } + return 0; } @@ -453,16 +504,20 @@ int plugin_driver_restart(plugin_driver_t *driver) } // Clean up plugins without destroying the driver - gstate = PyGILState_Ensure(); - for (int i = 0; i < driver->plugin_count; i++) + // Note: No need for GIL here as stop() already handled Python operations + if (has_python_plugin) { - plugin_instance_t *plugin = &driver->plugins[i]; - if (plugin->python_plugin) + gstate = PyGILState_Ensure(); + for (int i = 0; i < driver->plugin_count; i++) { - python_plugin_cleanup(plugin); + plugin_instance_t *plugin = &driver->plugins[i]; + if (plugin->python_plugin) + { + python_plugin_cleanup(plugin); + } } + PyGILState_Release(gstate); } - PyGILState_Release(gstate); // CRITICAL: Reload configuration from plugins.conf file printf("[PLUGIN]: Reloading plugin configuration...\n"); @@ -503,9 +558,13 @@ void plugin_driver_destroy(plugin_driver_t *driver) return; } + PyGILState_STATE local_gstate; + bool gil_acquired = false; + if (has_python_plugin) { - gstate = PyGILState_Ensure(); + local_gstate = PyGILState_Ensure(); + gil_acquired = true; } plugin_driver_stop(driver); @@ -513,11 +572,6 @@ void plugin_driver_destroy(plugin_driver_t *driver) for (int i = 0; i < driver->plugin_count; i++) { plugin_instance_t *plugin = &driver->plugins[i]; - if (plugin->manager) - { - plugin_manager_destroy(plugin->manager); - plugin->manager = NULL; - } if (plugin->python_plugin) { python_plugin_cleanup(plugin); @@ -543,9 +597,9 @@ void plugin_driver_destroy(plugin_driver_t *driver) } } - if (has_python_plugin) + if (gil_acquired) { - PyGILState_Release(gstate); + PyGILState_Release(local_gstate); PyEval_RestoreThread(main_tstate); Py_FinalizeEx(); } diff --git a/core/src/drivers/plugin_driver.h b/core/src/drivers/plugin_driver.h index 99175602..da84babe 100644 --- a/core/src/drivers/plugin_driver.h +++ b/core/src/drivers/plugin_driver.h @@ -97,6 +97,7 @@ typedef struct // Driver management functions plugin_driver_t *plugin_driver_create(void); int plugin_driver_load_config(plugin_driver_t *driver, const char *config_file); +int plugin_driver_update_config(plugin_driver_t *driver, const char *config_file); int plugin_driver_init(plugin_driver_t *driver); int plugin_driver_start(plugin_driver_t *driver); int plugin_driver_stop(plugin_driver_t *driver); diff --git a/core/src/plc_app/plc_state_manager.c b/core/src/plc_app/plc_state_manager.c index 3982be6a..47e5eb46 100644 --- a/core/src/plc_app/plc_state_manager.c +++ b/core/src/plc_app/plc_state_manager.c @@ -97,6 +97,24 @@ int load_plc_program(PluginManager *pm) pthread_mutex_unlock(&state_mutex); log_info("PLC State: INIT"); + // Restart all plugins after successful thread creation + if (plugin_driver) + { + log_info("[PLUGIN]: Plugin driver system created"); + // Load plugin configuration + if (plugin_driver_update_config(plugin_driver, "./plugins.conf") == 0) + { + // Start plugins + plugin_driver_init(plugin_driver); + plugin_driver_start(plugin_driver); + log_info("[PLUGIN]: Plugin driver system initialized"); + } + else + { + log_error("[PLUGIN]: Failed to load plugin configuration"); + } + } + if (pthread_create(&plc_thread, NULL, plc_cycle_thread, pm) != 0) { log_error("Failed to create PLC cycle thread"); @@ -108,18 +126,7 @@ int load_plc_program(PluginManager *pm) return -1; } - - // Restart all plugins after successful thread creation - if (plugin_driver && plugin_driver_restart(plugin_driver) != 0) - { - log_error("Failed to restart plugins after PLC thread creation"); - // Note: We don't return error here as PLC is already running - // This is a warning condition, not a fatal error - } - else - { - log_info("Plugins restarted successfully after PLC thread creation"); - } + return 0; } @@ -151,6 +158,7 @@ int unload_plc_program(PluginManager *pm) // Clear temporary pointers from image tables before unloading // This ensures clean state for the next program load plugin_mutex_take(&plugin_driver->buffer_mutex); + plugin_driver_stop(plugin_driver); image_tables_clear_null_pointers(); plugin_mutex_give(&plugin_driver->buffer_mutex); From 1d55d6c333cea39786c1bff991676ec95886478f Mon Sep 17 00:00:00 2001 From: Marcone Tenorio Date: Fri, 19 Dec 2025 21:12:25 +0100 Subject: [PATCH 13/17] Simplify GIL management in plugin driver initialization and destruction --- core/src/drivers/plugin_driver.c | 68 ++++++++------------------------ 1 file changed, 16 insertions(+), 52 deletions(-) diff --git a/core/src/drivers/plugin_driver.c b/core/src/drivers/plugin_driver.c index f986c6df..9c90e7ba 100644 --- a/core/src/drivers/plugin_driver.c +++ b/core/src/drivers/plugin_driver.c @@ -227,15 +227,7 @@ int plugin_driver_init(plugin_driver_t *driver) return -1; } - PyGILState_STATE local_gstate; - bool gil_acquired = false; - - // Acquire GIL if we have Python plugins - if (has_python_plugin) - { - local_gstate = PyGILState_Ensure(); - gil_acquired = true; - } + PyGILState_STATE local_gstate = PyGILState_Ensure(); // #chamdo a função init de cada plugin aqui for (int i = 0; i < driver->plugin_count; i++) @@ -259,10 +251,8 @@ int plugin_driver_init(plugin_driver_t *driver) { fprintf(stderr, "Failed to generate runtime args for plugin: %s\n", plugin->config.name); - if (gil_acquired) - { - PyGILState_Release(local_gstate); - } + + PyGILState_Release(local_gstate); return -1; } // Call the Python init function with proper capsule @@ -277,10 +267,8 @@ int plugin_driver_init(plugin_driver_t *driver) PyErr_Print(); fprintf(stderr, "Python init function failed for plugin: %s\n", plugin->config.name); - if (gil_acquired) - { - PyGILState_Release(local_gstate); - } + + PyGILState_Release(local_gstate); return -1; } Py_DECREF(result); @@ -314,10 +302,7 @@ int plugin_driver_init(plugin_driver_t *driver) } } - if (gil_acquired) - { - PyGILState_Release(local_gstate); - } + PyGILState_Release(local_gstate); return 0; } @@ -336,11 +321,8 @@ int plugin_driver_start(plugin_driver_t *driver) return 0; } - if (has_python_plugin) - { - gstate = PyGILState_Ensure(); - main_tstate = PyEval_SaveThread(); - } + gstate = PyGILState_Ensure(); + main_tstate = PyEval_SaveThread(); for (int i = 0; i < driver->plugin_count; i++) { @@ -429,14 +411,7 @@ int plugin_driver_stop(plugin_driver_t *driver) return 0; } - PyGILState_STATE local_gstate; - bool gil_acquired = false; - - if (has_python_plugin) - { - local_gstate = PyGILState_Ensure(); - gil_acquired = true; - } + PyGILState_STATE local_gstate = PyGILState_Ensure(); // Signal all plugins to stop for (int i = 0; i < driver->plugin_count; i++) @@ -479,10 +454,7 @@ int plugin_driver_stop(plugin_driver_t *driver) // Plugin manager only handles destruction, not stopping } - if (gil_acquired) - { - PyGILState_Release(local_gstate); - } + PyGILState_Release(local_gstate); return 0; } @@ -558,14 +530,7 @@ void plugin_driver_destroy(plugin_driver_t *driver) return; } - PyGILState_STATE local_gstate; - bool gil_acquired = false; - - if (has_python_plugin) - { - local_gstate = PyGILState_Ensure(); - gil_acquired = true; - } + PyGILState_STATE local_gstate = PyGILState_Ensure(); plugin_driver_stop(driver); @@ -597,12 +562,11 @@ void plugin_driver_destroy(plugin_driver_t *driver) } } - if (gil_acquired) - { - PyGILState_Release(local_gstate); - PyEval_RestoreThread(main_tstate); - Py_FinalizeEx(); - } + + PyGILState_Release(local_gstate); + PyEval_RestoreThread(main_tstate); + Py_FinalizeEx(); + pthread_mutex_destroy(&driver->buffer_mutex); free(driver); From 0ad7803ae6471728e80d0c044f98e175c9b46499 Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Mon, 22 Dec 2025 12:30:22 +0000 Subject: [PATCH 14/17] Update plugin driver documentation to reflect native plugin support - Update README.md to document that native C/C++ plugins are now fully supported - Fix incorrect type comment in plugin_config.h (0=python, 1=native) - Update DEVELOPMENT.md plugin section with accurate lifecycle description - Document native plugin args lifetime (must copy during init) - Update Python import examples to use correct 'from shared import' syntax - Add native plugin examples to See Also section Co-Authored-By: Thiago Alves --- core/src/drivers/README.md | 230 +++++++++++++++++++------------ core/src/drivers/plugin_config.h | 2 +- docs/DEVELOPMENT.md | 14 +- 3 files changed, 154 insertions(+), 92 deletions(-) diff --git a/core/src/drivers/README.md b/core/src/drivers/README.md index c50f2c69..021c88e5 100644 --- a/core/src/drivers/README.md +++ b/core/src/drivers/README.md @@ -1,6 +1,6 @@ # OpenPLC Runtime Plugin System -This directory contains the OpenPLC Runtime plugin system, which allows extending the runtime with custom drivers and communication protocols. **Currently, the system actively supports Python plugins. Support for native C plugins is planned for a future release.** +This directory contains the OpenPLC Runtime plugin system, which allows extending the runtime with custom drivers and communication protocols. The system supports both Python plugins and native C/C++ plugins (compiled shared libraries). ## Overview @@ -8,7 +8,7 @@ The plugin system provides a flexible architecture for integrating external hard **Current Status:** * **Supported:** Python plugins (`.py` files) are fully supported and operational. -* **Planned:** Native C plugins (`.so` files) are part of the design and API but are not yet implemented or functional. All references to native C plugins in this document describe the intended future functionality. +* **Supported:** Native C/C++ plugins (`.so` files) are fully supported and operational. ## Architecture @@ -18,28 +18,33 @@ The plugin system provides a flexible architecture for integrating external hard core/src/drivers/ ├── plugin_driver.c/h # Main plugin driver system ├── plugin_config.c/h # Configuration file parsing -├── python_plugin_bridge.c/h # Python plugin integration +├── python_plugin_bridge.h # Python plugin integration ├── CMakeLists.txt # Build configuration -├── plugins/python/ # Python plugin implementations -│ ├── examples/ # Python plugin examples -│ ├── modbus_slave/ # Modbus TCP slave Python plugin -│ └── shared/ # Shared Python modules (e.g., type definitions) -└── *.py # Standalone Python plugin files (if any) +├── plugins/ +│ ├── python/ # Python plugin implementations +│ │ ├── examples/ # Python plugin examples +│ │ ├── modbus_master/ # Modbus TCP master Python plugin +│ │ ├── modbus_slave/ # Modbus TCP slave Python plugin +│ │ └── shared/ # Shared Python modules (e.g., type definitions) +│ └── native/ # Native C/C++ plugin implementations +│ └── examples/ # Native plugin examples and templates +└── README.md # This documentation ``` ### Plugin Types -1. **Python Plugins** (`PLUGIN_TYPE_PYTHON = 0`) - **Currently Supported** +1. **Python Plugins** (`PLUGIN_TYPE_PYTHON = 0`) - **Supported** * Python scripts (`.py` files). * Embedded Python interpreter. * Easier development and debugging. - * Enhanced type safety and buffer access with `python_plugin_types.py`. + * Enhanced type safety and buffer access with the `shared` module. + * Support for isolated virtual environments per plugin. -2. **Native C Plugins** (`PLUGIN_TYPE_NATIVE = 1`) - **Future Support** +2. **Native C/C++ Plugins** (`PLUGIN_TYPE_NATIVE = 1`) - **Supported** * Compiled shared libraries (`.so` files). - * Direct C function calls. - * Maximum performance (intended). - * *Note: This plugin type is defined in the API but not yet implemented.* + * Direct C function calls via `dlopen`/`dlsym`. + * Maximum performance for time-critical operations. + * No Python interpreter overhead. ## Plugin Interface @@ -73,20 +78,40 @@ def cleanup(): pass ``` -#### Native C Plugins (Future Support - API Defined) +#### Native C/C++ Plugins (Supported) ```c // Mandatory initialization function -int init(plugin_runtime_args_t *args); +// args: pointer to plugin_runtime_args_t structure +// IMPORTANT: The args pointer is freed after init() returns. +// You must copy any data you need to retain during init(). +int init(void *args); // Optional lifecycle functions -void start_loop(void); -void stop_loop(void); -void run_cycle(void); +void start_loop(void); // Called when plugin should start operations +void stop_loop(void); // Called when plugin should stop operations +void cleanup(void); // Called when plugin is being unloaded -// Mandatory cleanup function -void cleanup(void); +// Reserved hooks (symbols loaded but not currently invoked by the runtime) +void cycle_start(void); // Reserved for future per-cycle start hook +void cycle_end(void); // Reserved for future per-cycle end hook +``` + +**Important: Native Plugin Args Lifetime** + +The `plugin_runtime_args_t*` pointer passed to `init()` is freed immediately after the function returns. Native plugins must copy the structure contents (or the specific fields they need) into plugin-owned storage during `init()`. Do not store the pointer itself for later use, as this will result in a use-after-free bug. + +```c +// Example: Properly storing runtime args in a native plugin +static plugin_runtime_args_t g_args; // Plugin-owned copy + +int init(void *args) { + if (!args) return -1; + // Copy the entire structure + memcpy(&g_args, args, sizeof(plugin_runtime_args_t)); + // Now g_args can be safely used in start_loop, stop_loop, etc. + return 0; +} ``` -*Note: The C API is defined but the loading and execution mechanism for native plugins is not yet implemented.* ### Runtime Arguments Structure @@ -108,12 +133,12 @@ typedef struct { IEC_UINT **int_memory; // Internal memory (16-bit) IEC_UDINT **dint_memory; // Internal memory (32-bit) IEC_ULINT **lint_memory; // Internal memory (64-bit) - + // Thread synchronization int (*mutex_take)(pthread_mutex_t *mutex); int (*mutex_give)(pthread_mutex_t *mutex); pthread_mutex_t *buffer_mutex; - + // Buffer metadata int buffer_size; // Number of buffers int bits_per_buffer; // Bits per boolean buffer (typically 8) @@ -124,10 +149,10 @@ typedef struct { ### Enhanced Python SafeBufferAccess (Recommended) -The `plugins/python/shared/python_plugin_types.py` module provides a `SafeBufferAccess` wrapper class for robust and safe buffer operations. This is the recommended way to interact with OpenPLC buffers. +The `plugins/python/shared/` module provides a `SafeBufferAccess` wrapper class for robust and safe buffer operations. This is the recommended way to interact with OpenPLC buffers. ```python -from plugins.python.shared.python_plugin_types import SafeBufferAccess, safe_extract_runtime_args_from_capsule +from shared import SafeBufferAccess, safe_extract_runtime_args_from_capsule def init(runtime_args_capsule): # Safely extract runtime arguments from the PyCapsule @@ -141,7 +166,7 @@ def init(runtime_args_capsule): if not safe_buffer.is_valid: print(f"Failed to create SafeBufferAccess: {safe_buffer.error_msg}") return False - + global safe_access safe_access = safe_buffer return True @@ -193,21 +218,22 @@ def manual_safe_write_output(runtime_args, buffer_idx, bit_pos, value): Plugins are configured via a text file, typically `plugins.conf`, located in the project root. Each line defines a plugin: ``` -# Format: name,path,enabled,type,plugin_related_config_path +# Format: name,path,enabled,type,plugin_related_config_path,venv_path # Example for a Python Modbus Slave plugin: -modbus_slave,./core/src/drivers/plugins/python/modbus_slave/simple_modbus.py,1,0,./core/src/drivers/plugins/python/modbus_slave/modbus_slave_config.json +modbus_slave,./core/src/drivers/plugins/python/modbus_slave/simple_modbus.py,1,0,./core/src/drivers/plugins/python/modbus_slave/modbus_slave_config.json,./venvs/modbus_slave # Example for a custom Python plugin: -my_custom_plugin,./core/src/drivers/plugins/python/examples/my_custom_plugin.py,1,0,./my_custom_plugin_config.ini -# Example for a future Native C plugin (not yet supported): -# future_native_plugin,./plugins/native/example.so,1,1,./config/example.conf +my_custom_plugin,./core/src/drivers/plugins/python/examples/my_custom_plugin.py,1,0,./my_custom_plugin_config.ini,./venvs/my_custom_plugin +# Example for a Native C/C++ plugin: +my_native_plugin,./core/src/drivers/plugins/native/my_plugin.so,1,1,./config/my_native_plugin.conf ``` **Fields:** * `name`: A unique identifier for the plugin. -* `path`: Path to the plugin file (`.py` for Python, `.so` for native C). +* `path`: Path to the plugin file (`.py` for Python, `.so` for native C/C++). * `enabled`: `1` for enabled, `0` for disabled. -* `type`: `0` for Python (`PLUGIN_TYPE_PYTHON`), `1` for Native C (`PLUGIN_TYPE_NATIVE`). **Currently, only `0` is functional.** +* `type`: `0` for Python (`PLUGIN_TYPE_PYTHON`), `1` for Native C/C++ (`PLUGIN_TYPE_NATIVE`). * `plugin_related_config_path`: (Optional) Path to a plugin-specific configuration file (e.g., `.ini`, `.json`, `.conf`). +* `venv_path`: (Optional, Python only) Path to a Python virtual environment for the plugin. ### Loading Configuration in Code The configuration is loaded and managed by the plugin driver: @@ -278,9 +304,9 @@ python3 ./core/src/drivers/plugins/python/modbus_slave/simple_modbus.py ## Python Plugin Type System and Safety -### Enhanced Type Safety with `python_plugin_types.py` +### Enhanced Type Safety with the `shared` Module -The `plugins/python/shared/python_plugin_types.py` module is crucial for developing robust Python plugins. It provides: +The `plugins/python/shared/` package is crucial for developing robust Python plugins. It provides: #### Key Components @@ -293,19 +319,24 @@ The `plugins/python/shared/python_plugin_types.py` module is crucial for develop * Handles `mutex_take`/`mutex_give` automatically. * Provides clear error messages for invalid access attempts (e.g., out of bounds, null pointers). -3. **`PluginStructureValidator`** +3. **`SafeLoggingAccess` Wrapper** + * Provides safe access to the runtime logging functions. + * Supports `log_info`, `log_debug`, `log_warn`, and `log_error` methods. + +4. **`PluginStructureValidator`** * Utilities for debugging, such as `print_structure_info()` to verify `ctypes` structure alignment and sizes against the C definitions. -4. **`safe_extract_runtime_args_from_capsule(runtime_args_capsule)`** +5. **`safe_extract_runtime_args_from_capsule(runtime_args_capsule)`** * The **recommended** function to extract the `plugin_runtime_args_t` pointer from the `PyCapsule` passed to the `init` function. * Performs comprehensive error checking (capsule validity, name, null pointer) and returns a tuple `(runtime_args_ptr, error_message)`. #### Usage Example (Reiterated from SafeBufferAccess) ```python -from plugins.python.shared.python_plugin_types import ( +from shared import ( PluginRuntimeArgs, # For type hinting or direct use if extraction is manual safe_extract_runtime_args_from_capsule, SafeBufferAccess, + SafeLoggingAccess, PluginStructureValidator ) @@ -326,7 +357,7 @@ def init(runtime_args_capsule): if not _safe_buffer_access.is_valid: print(f"[Plugin Error] Failed to initialize SafeBufferAccess: {_safe_buffer_access.error_msg}") return False - + print("Plugin initialized successfully.") return True @@ -340,9 +371,10 @@ def init(runtime_args_capsule): 1. **Create your plugin file**, e.g., `my_driver.py`, in a suitable location like `plugins/python/` or a project-specific subdirectory. ```python #!/usr/bin/env python3 - from plugins.python.shared.python_plugin_types import ( + from shared import ( safe_extract_runtime_args_from_capsule, - SafeBufferAccess + SafeBufferAccess, + SafeLoggingAccess ) _safe_buffer_access = None @@ -350,17 +382,17 @@ def init(runtime_args_capsule): def init(runtime_args_capsule): global _safe_buffer_access print("MyDriver: Initializing...") - + runtime_args, error_msg = safe_extract_runtime_args_from_capsule(runtime_args_capsule) if runtime_args is None: print(f"MyDriver Error: {error_msg}") return False - + _safe_buffer_access = SafeBufferAccess(runtime_args) if not _safe_buffer_access.is_valid: print(f"MyDriver Error: SafeBufferAccess init failed: {_safe_buffer_access.error_msg}") return False - + # Perform other initializations, e.g., loading config, setting up hardware print("MyDriver: Initialized successfully.") return True @@ -407,65 +439,87 @@ def init(runtime_args_capsule): * Run OpenPLC. Check logs for "MyDriver: Initializing..." and "MyDriver: Initialized successfully." * Test the functionality of your driver. -### Creating a Native C Plugin (Future Guide) - -*This section outlines the intended process for when native C plugin support is implemented.* +### Creating a Native C/C++ Plugin 1. **Implement required functions** in a C file (e.g., `my_native_plugin.c`): ```c - #include "plugin_driver.h" // Assuming this header will define the interface #include #include - - static plugin_runtime_args_t *g_runtime_args = NULL; - - int init(plugin_runtime_args_t *args) { - if (!args) return -1; // Error - g_runtime_args = args; - printf("Native C Plugin: Initialized\n"); - // Perform hardware initialization, etc. - return 0; // Success + #include + #include + + // Include IEC types from the OpenPLC runtime + #include "iec_types.h" + + // Define plugin_runtime_args_t structure locally to avoid Python dependencies + typedef struct { + IEC_BOOL *(*bool_input)[8]; + IEC_BOOL *(*bool_output)[8]; + IEC_BYTE **byte_input; + IEC_BYTE **byte_output; + IEC_UINT **int_input; + IEC_UINT **int_output; + IEC_UDINT **dint_input; + IEC_UDINT **dint_output; + IEC_ULINT **lint_input; + IEC_ULINT **lint_output; + IEC_UINT **int_memory; + IEC_UDINT **dint_memory; + IEC_ULINT **lint_memory; + int (*mutex_take)(pthread_mutex_t *mutex); + int (*mutex_give)(pthread_mutex_t *mutex); + pthread_mutex_t *buffer_mutex; + char plugin_specific_config_file_path[256]; + int buffer_size; + int bits_per_buffer; + } plugin_runtime_args_t; + + // IMPORTANT: Copy args during init - the pointer is freed after init returns + static plugin_runtime_args_t g_args; + static int plugin_initialized = 0; + + int init(void *args) { + if (!args) return -1; + // Copy the entire structure - do NOT store the pointer + memcpy(&g_args, args, sizeof(plugin_runtime_args_t)); + plugin_initialized = 1; + printf("Native Plugin: Initialized (buffer_size=%d)\n", g_args.buffer_size); + return 0; } void start_loop(void) { - printf("Native C Plugin: Starting operations\n"); + if (!plugin_initialized) return; + printf("Native Plugin: Starting operations\n"); // Start threads, timers, etc. } void stop_loop(void) { - printf("Native C Plugin: Stopping operations\n"); + printf("Native Plugin: Stopping operations\n"); // Signal threads to stop, etc. } - void run_cycle(void) { - // Example: safely read an input and write to an output - if (g_runtime_args && g_runtime_args->buffer_mutex) { - g_runtime_args->mutex_take(g_runtime_args->buffer_mutex); - // Assuming buffer_size > 0 and buffers are valid - IEC_UINT input_val = g_runtime_args->int_input[0][0]; // Read first word of first input buffer - g_runtime_args->int_output[0][0] = input_val; // Write to first word of first output buffer - g_runtime_args->mutex_give(g_runtime_args->buffer_mutex); - } - } - void cleanup(void) { - printf("Native C Plugin: Cleaning up\n"); - // Release resources, close files, destroy threads. - g_runtime_args = NULL; + printf("Native Plugin: Cleaning up\n"); + plugin_initialized = 0; } ``` 2. **Compile as a shared library:** ```bash - gcc -shared -fPIC -I -o my_native_plugin.so my_native_plugin.c + gcc -shared -fPIC -I/path/to/openplc-runtime/core/lib -o my_native_plugin.so my_native_plugin.c -lpthread ``` - (The exact include paths and dependencies will be defined when native plugin support is developed). + See `plugins/native/examples/Makefile` for a complete build example. -3. **Add to `plugins.conf` (for future use):** +3. **Add to `plugins.conf`:** ``` - my_native_plugin,./plugins/my_native_plugin.so,1,1,./config/my_native_plugin.conf + my_native_plugin,./path/to/my_native_plugin.so,1,1,./config/my_native_plugin.conf ``` +4. **Test your plugin:** + * Place the `.so` file at the specified path. + * Run OpenPLC. Check logs for your plugin's initialization messages. + * See `plugins/native/examples/test_plugin_loader.c` for a standalone test harness. + ## Buffer Mapping The OpenPLC I/O memory is organized into buffers accessible by plugins. @@ -604,10 +658,11 @@ void plugin_driver_destroy(plugin_driver_t *driver); * `pymodbus`: Required for the `simple_modbus.py` plugin. * Other Python packages: As needed by specific custom plugins. -### For Future Native C Plugins -* C Compiler (e.g., GCC). +### For Native C/C++ Plugins +* C Compiler (e.g., GCC) with `-fPIC` and `-shared` support. * Standard C library. -* Potentially other system-level libraries depending on the plugin's purpose. +* `pthread` library for mutex operations. +* `dl` library for dynamic loading (usually included by default). ## License @@ -625,17 +680,22 @@ When contributing new plugins: * Provide clear usage examples in comments or a separate `README`. * Test your plugin thoroughly with various OpenPLC programs and I/O configurations. * Ensure thread safety for all OpenPLC buffer interactions. -2. **Future Native C Plugins:** - * Adhere to the C API once it's fully implemented and documented. +2. **Native C/C++ Plugins:** + * Adhere to the C API defined in this document. * Pay close attention to memory management and thread safety. + * Remember to copy `plugin_runtime_args_t` contents during `init()` - do not store the pointer. + * Test with the standalone loader before integrating with the full runtime. ## See Also * `plugins/python/examples/example_python_plugin.py` - Basic Python plugin template. * `plugins/python/modbus_slave/simple_modbus.py` - Advanced Modbus TCP slave implementation. -* `plugins/python/shared/python_plugin_types.py` - Core type definitions and safety utilities. +* `plugins/python/shared/` - Core type definitions and safety utilities for Python plugins. +* `plugins/native/examples/test_plugin.c` - Example native C plugin implementation. +* `plugins/native/examples/Makefile` - Build configuration for native plugins. +* `plugins/native/examples/test_plugin_loader.c` - Standalone test harness for native plugins. * `plugins.conf` - Example active plugin configuration file. -* `core/src/drivers/plugin_driver.h` - C API for the plugin system (internal/future facing). +* `core/src/drivers/plugin_driver.h` - C API for the plugin system. * `core/src/drivers/python_plugin_bridge.h` - C interface for Python integration. * `docs/PLUGIN_VENV_GUIDE.md` - Guide on managing Python virtual environments for plugins. * OpenPLC Runtime main documentation. diff --git a/core/src/drivers/plugin_config.h b/core/src/drivers/plugin_config.h index 5a88c517..bb134377 100644 --- a/core/src/drivers/plugin_config.h +++ b/core/src/drivers/plugin_config.h @@ -9,7 +9,7 @@ typedef struct char name[MAX_PLUGIN_NAME_LEN]; char path[MAX_PLUGIN_PATH_LEN]; int enabled; - int type; // 0 = native, 1 = python + int type; // 0 = python, 1 = native char plugin_related_config_path[MAX_PLUGIN_PATH_LEN]; char venv_path[MAX_PLUGIN_PATH_LEN]; // Path to virtual environment } plugin_config_t; diff --git a/docs/DEVELOPMENT.md b/docs/DEVELOPMENT.md index a18e3694..c8d72c7f 100644 --- a/docs/DEVELOPMENT.md +++ b/docs/DEVELOPMENT.md @@ -505,13 +505,15 @@ State transitions are validated and logged. See `core/src/plc_app/plc_state_mana ### Plugin System -Plugins extend I/O capabilities: +Plugins extend I/O capabilities. Both Python and native C/C++ plugins are supported: -1. **Configuration:** `plugins.conf` defines enabled plugins -2. **Loading:** `plugin_driver_load_config()` parses configuration -3. **Initialization:** `plugin_driver_init()` initializes plugins -4. **Execution:** Plugins called during scan cycle -5. **Cleanup:** `plugin_driver_destroy()` cleans up resources +1. **Configuration:** `plugins.conf` defines enabled plugins (type 0=Python, 1=Native) +2. **Loading:** `plugin_driver_load_config()` parses configuration and loads symbols +3. **Initialization:** `plugin_driver_init()` calls each plugin's `init()` function +4. **Execution:** `plugin_driver_start()` calls each plugin's `start_loop()` function +5. **Cleanup:** `plugin_driver_destroy()` calls `cleanup()` and releases resources + +See `core/src/drivers/README.md` for detailed plugin development documentation. ### Debug Protocol From 58323cd41071164925169faa324efb8aff6f3135 Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Mon, 22 Dec 2025 23:41:57 +0000 Subject: [PATCH 15/17] Implement native plugin cycle hooks for real-time PLC synchronization - Add plugin_driver_cycle_start() and plugin_driver_cycle_end() functions - Integrate cycle hooks into PLC scan cycle in plc_state_manager.c - Call cycle_start before PLC logic execution, cycle_end after - Plugins opt-in by implementing cycle_start/cycle_end; opt-out by not implementing - Only active (enabled + running) native plugins are called - Update README.md with comprehensive cycle hooks documentation - Add cycle hook functions to API Reference section Co-Authored-By: Thiago Alves --- core/src/drivers/README.md | 48 ++++++++++++++++++++-- core/src/drivers/plugin_driver.c | 61 +++++++++++++++++++++++++++- core/src/drivers/plugin_driver.h | 6 +++ core/src/plc_app/plc_state_manager.c | 7 +++- 4 files changed, 115 insertions(+), 7 deletions(-) diff --git a/core/src/drivers/README.md b/core/src/drivers/README.md index 021c88e5..be473eb3 100644 --- a/core/src/drivers/README.md +++ b/core/src/drivers/README.md @@ -91,9 +91,9 @@ void start_loop(void); // Called when plugin should start operations void stop_loop(void); // Called when plugin should stop operations void cleanup(void); // Called when plugin is being unloaded -// Reserved hooks (symbols loaded but not currently invoked by the runtime) -void cycle_start(void); // Reserved for future per-cycle start hook -void cycle_end(void); // Reserved for future per-cycle end hook +// Per-cycle hooks (called during each PLC scan cycle, synchronized with PLC execution) +void cycle_start(void); // Called at start of each scan cycle, before PLC logic +void cycle_end(void); // Called at end of each scan cycle, after PLC logic ``` **Important: Native Plugin Args Lifetime** @@ -573,6 +573,14 @@ int plugin_driver_start(plugin_driver_t *driver); // Returns 0 on success int plugin_driver_stop(plugin_driver_t *driver); +// Call cycle_start for all active native plugins (called at start of each PLC scan cycle) +// Plugins opt-in by implementing cycle_start(); opt-out by not implementing it +void plugin_driver_cycle_start(plugin_driver_t *driver); + +// Call cycle_end for all active native plugins (called at end of each PLC scan cycle) +// Plugins opt-in by implementing cycle_end(); opt-out by not implementing it +void plugin_driver_cycle_end(plugin_driver_t *driver); + // Destroy the plugin driver and free resources (calls 'cleanup' on plugins) void plugin_driver_destroy(plugin_driver_t *driver); ``` @@ -640,9 +648,41 @@ void plugin_driver_destroy(plugin_driver_t *driver); 2. **Plugin Lifecycle Management:** * `init()`: Perform one-time setup (load config, initialize data structures). * `start_loop()`: Start long-running tasks (servers, periodic threads). - * `run_cycle()`: Keep this function lightweight if called frequently by the main OpenPLC loop. * `cleanup()`: Reliably release all resources (stop threads, close connections, free memory). +3. **Native Plugin Cycle Hooks (Real-Time Synchronization):** + + Native plugins can optionally implement `cycle_start()` and `cycle_end()` functions to synchronize with the PLC scan cycle. These hooks are called during each scan cycle while the buffer mutex is held, allowing direct access to I/O buffers without additional locking. + + * `cycle_start()`: Called at the beginning of each scan cycle, before PLC logic execution. Use this to read inputs or prepare data for the PLC program. + * `cycle_end()`: Called at the end of each scan cycle, after PLC logic execution. Use this to process outputs or perform post-cycle operations. + + **Opt-in/Opt-out:** Plugins opt-in by implementing these functions. If a plugin does not implement them (NULL pointer), the runtime simply skips calling them for that plugin. This allows plugins to choose between: + * **Synchronous operation**: Implement cycle hooks to run in lockstep with the PLC scan cycle (real-time). + * **Asynchronous operation**: Use `start_loop()` to create separate threads that run independently. + + **Important:** Keep cycle hook implementations as fast as possible since they run in the critical path of the PLC scan cycle. Long-running operations will increase scan cycle time and may cause timing issues. + + ```c + // Example: Native plugin with cycle hooks + static plugin_runtime_args_t g_args; + + int init(void *args) { + memcpy(&g_args, args, sizeof(plugin_runtime_args_t)); + return 0; + } + + void cycle_start(void) { + // Read inputs before PLC logic runs + // Buffer mutex is already held - safe to access buffers directly + } + + void cycle_end(void) { + // Process outputs after PLC logic runs + // Buffer mutex is still held - safe to access buffers directly + } + ``` + 3. **Memory Management (Python):** * Python's garbage collector handles memory. However, explicitly close files, sockets, or release other external resources in `cleanup()`. diff --git a/core/src/drivers/plugin_driver.c b/core/src/drivers/plugin_driver.c index 9c90e7ba..b278bab7 100644 --- a/core/src/drivers/plugin_driver.c +++ b/core/src/drivers/plugin_driver.c @@ -345,7 +345,7 @@ int plugin_driver_start(plugin_driver_t *driver) { // Acquire GIL for this specific Python call PyGILState_STATE local_gil = PyGILState_Ensure(); - PyObject *res = PyObject_CallNoArgs(plugin->python_plugin->pFuncStart); + PyObject *res = PyObject_CallNoArgs(plugin->python_plugin->pFuncStart); if (!res) { PyErr_Print(); @@ -562,7 +562,6 @@ void plugin_driver_destroy(plugin_driver_t *driver) } } - PyGILState_Release(local_gstate); PyEval_RestoreThread(main_tstate); Py_FinalizeEx(); @@ -968,6 +967,64 @@ void python_plugin_cycle(plugin_instance_t *plugin) // and call the cycle function } +// Call cycle_start for all active native plugins that have registered the hook +// This should be called at the beginning of each PLC scan cycle, before PLC logic execution +// Plugins opt-in by implementing cycle_start(); opt-out by not implementing it (NULL pointer) +void plugin_driver_cycle_start(plugin_driver_t *driver) +{ + if (!driver || driver->plugin_count == 0) + { + return; + } + + for (int i = 0; i < driver->plugin_count; i++) + { + plugin_instance_t *plugin = &driver->plugins[i]; + + // Skip disabled or non-running plugins + if (!plugin->config.enabled || !plugin->running) + { + continue; + } + + // Only native plugins support cycle hooks (they can run in real-time) + if (plugin->config.type == PLUGIN_TYPE_NATIVE && plugin->native_plugin && + plugin->native_plugin->cycle_start) + { + plugin->native_plugin->cycle_start(); + } + } +} + +// Call cycle_end for all active native plugins that have registered the hook +// This should be called at the end of each PLC scan cycle, after PLC logic execution +// Plugins opt-in by implementing cycle_end(); opt-out by not implementing it (NULL pointer) +void plugin_driver_cycle_end(plugin_driver_t *driver) +{ + if (!driver || driver->plugin_count == 0) + { + return; + } + + for (int i = 0; i < driver->plugin_count; i++) + { + plugin_instance_t *plugin = &driver->plugins[i]; + + // Skip disabled or non-running plugins + if (!plugin->config.enabled || !plugin->running) + { + continue; + } + + // Only native plugins support cycle hooks (they can run in real-time) + if (plugin->config.type == PLUGIN_TYPE_NATIVE && plugin->native_plugin && + plugin->native_plugin->cycle_end) + { + plugin->native_plugin->cycle_end(); + } + } +} + // Cleanup Python plugin static void python_plugin_cleanup(plugin_instance_t *plugin) { diff --git a/core/src/drivers/plugin_driver.h b/core/src/drivers/plugin_driver.h index da84babe..5bb70109 100644 --- a/core/src/drivers/plugin_driver.h +++ b/core/src/drivers/plugin_driver.h @@ -106,6 +106,12 @@ void plugin_driver_destroy(plugin_driver_t *driver); int plugin_mutex_take(pthread_mutex_t *mutex); int plugin_mutex_give(pthread_mutex_t *mutex); +// Cycle hook functions for native plugins (called during PLC scan cycle) +// These iterate through all active native plugins and call their cycle hooks +// Plugins opt-in by implementing cycle_start/cycle_end; opt-out by not implementing them +void plugin_driver_cycle_start(plugin_driver_t *driver); +void plugin_driver_cycle_end(plugin_driver_t *driver); + // Python plugin functions int python_plugin_get_symbols(plugin_instance_t *plugin); diff --git a/core/src/plc_app/plc_state_manager.c b/core/src/plc_app/plc_state_manager.c index 47e5eb46..c9f84be7 100644 --- a/core/src/plc_app/plc_state_manager.c +++ b/core/src/plc_app/plc_state_manager.c @@ -53,10 +53,16 @@ void *plc_cycle_thread(void *arg) scan_cycle_time_start(); plugin_mutex_take(&plugin_driver->buffer_mutex); + // Call cycle_start for all active native plugins that registered the hook + plugin_driver_cycle_start(plugin_driver); + // Execute the PLC cycle ext_config_run__(tick__++); ext_updateTime(); + // Call cycle_end for all active native plugins that registered the hook + plugin_driver_cycle_end(plugin_driver); + // Update Watchdog Heartbeat atomic_store(&plc_heartbeat, time(NULL)); @@ -126,7 +132,6 @@ int load_plc_program(PluginManager *pm) return -1; } - return 0; } From 0c0c9a60507476ebb30b54b6478f1fae04ca3061 Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Mon, 22 Dec 2025 23:48:04 +0000 Subject: [PATCH 16/17] Fix section numbering in README.md after adding cycle hooks section Co-Authored-By: Thiago Alves --- core/src/drivers/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/drivers/README.md b/core/src/drivers/README.md index be473eb3..df69a407 100644 --- a/core/src/drivers/README.md +++ b/core/src/drivers/README.md @@ -683,7 +683,7 @@ void plugin_driver_destroy(plugin_driver_t *driver); } ``` -3. **Memory Management (Python):** +4. **Memory Management (Python):** * Python's garbage collector handles memory. However, explicitly close files, sockets, or release other external resources in `cleanup()`. ## Dependencies From c49575875daf30301d801ff708632e7cf13c8f8f Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Thu, 25 Dec 2025 19:12:25 +0000 Subject: [PATCH 17/17] Fix function prototype declarations for cross-platform compatibility - Fix close_unix_socket() signature mismatch: header declared with no parameters but implementation took int server_fd parameter - Update empty () declarations to (void) in headers and definitions for C standards compliance: - setup_unix_socket() - watchdog_init() - scan_cycle_time_start() - scan_cycle_time_end() - Update function pointer typedefs in plugin_driver.h to use (void) - Make store_on_buffer and retrieve_from_buffer static in log.c These changes ensure the code compiles on stricter compilers that treat -Wstrict-prototypes and -Wold-style-definition as errors. Co-Authored-By: Thiago Alves --- core/src/drivers/plugin_driver.h | 10 +-- core/src/plc_app/scan_cycle_manager.c | 4 +- core/src/plc_app/scan_cycle_manager.h | 4 +- core/src/plc_app/unix_socket.c | 2 +- core/src/plc_app/unix_socket.h | 4 +- core/src/plc_app/utils/log.c | 92 +++++++++++++-------------- core/src/plc_app/utils/watchdog.c | 21 +++--- core/src/plc_app/utils/watchdog.h | 6 +- 8 files changed, 72 insertions(+), 71 deletions(-) diff --git a/core/src/drivers/plugin_driver.h b/core/src/drivers/plugin_driver.h index 5bb70109..7f271043 100644 --- a/core/src/drivers/plugin_driver.h +++ b/core/src/drivers/plugin_driver.h @@ -17,11 +17,11 @@ typedef enum } plugin_type_t; typedef int (*plugin_init_func_t)(void *); -typedef void (*plugin_start_loop_func_t)(); -typedef void (*plugin_stop_loop_func_t)(); -typedef void (*plugin_cycle_start_func_t)(); -typedef void (*plugin_cycle_end_func_t)(); -typedef void (*plugin_cleanup_func_t)(); +typedef void (*plugin_start_loop_func_t)(void); +typedef void (*plugin_stop_loop_func_t)(void); +typedef void (*plugin_cycle_start_func_t)(void); +typedef void (*plugin_cycle_end_func_t)(void); +typedef void (*plugin_cleanup_func_t)(void); // Logging function pointer types typedef void (*plugin_log_info_func_t)(const char *fmt, ...); diff --git a/core/src/plc_app/scan_cycle_manager.c b/core/src/plc_app/scan_cycle_manager.c index 78ef6534..55b92ee0 100644 --- a/core/src/plc_app/scan_cycle_manager.c +++ b/core/src/plc_app/scan_cycle_manager.c @@ -27,7 +27,7 @@ static uint64_t ts_now_us(void) return (uint64_t)ts.tv_sec * 1000000ull + ts.tv_nsec / 1000; } -void scan_cycle_time_start() +void scan_cycle_time_start(void) { uint64_t now_us = ts_now_us(); @@ -78,7 +78,7 @@ void scan_cycle_time_start() pthread_mutex_unlock(&stats_mutex); } -void scan_cycle_time_end() +void scan_cycle_time_end(void) { uint64_t now_us = ts_now_us(); diff --git a/core/src/plc_app/scan_cycle_manager.h b/core/src/plc_app/scan_cycle_manager.h index c624fe2e..7b62fdc9 100644 --- a/core/src/plc_app/scan_cycle_manager.h +++ b/core/src/plc_app/scan_cycle_manager.h @@ -22,8 +22,8 @@ typedef struct int64_t overruns; } plc_timing_stats_t; -void scan_cycle_time_start(); -void scan_cycle_time_end(); +void scan_cycle_time_start(void); +void scan_cycle_time_end(void); // Thread-safe function to get a snapshot of timing stats // Returns true if stats are valid (scan_count > 0), false otherwise diff --git a/core/src/plc_app/unix_socket.c b/core/src/plc_app/unix_socket.c index 7c8663b5..c74cb085 100644 --- a/core/src/plc_app/unix_socket.c +++ b/core/src/plc_app/unix_socket.c @@ -215,7 +215,7 @@ void close_unix_socket(int server_fd) } } -int setup_unix_socket() +int setup_unix_socket(void) { int server_fd; struct sockaddr_un address; diff --git a/core/src/plc_app/unix_socket.h b/core/src/plc_app/unix_socket.h index fc0ec452..d0df1f1a 100644 --- a/core/src/plc_app/unix_socket.h +++ b/core/src/plc_app/unix_socket.h @@ -6,8 +6,8 @@ #define MAX_RESPONSE_SIZE 16384 #define MAX_CLIENTS 1 -int setup_unix_socket(); -void close_unix_socket(); +int setup_unix_socket(void); +void close_unix_socket(int server_fd); void *unix_socket_thread(void *arg); #endif // UNIX_SOCKET_H diff --git a/core/src/plc_app/utils/log.c b/core/src/plc_app/utils/log.c index 099454dc..08e906e3 100644 --- a/core/src/plc_app/utils/log.c +++ b/core/src/plc_app/utils/log.c @@ -1,45 +1,43 @@ #include "log.h" +#include #include +#include #include -#include -#include -#include -#include -#include -#include -#include #include -#include +#include +#include +#include #include #include -#include +#include #include -#include -static LogLevel current_level = LOG_LEVEL_INFO; +static LogLevel current_level = LOG_LEVEL_INFO; static pthread_mutex_t log_mutex = PTHREAD_MUTEX_INITIALIZER; -int socket_fd = -1; -bool print_logs = false; +int socket_fd = -1; +bool print_logs = false; extern volatile sig_atomic_t keep_running; -void log_set_level(LogLevel level) { current_level = level; } +void log_set_level(LogLevel level) +{ + current_level = level; +} // Create circular buffer for unsent logs #define LOG_BUFFER_SIZE 1024 #define LOG_MESSAGE_SIZE 2048 char log_buffer[LOG_BUFFER_SIZE][LOG_MESSAGE_SIZE]; int log_buffer_start = 0; -int log_buffer_end = 0; - +int log_buffer_end = 0; -void *log_thread_management(void *arg) +void *log_thread_management(void *arg) { char *unix_socket_path = (char *)arg; - while(keep_running) + while (keep_running) { - if (socket_fd < 0) + if (socket_fd < 0) { struct sockaddr_un addr; socket_fd = socket(AF_UNIX, SOCK_STREAM, 0); @@ -72,7 +70,7 @@ void *log_thread_management(void *arg) return NULL; } -void store_on_buffer(const char *msg) +static void store_on_buffer(const char *msg) { strncpy(log_buffer[log_buffer_end], msg, sizeof(log_buffer[log_buffer_end]) - 1); log_buffer[log_buffer_end][sizeof(log_buffer[log_buffer_end]) - 1] = '\0'; @@ -85,19 +83,19 @@ void store_on_buffer(const char *msg) } } -char *retrieve_from_buffer() +static char *retrieve_from_buffer(void) { if (log_buffer_start == log_buffer_end) { return NULL; // Buffer is empty } - char *msg = log_buffer[log_buffer_start]; + char *msg = log_buffer[log_buffer_start]; log_buffer_start = (log_buffer_start + 1) % LOG_BUFFER_SIZE; return msg; } -int log_init(char *unix_socket_path) +int log_init(char *unix_socket_path) { // Create a copy of the socket path in the heap char *path_copy = malloc(strlen(unix_socket_path) + 1); @@ -110,7 +108,7 @@ int log_init(char *unix_socket_path) // Create the logging thread pthread_t thread_id; - if (pthread_create(&thread_id, NULL, log_thread_management, path_copy) != 0) + if (pthread_create(&thread_id, NULL, log_thread_management, path_copy) != 0) { free(path_copy); perror("Failed to create log thread"); @@ -120,31 +118,30 @@ int log_init(char *unix_socket_path) return 0; // Success } - -static const char *level_to_str(LogLevel level) +static const char *level_to_str(LogLevel level) { - switch (level) + switch (level) { - case LOG_LEVEL_DEBUG: - return "DEBUG"; - case LOG_LEVEL_INFO: - return "INFO"; - case LOG_LEVEL_WARN: - return "WARN"; - case LOG_LEVEL_ERROR: - return "ERROR"; - default: - return "UNKNOWN"; + case LOG_LEVEL_DEBUG: + return "DEBUG"; + case LOG_LEVEL_INFO: + return "INFO"; + case LOG_LEVEL_WARN: + return "WARN"; + case LOG_LEVEL_ERROR: + return "ERROR"; + default: + return "UNKNOWN"; } } -static void log_write(LogLevel level, const char *fmt, va_list args) +static void log_write(LogLevel level, const char *fmt, va_list args) { if (level < current_level) { return; } - + // Capture time for timestamp time_t now = time(NULL); struct tm t; @@ -163,7 +160,8 @@ static void log_write(LogLevel level, const char *fmt, va_list args) char time_buf[20]; strftime(time_buf, sizeof(time_buf), "%Y-%m-%d %H:%M:%S", &t); - int n = snprintf(stdout_msg, sizeof(stdout_msg), "[%s] [%s] ", time_buf, level_to_str(level)); + int n = + snprintf(stdout_msg, sizeof(stdout_msg), "[%s] [%s] ", time_buf, level_to_str(level)); n += vsnprintf(stdout_msg + n, sizeof(stdout_msg) - n, fmt, args_copy); snprintf(stdout_msg + n, sizeof(stdout_msg) - n, "\n"); @@ -172,13 +170,15 @@ static void log_write(LogLevel level, const char *fmt, va_list args) } // Format the log message in JSON format - int n = snprintf(log_msg, sizeof(log_msg), "{\"timestamp\":\"%ld\",\"level\":\"%s\",\"message\":\"", (long)now, level_to_str(level)); + int n = + snprintf(log_msg, sizeof(log_msg), "{\"timestamp\":\"%ld\",\"level\":\"%s\",\"message\":\"", + (long)now, level_to_str(level)); n += vsnprintf(log_msg + n, sizeof(log_msg) - n, fmt, args); snprintf(log_msg + n, sizeof(log_msg) - n, "\"}\n"); // Send to unix socket if connected pthread_mutex_lock(&log_mutex); - if (socket_fd >= 0) + if (socket_fd >= 0) { // Send any buffered messages first char *buffered_msg = retrieve_from_buffer(); @@ -224,7 +224,7 @@ static void log_write(LogLevel level, const char *fmt, va_list args) pthread_mutex_unlock(&log_mutex); } -void log_info(const char *fmt, ...) +void log_info(const char *fmt, ...) { va_list args; va_start(args, fmt); @@ -232,7 +232,7 @@ void log_info(const char *fmt, ...) va_end(args); } -void log_debug(const char *fmt, ...) +void log_debug(const char *fmt, ...) { va_list args; va_start(args, fmt); @@ -240,7 +240,7 @@ void log_debug(const char *fmt, ...) va_end(args); } -void log_warn(const char *fmt, ...) +void log_warn(const char *fmt, ...) { va_list args; va_start(args, fmt); @@ -248,7 +248,7 @@ void log_warn(const char *fmt, ...) va_end(args); } -void log_error(const char *fmt, ...) +void log_error(const char *fmt, ...) { va_list args; va_start(args, fmt); diff --git a/core/src/plc_app/utils/watchdog.c b/core/src/plc_app/utils/watchdog.c index cd630c1c..ee723965 100644 --- a/core/src/plc_app/utils/watchdog.c +++ b/core/src/plc_app/utils/watchdog.c @@ -5,10 +5,10 @@ #include #include -#include "watchdog.h" +#include "../plc_state_manager.h" #include "log.h" #include "utils.h" -#include "../plc_state_manager.h" +#include "watchdog.h" atomic_long plc_heartbeat; extern PLCState plc_state; @@ -18,19 +18,22 @@ void *watchdog_thread(void *arg) (void)arg; long last = atomic_load(&plc_heartbeat); - while (1) + while (1) { sleep(2); // Watch every 2 seconds - if (plc_get_state() != PLC_STATE_RUNNING) + if (plc_get_state() != PLC_STATE_RUNNING) { continue; // Only monitor when PLC is running } - + long now = atomic_load(&plc_heartbeat); - if (now == last) + if (now == last) { - fprintf(stderr, "[Watchdog] No heartbeat! PLC unresponsive.\n"); // Use stderr to ensure visibility and avoid lockups in log system + fprintf( + stderr, + "[Watchdog] No heartbeat! PLC unresponsive.\n"); // Use stderr to ensure visibility + // and avoid lockups in log system exit(EXIT_FAILURE); } @@ -40,10 +43,10 @@ void *watchdog_thread(void *arg) return NULL; } -int watchdog_init() +int watchdog_init(void) { pthread_t wd_thread; - if (pthread_create(&wd_thread, NULL, watchdog_thread, NULL) != 0) + if (pthread_create(&wd_thread, NULL, watchdog_thread, NULL) != 0) { log_error("Failed to create watchdog thread"); return -1; diff --git a/core/src/plc_app/utils/watchdog.h b/core/src/plc_app/utils/watchdog.h index 238423ba..065c6ccd 100644 --- a/core/src/plc_app/utils/watchdog.h +++ b/core/src/plc_app/utils/watchdog.h @@ -1,12 +1,10 @@ #ifndef WATCHDOG_H #define WATCHDOG_H - /** * @brief Initialize the watchdog * @return int 0 on success, -1 on failure */ -int watchdog_init(); - +int watchdog_init(void); -#endif // WATCHDOG_H \ No newline at end of file +#endif // WATCHDOG_H