niscope: expand channels repeated_capability before populating channel and record info in fetched data (#1958)

* Fix fetch, read?

* Fix fetch_array_measurement?

* Fix fetch_measurement_stats?

* fix fetch_into?

* Update changelog in anticipation of fix

* enable correct test cases for catching #1770

* Use subtemplate for population of channel and record info in fetched data

* remove mention of niscope from changelog fix description

* Remove get_channel_names TODO from tests

Author: ni-jfitzger

CHANGELOG.md

@@ -61,6 +61,7 @@ All notable changes to this project will be documented in this file.
     * #### Added
         * `get_channel_names()`
     * #### Changed
+        * Fix [#1770](https://github.com/ni/nimi-python/issues/1770): fetch(), read(), and friends return wrong data when called with channel ranges on multi-instrument session.
     * #### Removed
 * ### `niswitch` (NI-SWITCH)
     * #### Added

generated/niscope/niscope/session.py

@@ -2132,12 +2132,17 @@ def fetch(self, num_samples=None, relative_to=enums.FetchRelativeTo.PRETRIGGER,
 
         waveform_info._populate_samples_info(wfm_info, mv, num_samples)
 
-        lwfm_i = len(wfm_info)
-        lrcl = len(self._repeated_capability_list)
+        channel_names = _converters.expand_channel_string(
+            self._repeated_capability,
+            self._all_channels_in_session
+        )
+
+        wfm_info_count = len(wfm_info)
+        channel_count = len(channel_names)
         # Should this raise instead? If this asserts, is it the users fault?
-        assert lwfm_i % lrcl == 0, 'Number of waveforms should be evenly divisible by the number of channels: len(wfm_info) == {0}, len(self._repeated_capability_list) == {1}'.format(lwfm_i, lrcl)
-        actual_num_records = int(lwfm_i / lrcl)
-        waveform_info._populate_channel_and_record_info(wfm_info, self._repeated_capability_list, range(record_number, record_number + actual_num_records))
+        assert wfm_info_count % channel_count == 0, 'Number of waveforms should be evenly divisible by the number of channels: len(wfm_info) == {0}, len(channel_names) == {1}'.format(wfm_info_count, channel_count)
+        actual_num_records = int(wfm_info_count / channel_count)
+        waveform_info._populate_channel_and_record_info(wfm_info, channel_names, range(record_number, record_number + actual_num_records))
 
         return wfm_info
 
@@ -2231,11 +2236,17 @@ def fetch_array_measurement(self, array_meas_function, meas_wfm_size=None, relat
         record_length = int(len(meas_wfm) / len(wfm_info))
         waveform_info._populate_samples_info(wfm_info, meas_wfm, record_length)
 
+        channel_names = _converters.expand_channel_string(
+            self._repeated_capability,
+            self._all_channels_in_session
+        )
+
         wfm_info_count = len(wfm_info)
-        channel_count = len(self._repeated_capability_list)
-        assert wfm_info_count % channel_count == 0, 'Number of waveforms should be evenly divisible by the number of channels: len(wfm_info) == {0}, len(self._repeated_capability_list) == {1}'.format(wfm_info_count, channel_count)
+        channel_count = len(channel_names)
+        # Should this raise instead? If this asserts, is it the users fault?
+        assert wfm_info_count % channel_count == 0, 'Number of waveforms should be evenly divisible by the number of channels: len(wfm_info) == {0}, len(channel_names) == {1}'.format(wfm_info_count, channel_count)
         actual_num_records = int(wfm_info_count / channel_count)
-        waveform_info._populate_channel_and_record_info(wfm_info, self._repeated_capability_list, range(record_number, record_number + actual_num_records))
+        waveform_info._populate_channel_and_record_info(wfm_info, channel_names, range(record_number, record_number + actual_num_records))
 
         return wfm_info
 
@@ -2325,11 +2336,17 @@ def fetch_measurement_stats(self, scalar_meas_function, relative_to=enums.FetchR
             measurement_stat = measurement_stats.MeasurementStats(result, mean, stdev, min_val, max_val, num_in_stats)
             output.append(measurement_stat)
 
+        channel_names = _converters.expand_channel_string(
+            self._repeated_capability,
+            self._all_channels_in_session
+        )
+
         results_count = len(results)
-        channel_count = len(self._repeated_capability_list)
-        assert results_count % channel_count == 0, 'Number of results should be evenly divisible by the number of channels: len(results) == {0}, len(self._repeated_capability_list) == {1}'.format(results_count, channel_count)
+        channel_count = len(channel_names)
+        # Should this raise instead? If this asserts, is it the users fault?
+        assert results_count % channel_count == 0, 'Number of results should be evenly divisible by the number of channels: len(results) == {0}, len(channel_names) == {1}'.format(results_count, channel_count)
         actual_num_records = int(results_count / channel_count)
-        waveform_info._populate_channel_and_record_info(output, self._repeated_capability_list, range(record_number, record_number + actual_num_records))
+        waveform_info._populate_channel_and_record_info(output, channel_names, range(record_number, record_number + actual_num_records))
 
         return output
 
@@ -2434,12 +2451,17 @@ def read(self, num_samples=None, relative_to=enums.FetchRelativeTo.PRETRIGGER, o
 
         waveform_info._populate_samples_info(wfm_info, mv, num_samples)
 
-        lwfm_i = len(wfm_info)
-        lrcl = len(self._repeated_capability_list)
+        channel_names = _converters.expand_channel_string(
+            self._repeated_capability,
+            self._all_channels_in_session
+        )
+
+        wfm_info_count = len(wfm_info)
+        channel_count = len(channel_names)
         # Should this raise instead? If this asserts, is it the users fault?
-        assert lwfm_i % lrcl == 0, 'Number of waveforms should be evenly divisible by the number of channels: len(wfm_info) == {0}, len(self._repeated_capability_list) == {1}'.format(lwfm_i, lrcl)
-        actual_num_records = int(lwfm_i / lrcl)
-        waveform_info._populate_channel_and_record_info(wfm_info, self._repeated_capability_list, range(record_number, record_number + actual_num_records))
+        assert wfm_info_count % channel_count == 0, 'Number of waveforms should be evenly divisible by the number of channels: len(wfm_info) == {0}, len(channel_names) == {1}'.format(wfm_info_count, channel_count)
+        actual_num_records = int(wfm_info_count / channel_count)
+        waveform_info._populate_channel_and_record_info(wfm_info, channel_names, range(record_number, record_number + actual_num_records))
 
         return wfm_info
 
@@ -3147,11 +3169,17 @@ def fetch_into(self, waveform, relative_to=enums.FetchRelativeTo.PRETRIGGER, off
 
         waveform_info._populate_samples_info(wfm_info, mv, num_samples)
 
-        lwfm_i = len(wfm_info)
-        lrcl = len(self._repeated_capability_list)
-        assert lwfm_i % lrcl == 0, 'Number of waveforms should be evenly divisible by the number of channels: len(wfm_info) == {0}, len(self._repeated_capability_list) == {1}'.format(lwfm_i, lrcl)
-        actual_num_records = int(lwfm_i / lrcl)
-        waveform_info._populate_channel_and_record_info(wfm_info, self._repeated_capability_list, range(record_number, record_number + actual_num_records))
+        channel_names = _converters.expand_channel_string(
+            self._repeated_capability,
+            self._all_channels_in_session
+        )
+
+        wfm_info_count = len(wfm_info)
+        channel_count = len(channel_names)
+        # Should this raise instead? If this asserts, is it the users fault?
+        assert wfm_info_count % channel_count == 0, 'Number of waveforms should be evenly divisible by the number of channels: len(wfm_info) == {0}, len(channel_names) == {1}'.format(wfm_info_count, channel_count)
+        actual_num_records = int(wfm_info_count / channel_count)
+        waveform_info._populate_channel_and_record_info(wfm_info, channel_names, range(record_number, record_number + actual_num_records))
 
         return wfm_info
 

src/niscope/system_tests/test_system_niscope.py

@@ -18,12 +18,10 @@
 
 
 instruments = ['FakeDevice1', 'FakeDevice2']
-# TODO(sbethur): Use `get_channel_names` when #1402 is fixed
 test_channels_1 = 'FakeDevice2/0,FakeDevice1/1'
 test_channels_1_expanded = test_channels_1
-# TODO(jfitzger): Use the commented values, once #1770 is fixed
-test_channels_2 = 'FakeDevice2/0'  # 'FakeDevice2/0:1'
-test_channels_2_expanded = 'FakeDevice2/0'  # 'FakeDevice2/0,FakeDevice2/1'
+test_channels_2 = 'FakeDevice2/0:1'
+test_channels_2_expanded = 'FakeDevice2/0,FakeDevice2/1'
 
 
 # There are system tests below that need either a PXI-5124 or a PXI-5142 instead of the PXIe-5164 we use everywhere else

src/niscope/templates/session.py/fancy_fetch.py.mako

@@ -28,11 +28,6 @@
 
         waveform_info._populate_samples_info(wfm_info, mv, num_samples)
 
-        lwfm_i = len(wfm_info)
-        lrcl = len(self._repeated_capability_list)
-        # Should this raise instead? If this asserts, is it the users fault?
-        assert lwfm_i % lrcl == 0, 'Number of waveforms should be evenly divisible by the number of channels: len(wfm_info) == {0}, len(self._repeated_capability_list) == {1}'.format(lwfm_i, lrcl)
-        actual_num_records = int(lwfm_i / lrcl)
-        waveform_info._populate_channel_and_record_info(wfm_info, self._repeated_capability_list, range(record_number, record_number + actual_num_records))
+<%include file="./fetch_waveform_info_population.py.mako"/>
 
         return wfm_info

src/niscope/templates/session.py/fancy_fetch_array_measurement.py.mako

@@ -27,10 +27,6 @@
         record_length = int(len(meas_wfm) / len(wfm_info))
         waveform_info._populate_samples_info(wfm_info, meas_wfm, record_length)
 
-        wfm_info_count = len(wfm_info)
-        channel_count = len(self._repeated_capability_list)
-        assert wfm_info_count % channel_count == 0, 'Number of waveforms should be evenly divisible by the number of channels: len(wfm_info) == {0}, len(self._repeated_capability_list) == {1}'.format(wfm_info_count, channel_count)
-        actual_num_records = int(wfm_info_count / channel_count)
-        waveform_info._populate_channel_and_record_info(wfm_info, self._repeated_capability_list, range(record_number, record_number + actual_num_records))
+<%include file="./fetch_waveform_info_population.py.mako"/>
 
         return wfm_info

src/niscope/templates/session.py/fancy_fetch_measurement_stats.py.mako

@@ -23,10 +23,6 @@
             measurement_stat = measurement_stats.MeasurementStats(result, mean, stdev, min_val, max_val, num_in_stats)
             output.append(measurement_stat)
 
-        results_count = len(results)
-        channel_count = len(self._repeated_capability_list)
-        assert results_count % channel_count == 0, 'Number of results should be evenly divisible by the number of channels: len(results) == {0}, len(self._repeated_capability_list) == {1}'.format(results_count, channel_count)
-        actual_num_records = int(results_count / channel_count)
-        waveform_info._populate_channel_and_record_info(output, self._repeated_capability_list, range(record_number, record_number + actual_num_records))
+<%include file="./fetch_waveform_info_population.py.mako" args="results_name='results', results_description='results', output_name='output'"/>
 
         return output

src/niscope/templates/session.py/fetch_waveform.py.mako

@@ -36,10 +36,6 @@
 
         waveform_info._populate_samples_info(wfm_info, mv, num_samples)
 
-        lwfm_i = len(wfm_info)
-        lrcl = len(self._repeated_capability_list)
-        assert lwfm_i % lrcl == 0, 'Number of waveforms should be evenly divisible by the number of channels: len(wfm_info) == {0}, len(self._repeated_capability_list) == {1}'.format(lwfm_i, lrcl)
-        actual_num_records = int(lwfm_i / lrcl)
-        waveform_info._populate_channel_and_record_info(wfm_info, self._repeated_capability_list, range(record_number, record_number + actual_num_records))
+<%include file="./fetch_waveform_info_population.py.mako"/>
 
         return wfm_info

src/niscope/templates/session.py/fetch_waveform_info_population.py.mako

@@ -0,0 +1,15 @@
+<%page args="results_name='wfm_info', results_description='waveforms', output_name='wfm_info'"/>\
+<%
+    '''Sub-template to populate channel and record info for a waveform in a fetch method.'''
+%>\
+        channel_names = _converters.expand_channel_string(
+            self._repeated_capability,
+            self._all_channels_in_session
+        )
+
+        ${results_name}_count = len(${results_name})
+        channel_count = len(channel_names)
+        # Should this raise instead? If this asserts, is it the users fault?
+        assert ${results_name}_count % channel_count == 0, 'Number of ${results_description} should be evenly divisible by the number of channels: len(${results_name}) == {0}, len(channel_names) == {1}'.format(${results_name}_count, channel_count)
+        actual_num_records = int(${results_name}_count / channel_count)
+        waveform_info._populate_channel_and_record_info(${output_name}, channel_names, range(record_number, record_number + actual_num_records))