Adds `OPC UA` methods for device control by idzm · Pull Request #1304 · savushkin-r-d/ptusa_main

idzm · 2026-04-23T13:05:32Z

Introduces set_state, set_value, on, and off methods to the OPC UA server for remote device management. These methods are conditionally executable based on the P_IS_OPC_UA_SERVER_CONTROL parameter and include comprehensive unit tests for validation.

Introduces `set_state`, `set_value`, `on`, and `off` methods to the OPC UA server for remote device management. These methods are conditionally executable based on the `P_IS_OPC_UA_SERVER_CONTROL` parameter and include comprehensive unit tests for validation.

Copilot

Pull request overview

This PR extends the existing OPC UA server integration by adding callable method nodes on each device object, enabling remote device control operations (set state/value and on/off) gated by the PAC_info::P_IS_OPC_UA_SERVER_CONTROL parameter.

Changes:

Added OPC UA method nodes (set_state, set_value, on, off) per device during device node creation.
Implemented corresponding server-side method callbacks enforcing argument validation and access control via P_IS_OPC_UA_SERVER_CONTROL.
Added unit tests validating callback behavior for allowed/denied access and invalid argument handling; updated VS launch args to run OPC UA in rw mode.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.

File	Description
`PAC/common/OPCUAServer.h`	Declares new OPC UA method callbacks and a helper to attach method nodes to devices.
`PAC/common/OPCUAServer.cpp`	Registers method nodes under each device and implements callback logic for state/value and on/off.
`test/OPCUAServer_tests.cpp`	Adds unit tests covering the new method callbacks’ access control and input validation.
`.vs/launch.vs.json`	Enables OPC UA `rw` mode in the default Visual Studio launch configuration.

Adds a call to `valve::evaluate()` in the `methods_callbacks` test to handle valve shutdown delays. This ensures the device state is correctly updated before verifying the results of the `off` method.

codecov · 2026-04-23T13:23:39Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 56.16%. Comparing base (c97b644) to head (796e63c).

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1304      +/-   ##
==========================================
+ Coverage   56.00%   56.16%   +0.15%     
==========================================
  Files          76       76              
  Lines       26733    26835     +102     
  Branches     3274     3287      +13     
==========================================
+ Hits        14971    15071     +100     
- Misses      11762    11764       +2

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copilot

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

Updates OPC UA method descriptions with Russian translations and improves code readability through line formatting. Adjusts the `evaluate_non_blocking` test parameters for stricter timing requirements and refactors method callback tests to use static calls. Also ensures immediate valve state transitions in tests by resetting the off-delay parameter.

Introduces debug-level logging for the `set_state`, `set_value`, `on`, and `off` methods in the OPC UA server. This provides better visibility into remote device management by recording the device name and the specific parameters used in each method call.

Adds a new test case for `add_device_methods` and introduces null-pointer validation checks for `method_set_state` and `method_set_value`. These additions ensure the server handles uninitialized device contexts gracefully by returning `UA_STATUSCODE_BADINTERNALERROR`.

Copilot

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 12 comments.

Copilot · 2026-04-24T07:18:08Z

    const auto elapsed = std::chrono::duration_cast<std::chrono::milliseconds>(
        std::chrono::steady_clock::now() - start );

-    EXPECT_LT( elapsed.count(), 1000 );
+    EXPECT_LT( elapsed.count(), 10 );


The non-blocking timing assertion (elapsed < 10ms) is very likely to be flaky on slower/loaded CI machines, since it measures wall-clock time across multiple iterate calls and includes scheduler jitter. Consider using a significantly higher threshold (or asserting per-iteration bounds with a generous margin) so the test validates "doesn't block" without depending on sub-10ms timing.

Copilot · 2026-04-24T07:18:08Z

+    UA_Server_addMethodNode( server, UA_NODEID_NULL, deviceId,
+        UA_NODEID_NUMERIC( 0, UA_NS0ID_HASCOMPONENT ),
+        qn, methodAttr, method_set_state,
+        1, &inputState, 0, nullptr, dev, nullptr );


UA_Server_addMethodNode's return status is ignored here, so failures to create the method node would be silent and hard to diagnose. Capture the returned UA_StatusCode and handle/log/ASSERT it (and consider doing the same for the other method nodes added below).

Suggested change

