User story #351
Updated by Didac Magrina over 4 years ago
Documentation, linting and coverage checked. Deployed in develop.
Implement an OPCUA interface for signals and StructuredDataI.
h1. Source code files modified
h1. Architecture & design review
*Date of the review:* //2019
*Person who did the review:* Andre' Neto
*Version of architecture & design document:* N/A. Strategy and main ideas for implementation discussed with B. Bauvir in informal meetings.
*Result of review:* N/A
*List of non-conformities:* N/A
h1. Code and documentation review
*Date of the review:* 13/07/2020 //2019
*Person who did the review:* Dídac Magriñá
*Result of review:* PASS
*List of non-conformities:*
+Possible Memory Issues+:
* @OPCUAClientI@
** @GetReferences@:
*** missing check to number of nodes before accessing @bReq.nodesToBrowse@
* @OPCUAClientMethod@, @OPCUAClientRead@, @OPCUAClientWrite@
** Destructor (of three mentioned classes):
*** missing check if @tempVariant@ and @monitoredNodes@ are allocated before deleting them
** @GetExtensionObjectByteString@ (of three mentioned classes):
*** missing check if @index@ is within range before accessing @entryTypes@ and @entryArrayElements@
*** missing check if @valueMemories[nodeCounter]@ is not null before accessing
*** missing check if @tempDataPtr[nOfBytes]@ is not null before accessing
** @SetServiceRequest@ (of three mentioned classes):
*** missing delete @bReq.nodesToBrowse@
** @OPCUAClientMethod::SetObjectRequest@
*** missing delete @tempStringnodeId@ after its allocation in @OPCUAClientI::GetReferences@
*** missing delete @ombReq.nodesToBrowse@
** @OPCUAClientMethod::SetMethodRequest@
*** missing delete @tempStringnodeId@ after its allocation in @OPCUAClientI::GetReferences@
*** missing delete @mbReq.nodesToBrowse@
** @OPCUAClientWrite::SetWriteRequest@
*** missing check if @writeValues[idx]@ is not null before accessing
* @OPCUADSInput@, @OPCUADSOutput@
** @GetSignalMemoryBuffer@ (of both classes)
*** missing check if @signalIdx@ is within range before accessing @types@ and @nElements@
** @GetStructure@ (of both classes)
*** missing check if @index@ is within range before accessing @entryArrayElements@, @entryTypes@, and @entryNumberOfMembers@
* @OPCUANode@
** Destructor
*** missing delete of @settings->attr.arrayDimensions@ created on @OPCUANode::InitArray@
*** missing delete of @settings->attr.value.arrayDimensions@ created on @OPCUANode::InitArray@
* @OPCUAClient@
** @MapStructuredData@
*** missing check if @index@ is within range before accessing @entryArrayElements@, @entryTypes@, and @entryMemberNames@
+Documentation Improvements+:
* Document structures in @OPCUATypes.h@ following Doxygen style
* Confirm that multiple extension objects are not supported as per @OPCUADSInput@/@OPCUADSOutput@ documentation. If they are supported, consider adding a test for this use case.
<pre>
When using Complex DataType Extension, the DataSource only allows to write 1 structure. If you need to add more signals you must add another OPCUADSInput DataSource to your real time application.
</pre>
+Refactoring Suggestions+:
* @OPCUAClientRead@, @OPCUAClientWrite@, and @OPCUAClientMethod@ have very similar methods that could be moved up to their interface @OPCUAClientI@:
** SetServiceRequest
** GetExtensionObjectByteString
h1. Unit test review
*Date of the review:* 13/07/2019 //2019
*Person who did the review:* Dídac Magriñá
*Result of coverage tests review:* PASS
*Result of functional tests review:* PASS
*Result of review:* PASS
*List of non-conformities:*
* @OPCUAMessageClient@: coverage lower than 90% (82.9%)
* The mock OPCUA server used in other tests could be leveraged on @OPCUAClientMethodTest@, @OPCUAMessageClientTest@ to check if UpdatePoint method has been called with expected parameters
* @OPCUAServerTest@: on @Execute@ tests there is no check to confirm that data has been updated correctly
* @OPCUAClientReadTest@, @OPCUAClientWriteTest@: there is no check to confirm that data has been read/written correctly on server
_GTest report_
<pre>
[----------] Global test environment tear-down
[==========] 154 tests from 11 test cases ran. (135431 ms total)
[ PASSED ] 154 tests.
</pre>
NONE
Implement an OPCUA interface for signals and StructuredDataI.
h1. Source code files modified
h1. Architecture & design review
*Date of the review:* //2019
*Person who did the review:* Andre' Neto
*Version of architecture & design document:* N/A. Strategy and main ideas for implementation discussed with B. Bauvir in informal meetings.
*Result of review:* N/A
*List of non-conformities:* N/A
h1. Code and documentation review
*Date of the review:* 13/07/2020 //2019
*Person who did the review:* Dídac Magriñá
*Result of review:* PASS
*List of non-conformities:*
+Possible Memory Issues+:
* @OPCUAClientI@
** @GetReferences@:
*** missing check to number of nodes before accessing @bReq.nodesToBrowse@
* @OPCUAClientMethod@, @OPCUAClientRead@, @OPCUAClientWrite@
** Destructor (of three mentioned classes):
*** missing check if @tempVariant@ and @monitoredNodes@ are allocated before deleting them
** @GetExtensionObjectByteString@ (of three mentioned classes):
*** missing check if @index@ is within range before accessing @entryTypes@ and @entryArrayElements@
*** missing check if @valueMemories[nodeCounter]@ is not null before accessing
*** missing check if @tempDataPtr[nOfBytes]@ is not null before accessing
** @SetServiceRequest@ (of three mentioned classes):
*** missing delete @bReq.nodesToBrowse@
** @OPCUAClientMethod::SetObjectRequest@
*** missing delete @tempStringnodeId@ after its allocation in @OPCUAClientI::GetReferences@
*** missing delete @ombReq.nodesToBrowse@
** @OPCUAClientMethod::SetMethodRequest@
*** missing delete @tempStringnodeId@ after its allocation in @OPCUAClientI::GetReferences@
*** missing delete @mbReq.nodesToBrowse@
** @OPCUAClientWrite::SetWriteRequest@
*** missing check if @writeValues[idx]@ is not null before accessing
* @OPCUADSInput@, @OPCUADSOutput@
** @GetSignalMemoryBuffer@ (of both classes)
*** missing check if @signalIdx@ is within range before accessing @types@ and @nElements@
** @GetStructure@ (of both classes)
*** missing check if @index@ is within range before accessing @entryArrayElements@, @entryTypes@, and @entryNumberOfMembers@
* @OPCUANode@
** Destructor
*** missing delete of @settings->attr.arrayDimensions@ created on @OPCUANode::InitArray@
*** missing delete of @settings->attr.value.arrayDimensions@ created on @OPCUANode::InitArray@
* @OPCUAClient@
** @MapStructuredData@
*** missing check if @index@ is within range before accessing @entryArrayElements@, @entryTypes@, and @entryMemberNames@
+Documentation Improvements+:
* Document structures in @OPCUATypes.h@ following Doxygen style
* Confirm that multiple extension objects are not supported as per @OPCUADSInput@/@OPCUADSOutput@ documentation. If they are supported, consider adding a test for this use case.
<pre>
When using Complex DataType Extension, the DataSource only allows to write 1 structure. If you need to add more signals you must add another OPCUADSInput DataSource to your real time application.
</pre>
+Refactoring Suggestions+:
* @OPCUAClientRead@, @OPCUAClientWrite@, and @OPCUAClientMethod@ have very similar methods that could be moved up to their interface @OPCUAClientI@:
** SetServiceRequest
** GetExtensionObjectByteString
h1. Unit test review
*Date of the review:* 13/07/2019 //2019
*Person who did the review:* Dídac Magriñá
*Result of coverage tests review:* PASS
*Result of functional tests review:* PASS
*Result of review:* PASS
*List of non-conformities:*
* @OPCUAMessageClient@: coverage lower than 90% (82.9%)
* The mock OPCUA server used in other tests could be leveraged on @OPCUAClientMethodTest@, @OPCUAMessageClientTest@ to check if UpdatePoint method has been called with expected parameters
* @OPCUAServerTest@: on @Execute@ tests there is no check to confirm that data has been updated correctly
* @OPCUAClientReadTest@, @OPCUAClientWriteTest@: there is no check to confirm that data has been read/written correctly on server
_GTest report_
<pre>
[----------] Global test environment tear-down
[==========] 154 tests from 11 test cases ran. (135431 ms total)
[ PASSED ] 154 tests.
</pre>
NONE