Rtop 78 create log filters#18
Merged
Merged
Conversation
Introduces a plugin driver framework supporting both native and Python plugins, including configuration parsing, driver management, and integration into the main PLC application. Updates CMake to link Python libraries and adds example configuration files for plugins. (WIP)
Removed legacy driver_api files and introduced new plugin driver structures to support Python-based plugins. Added a simple_modbus.py driver and configuration for Modbus slave integration. Updated plugin_driver.h and python_plugin_bridge.h to support Python plugin bindings and runtime argument passing. Modified plc_main.c to call plugin_driver_init instead of plugin_driver_start. Updated plugins.conf to use the new Python Modbus plugin.
Accordingly with the documentation, python header should be the first called and for security, we have to define PY_SSIZE_T_CLEAN
Everything accordingly BARR Standard
everything accordingly BARR standard
deleting python cycle function since it will be running async
for some reason, when init is called it can successfully link buffers and function address between runtime and plugin, but when the same previous parsed "struct" is called within start function, the buffer pointer was no longer pointing to the right address.
Corrects how bool_output buffer values are read and written by accessing the actual value via .contents.value instead of the pointer. Updates example plugin to use SafeBufferAccess for safer buffer operations and improves output logging.
Stop is already being called within destroy function
Eliminated the _runtime_args_capsule global variable and related code from example_python_plugin.py, simplifying state management and usage of runtime arguments.
Moved plugin thread creation to Python side and removed native thread management for Python plugins. Updated function names in python_binds_t for clarity. Improved plugin start, stop, and cleanup logic to use Python-side functions and ensured proper GIL handling. Cleaned up resource management and removed unused thread fields from plugin_instance_t.
Improves Python GIL state management in plugin_driver by using a static variable and ensuring proper acquisition/release during plugin lifecycle. Moves plugin driver cleanup earlier in plc_main to avoid double destruction and adds more informative logging during plugin stop.
The plugin's main loop now runs in a background thread using Python's threading module. Added a stop event to allow graceful termination of the loop in stop_loop, improving plugin lifecycle management and preventing blocking the main thread.
Replaces manual ctypes structure and buffer access with type-safe wrappers from python_plugin_types. Updates OpenPLCModbusDataBlock to use SafeBufferAccess for reading and writing coil values, improving safety and error handling. Refactors plugin initialization and server startup for better diagnostics and reliability.
Revised README to clarify plugin type values, enhance Python plugin type safety, and document usage of the new python_plugin_types.py module. Added examples for thread-safe buffer access, advanced Modbus implementation, and improved plugin initialization with structure validation and error handling.
Made plugin_mutex_take and plugin_mutex_give functions public in plugin_driver.h and used them to protect buffer access in the PLC cycle thread. Also refactored plugin_driver variable to be global in plc_main.c for thread safety.
…er-contracts Task/rtop 57 implement driver contracts
* thread safe and dump access in the same function * adding batch functions that allows multiple reads/writes * Providing wrappers for IS, IR and HR * avoiding magical numbers * avoiding generic exception handling
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR introduces a comprehensive logging infrastructure update that replaces the standard Python logging module with a custom JSON-based logging system. The changes implement structured log formatting, centralized log buffering, and log filtering capabilities.
Key changes:
- Replaced standard logging with custom JSON formatter and buffer handler
- Added log parsing capabilities for external log sources
- Implemented log filtering by ID and severity level
- Enhanced error handling with specific exception types
Reviewed Changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| webserver/unixserver.py | Updated to use custom logger and LogParser for processing incoming log lines |
| webserver/unixclient.py | Switched to custom logger with modified log levels and removed validation logic |
| webserver/runtimemanager.py | Integrated new logging system and added filtered log retrieval functionality |
| webserver/restapi.py | Enhanced error handling with specific database exceptions and JWT error handling |
| webserver/plcapp_management.py | Replaced standard logging with custom logger and improved safety check messaging |
| webserver/logger/parser.py | New module for parsing and normalizing log entries from various formats |
| webserver/logger/logger.py | New logger factory function with JSON formatting and buffer support |
| webserver/logger/formatter.py | Custom JSON formatter for structured log output |
| webserver/logger/bufferhandler.py | In-memory log buffer with filtering and normalization capabilities |
| webserver/logger/init.py | Logger module initialization with shared buffer handler |
| webserver/credentials.py | Updated to use custom logger with improved line formatting |
| webserver/config.py | Removed hardcoded logging configuration |
| webserver/app.py | Enhanced log filtering functionality and improved error handling order |
| webserver/init.py | Removed old logging configuration |
| .vscode/settings.json | Added C header file association |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
marconetsf
reviewed
Oct 10, 2025
marconetsf
reviewed
Oct 10, 2025
marconetsf
reviewed
Oct 10, 2025
marconetsf
requested changes
Oct 10, 2025
Contributor
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 10 out of 11 changed files in this pull request and generated 3 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
thiagoralves
added a commit
that referenced
this pull request
May 11, 2026
Hardens task lifecycle and the new STruC++ debugger ABI; rebuilds plugin reload around name-matching instead of slot-positional dlclose; replaces the build-breaking MatIEC-era test stub with new debugger ABI tests. Critical - Partial-spawn rollback in plc_cycle_thread: if pthread_create fails for task i>0, signal+join 0..i-1 and free plc_tasks before bailing so the next load doesn't dereference orphaned threads. - plugin_driver_update_config: tear down each old slot by its CURRENT stored type (Python vs Native) before rebuilding from configs[]. The old slot-positional dlclose leaked native handles when a slot's type changed and force-freed Python state on type swap. - STATS race: plc_tasks_lock brackets alloc/free in the cycle thread and the format_timing_stats_response iteration. is_transitioning doesn't bracket an in-flight STATS call, so a plugin-initiated stop could free plc_tasks underneath the unix-socket reader. - Test stub: drop the dead plugin_utils.h include and the removed get_var_list/get_var_size/get_var_count stubs that were preventing the C test target from compiling. High - plugin_driver_init now sets `initialized` per slot; new plugin_driver_cleanup_init mirror rolls those back in reverse order. load_plc_program calls it on init failure or post-init pthread_create failure so a retry doesn't double-init plugin state. - plugin_driver_init failure now propagates: load_plc_program drops to ERROR instead of silently spawning the cycle thread on top of a half-failed init. - plugin_driver_update_config returns -1 if any ENABLED native plugin fails native_plugin_get_symbols; previously this was a warn and the plugin loaded as a silent no-op. - teardown_plugin_instance refuses to dlclose a still-running plugin — defense against a future caller pulling the .so out from under live function pointers. - EtherCAT bus thread no longer re-installs SIGUSR1; one process-wide handler is set in plc_main.c. sigaction is process-wide so multiple bus threads / task threads racing to register were last-writer-wins and a future divergence between handlers would silently lose half the time. Medium - holding_mutex flag is set AFTER pthread_mutex_lock returns; setting it before meant a signal landing during lock acquisition would have the crash handler unlock a mutex this thread never owned (UB on PI mutexes). - EtherCAT latency_ns = signed(actual_wake) - signed(expected) so an early wake doesn't depend on impl-defined unsigned→signed conversion. - Debug handler validates `arr` against ext_strucpp_debug_array_count() at FC 0x42/0x43/0x44 entry. Defense-in-depth: the editor's codegen validates inside the .so, but a malformed .so could OOB-read its internal table. - Debug FC 0x45 caps the MD5 read at 32 bytes regardless of termination — bounded info disclosure if a .so exports a non-null- terminated strucpp_program_md5. - plc_begin_transition uses an atomic CAS on is_transitioning and re-checks plc_get_state() under the gate, fixing both the check-then-call race in plugin_request_plc_stop and the missing rate-limit (concurrent calls collapse to one worker). - update_plugin_configurations wrapped in try/except so an exception there flips status to FAILED instead of leaving it pinned at COMPILING (editor would otherwise poll forever). - VPP plugin .so output moved to build/vpp/ subdir so the cleanup glob can't accidentally match a future built-in plugin .so dropped into build/ directly. Tests - tests/test_debug_handler.c — 16 tests across FC 0x41-0x45 wire format: array layout, force/unforce, contiguous range read, cross-array batch, MD5 echo, OOB arr rejection at the runtime gate (issue #17), unbounded-string MD5 mitigation (issue #18), unknown FC, short frames, status passthrough. - tests/test_scan_cycle_tracker.c — 15 tests covering tracker init, first-cycle seeding, EWMA recovery at avg_window=1, overruns counter, NULL safety. - tests/support/debug_handler_mocks.{c,h} — controllable fakes for the ext_strucpp_debug_* function pointers and the program-MD5 char*. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.