UA_Server_addMethodNode( server, UA_NODEID_NULL, deviceId,

UA_NODEID_NUMERIC( 0, UA_NS0ID_HASCOMPONENT ),

qn, methodAttr, method_set_state,

1, &inputState, 0, nullptr, dev, nullptr );

UA_StatusCode addMethodStatus = UA_Server_addMethodNode( server,

UA_NODEID_NULL, deviceId,

UA_NODEID_NUMERIC( 0, UA_NS0ID_HASCOMPONENT ),

qn, methodAttr, method_set_state,

1, &inputState, 0, nullptr, dev, nullptr );

if ( addMethodStatus != UA_STATUSCODE_GOOD )

{

fprintf( stderr,

"UA_Server_addMethodNode failed for set_state: 0x%08x\n",

( unsigned int )addMethodStatus );

}

Copilot · 2026-04-24T07:18:08Z

+    qn = UA_QUALIFIEDNAME_ALLOC( 1, "on" );
+    UA_Server_addMethodNode( server, UA_NODEID_NULL, deviceId,
+        UA_NODEID_NUMERIC( 0, UA_NS0ID_HASCOMPONENT ),
+        qn, methodAttr, method_on,
+        0, nullptr, 0, nullptr, dev, nullptr );


UA_Server_addMethodNode's return status is ignored here, so failures to create the method node would be silent and hard to diagnose. Capture the returned UA_StatusCode and handle/log it.

Copilot · 2026-04-24T07:18:09Z

+    qn = UA_QUALIFIEDNAME_ALLOC( 1, "off" );
+    UA_Server_addMethodNode( server, UA_NODEID_NULL, deviceId,
+        UA_NODEID_NUMERIC( 0, UA_NS0ID_HASCOMPONENT ),
+        qn, methodAttr, method_off,
+        0, nullptr, 0, nullptr, dev, nullptr );


UA_Server_addMethodNode's return status is ignored here, so failures to create the method node would be silent and hard to diagnose. Capture the returned UA_StatusCode and handle/log it.

Copilot · 2026-04-24T07:18:09Z

