Add read and write for UA_String type; run tests against opc server #7
Conversation
|
@rrundzans Hi! Would it be possible to check and publish this? |
There was a problem hiding this comment.
Pull Request Overview
This PR adds support for reading and writing UA_String types in the OPC-UA client and enhances the test infrastructure by running tests against a real OPC server.
- Implements string type support in both C extension and test server
- Refactors server variable creation functions for better type safety
- Adds comprehensive integration tests using a compiled OPC server
Reviewed Changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tools/server/server.cpp | Adds string type support and refactors variable creation with type-specific functions |
| tools/server/makefile | Updates build output name from 'server' to 'opcua-server' |
| spec/core_spec.rb | Adds comprehensive integration tests for all data types using real OPC server |
| ext/opcua_client/opcua_client.c | Implements string read/write support in the C extension |
| README.md | Updates documentation with string methods and corrected server executable name |
| .github/workflows/build.yml | Removes Windows from CI matrix |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| addVariableV2(server, nsId, type, variable, &defaultValue); | ||
| } | ||
|
|
||
| static void addVariableFloat(UA_Server *server, UA_Int16 nsId, int type, const char *variable, UA_Float defaultValue = 0) { |
There was a problem hiding this comment.
The default parameter value should be 0.0f instead of 0 to maintain type consistency with UA_Float.
| static void addVariableFloat(UA_Server *server, UA_Int16 nsId, int type, const char *variable, UA_Float defaultValue = 0) { | |
| static void addVariableFloat(UA_Server *server, UA_Int16 nsId, int type, const char *variable, UA_Float defaultValue = 0.0f) { |
| Check_Type(v_newValue, T_STRING); | ||
| UA_String newValue = UA_STRING(StringValueCStr(v_newValue)); | ||
| values[i].data = UA_malloc(sizeof(UA_String)); | ||
| UA_String_copy(&newValue, (UA_String*)values[i].data); |
There was a problem hiding this comment.
Memory leak potential: if UA_String_copy fails, the allocated memory from UA_malloc will not be freed. Consider checking the return value of UA_String_copy and freeing the allocated memory on failure.
| UA_String_copy(&newValue, (UA_String*)values[i].data); | |
| UA_StatusCode copyStatus = UA_String_copy(&newValue, (UA_String*)values[i].data); | |
| if (copyStatus != UA_STATUSCODE_GOOD) { | |
| UA_free(values[i].data); | |
| return raise_ua_status_error(copyStatus); | |
| } |
| } else if (uaType == UA_TYPES_STRING) { | ||
| UA_String newValue = UA_STRING(StringValueCStr(v_newValue)); | ||
| value.data = UA_malloc(sizeof(UA_String)); | ||
| UA_String_copy(&newValue, (UA_String*)value.data); |
There was a problem hiding this comment.
Memory leak potential: same issue as in the multi-write function. If UA_String_copy fails, the allocated memory from UA_malloc will not be freed.
| UA_String_copy(&newValue, (UA_String*)value.data); | |
| UA_StatusCode copyStatus = UA_String_copy(&newValue, (UA_String*)value.data); | |
| if (copyStatus != UA_STATUSCODE_GOOD) { | |
| UA_free(value.data); | |
| rb_raise(cError, "UA_String_copy failed"); | |
| } |
2 participants
No description provided.