-
Notifications
You must be signed in to change notification settings - Fork 37
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix configuration of OPC UA A&C and Events layer #307
Conversation
src/com/opc_ua/CMakeLists.txt
Outdated
opcua_event_layer opcua_ac_layer opcua_client_config_parser) | ||
opcua_client_config_parser) | ||
|
||
if (FORTE_COM_OPC_UA_AC) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
these two ifs should not be necessary as the add network layer is allready adding the source and hpp files to the build.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The issue here is that when I remove the ifs, then opc_ua_ac_layer
is compiled regardless of the state of the FORTE_COM_OPC_UA_AC
flag. The build fails if a open62541 build is used without enabled A&C.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This must then be another problem, because the add network layer macro does that exactly for you
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just tested your patch on my machine and removed the ifs and it worked exactly as it should.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the ifs are not necessary after opcua_event_layer
and opcua_ac_layer
are removed on line 70 - otherwise adding those to the build manually will override whatever checks are done in forte_add_network_layer
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah yes, it works for me as well now. I still had them in the configuration.
Should we also remove the opcua_layer
in line 69 to keep it consistent with the CMake configurations of other layers?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As we are cleaning up yes please.
4b9ad99
to
652e83b
Compare
No description provided.