+
+    UA_Argument inputValue;
+    UA_Argument_init( &inputValue );
+    inputValue.description = UA_LOCALIZEDTEXT_ALLOC( "en-US",


The argument description is Russian text but is tagged with locale "en-US". Use a matching locale (e.g., "ru-RU") or provide an English description for "en-US".

Suggested change

inputValue.description = UA_LOCALIZEDTEXT_ALLOC( "en-US",

inputValue.description = UA_LOCALIZEDTEXT_ALLOC( "ru-RU",

Copilot · 2026-04-24T07:18:09Z

+    UA_Server_addMethodNode( server, UA_NODEID_NULL, deviceId,
+        UA_NODEID_NUMERIC( 0, UA_NS0ID_HASCOMPONENT ),
+        qn, methodAttr, method_set_value,
+        1, &inputValue, 0, nullptr, dev, nullptr );


UA_Server_addMethodNode's return status is ignored here, so failures to create the method node would be silent and hard to diagnose. Capture the returned UA_StatusCode and handle/log it (consistent with the other node creation calls).

Suggested change

UA_Server_addMethodNode( server, UA_NODEID_NULL, deviceId,

UA_NODEID_NUMERIC( 0, UA_NS0ID_HASCOMPONENT ),

qn, methodAttr, method_set_value,

1, &inputValue, 0, nullptr, dev, nullptr );

UA_StatusCode res = UA_Server_addMethodNode( server, UA_NODEID_NULL,

deviceId, UA_NODEID_NUMERIC( 0, UA_NS0ID_HASCOMPONENT ),

qn, methodAttr, method_set_value,

1, &inputValue, 0, nullptr, dev, nullptr );

if ( res != UA_STATUSCODE_GOOD )

{

printf( "OPC UA: failed to add method node 'set_value': %s\n",

UA_StatusCode_name( res ) );

}

Copilot · 2026-04-24T07:18:10Z

+
+    UA_Argument inputState;
+    UA_Argument_init( &inputState );
+    inputState.description = UA_LOCALIZEDTEXT_ALLOC( "en-US",


The argument description is Russian text but is tagged with locale "en-US". Use a matching locale (e.g., "ru-RU") or provide an English description for "en-US" to avoid misleading clients.

Suggested change

inputState.description = UA_LOCALIZEDTEXT_ALLOC( "en-US",

inputState.description = UA_LOCALIZEDTEXT_ALLOC( "ru-RU",

Copilot · 2026-04-24T07:18:10Z

+    dev->set_state( state );
+
+    G_LOG->debug( "%s\t OPCUA_server::method_set_state( %d )", 


These OPC UA control methods call device::set_state(), which is a no-op when the device is in manual mode (see i_DO_device::set_state). Since manual mode is documented as the mode where the device is controlled from the server, consider using device::set_cmd("S"/"ST", ...) or another server-control path that bypasses the manual-mode guard so OPC UA control works as intended in manual mode.

Suggested change

dev->set_state( state );

G_LOG->debug( "%s\t OPCUA_server::method_set_state( %d )",

dev->set_cmd( "ST", state );

G_LOG->debug( "%s\t OPCUA_server::method_set_state( %d )",

Copilot · 2026-04-24T07:18:10Z

+
+    auto dev = static_cast<device*>( methodContext );
+    const auto value = *static_cast<UA_Float*>( input[ 0 ].data );
+    dev->set_value( value );


These OPC UA control methods call i_AO_device::set_value(), which is a no-op in manual mode. If OPC UA is meant to act as "server" control, use device::set_cmd("V", ...) or another mechanism that bypasses the manual-mode guard so the value can actually be set while in manual mode.

Suggested change

dev->set_value( value );

dev->set_cmd( "V", value );

Copilot · 2026-04-24T07:18:10Z

+    auto dev = static_cast<device*>( methodContext );
+    dev->on();


Calling dev->on() respects the manual-mode guard (i_DO_device::on), so in manual mode this will do nothing. If OPC UA is intended to provide server-side/manual control, consider using device::set_cmd("S"/"ST", ...) or an equivalent direct control path so the method works in manual mode.

sonarqubecloud · 2026-04-30T12:57:47Z

Quality Gate passed

Issues
5 New issues
16 Accepted issues

Measures
0 Security Hotspots
100.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

Copilot AI review requested due to automatic review settings April 23, 2026 13:05

idzm requested a review from Lictwin as a code owner April 23, 2026 13:05

idzm self-assigned this Apr 23, 2026

idzm requested review from AlexandrBehunkov and SergeiSilakov April 23, 2026 13:05

idzm added the enhancement New feature or request label Apr 23, 2026

Copilot started reviewing on behalf of idzm April 23, 2026 13:06 View session

Copilot AI reviewed Apr 23, 2026

View reviewed changes

Comment thread PAC/common/OPCUAServer.cpp

Comment thread PAC/common/OPCUAServer.cpp

Comment thread PAC/common/OPCUAServer.cpp Outdated

Comment thread PAC/common/OPCUAServer.cpp

Updates OPC UA server tests to process valve evaluation

cfa5bcc

Adds a call to `valve::evaluate()` in the `methods_callbacks` test to handle valve shutdown delays. This ensures the device state is correctly updated before verifying the results of the `off` method.

Copilot AI review requested due to automatic review settings April 23, 2026 13:19

idzm force-pushed the add_opc_ua_methods branch from 3792924 to cfa5bcc Compare April 23, 2026 13:19

Copilot started reviewing on behalf of idzm April 23, 2026 13:20 View session

Copilot AI reviewed Apr 23, 2026

View reviewed changes

Comment thread test/OPCUAServer_tests.cpp

Comment thread test/OPCUAServer_tests.cpp

idzm added 3 commits April 24, 2026 09:15

Copilot AI review requested due to automatic review settings April 24, 2026 07:12

Copilot started reviewing on behalf of idzm April 24, 2026 07:12 View session

Copilot AI reviewed Apr 24, 2026

View reviewed changes

Merge branch 'master' into add_opc_ua_methods

796e63c

	inputValue.description = UA_LOCALIZEDTEXT_ALLOC( "en-US",
	inputValue.description = UA_LOCALIZEDTEXT_ALLOC( "ru-RU",

	inputState.description = UA_LOCALIZEDTEXT_ALLOC( "en-US",
	inputState.description = UA_LOCALIZEDTEXT_ALLOC( "ru-RU",

		dev->set_state( state );

		G_LOG->debug( "%s\t OPCUA_server::method_set_state( %d )",

Conversation

idzm commented Apr 23, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

codecov Bot commented Apr 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

Uh oh!

sonarqubecloud Bot commented Apr 30, 2026

Quality Gate passed

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

codecov Bot commented Apr 23, 2026 •

edited

Loading