Bug1070/nidigital remove redundant size param #1104
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1104 +/- ##
=======================================
Coverage 89.99% 89.99%
=======================================
Files 20 20
Lines 3679 3679
=======================================
Hits 3311 3311
Misses 368 368Continue to review full report at Codecov.
|
This reverts commit fd1c5b6
| @@ -664,11 +664,9 @@ def apply_tdr_offsets(self, num_offsets, offsets): | |||
| You can specify a subset of repeated capabilities using the Python index notation on an | |||
| nidigital.Session repeated capabilities container, and calling this method on the result.: | |||
|
|
|||
| session.channels[0,1].apply_tdr_offsets(num_offsets, offsets) | |||
| session.channels[0,1].apply_tdr_offsets(offsets) | |||
There was a problem hiding this comment.
I'm pretty confused why this is only a documentation change.
If you look at the old code to teh left, on line 677:
num_offsets_ctype = _visatype.ViInt32(0 if offsets is None else len(offsets)) # case S160
is the same as the right hand side.
Why was parameter num_offsets not being used?
Was the generator generating valid yet "incorrect" code?
Or more likely, should the generator have caught a mistake in metadata and broken at codegen time?
There was a problem hiding this comment.
{
'direction': 'in',
'name': 'offsets',
'size': {
'mechanism': 'passed-in',
'value': 'numOffsets'
},
'type': 'ViReal64[]'
}
This is how metadata looked like before my fix, which was to change mechanism to len. Generated code used passed in list to figure out the size that needs to be passed to C DLL, but also added numOffsets as API method parameter.
There was a problem hiding this comment.
The public method has a parameter num_offsets.
Metadata says 'size': {'mechanism': 'passed-in', 'value': 'numOffsets'}
But the generated code never uses numOffsets. In line 677 (left), variable num_offsets_ctype is initialized with the length of offsets.
marcoskirsch
deleted the
bug1070/nidigital_remove_redundant_size_param
branch
* Remove size param from apply_tdr_offsets() and write_source_waveform_broadcast_u32() * Add test support files * Updated Changelog * Remove size param from apply_tdr_offsets() and write_source_waveform_broadcast_u32() * Updated the os.path.join() call. * Revert "Add test support files" This reverts commit fd1c5b6 * Remove newly added system tests
What does this Pull Request accomplish?
Remove array-size parameter from
apply_tdr_offsetsandwrite_source_waveform_broadcast_u32methods.List issues fixed by this Pull Request below, if any.
What testing has been done?
Added system tests