Repository navigation
Adding Digital Filtering in NI-Scope - #2234
NI-velmurugan wants to merge 3 commits into
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #2234 +/- ##
==========================================
+ Coverage 86.40% 88.12% +1.72%
==========================================
Files 86 88 +2
Lines 21563 22830 +1267
==========================================
+ Hits 18631 20119 +1488
+ Misses 2932 2711 -221
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
| def _update_chart(waveform_axes, waveform_line, filtered, spectrum_axes, spectrum_line, spectrum): | ||
| for axes, line, waveforms in ((waveform_axes, waveform_line, filtered), (spectrum_axes, spectrum_line, spectrum)): | ||
| for waveform in waveforms: | ||
| x = [waveform.relative_initial_x + i * waveform.x_increment for i in range(len(waveform.samples))] | ||
| line.set_data(x, waveform.samples) | ||
| axes.relim() | ||
| axes.autoscale_view() |
There was a problem hiding this comment.
Currently, each waveform is written to the same plot line. If multiple channels are selected, each update replaces the previous one, so only the last channel is shown. Could we create or update one line per returned waveform, and label each line by channel, so all selected channels are visible and identifiable?
There was a problem hiding this comment.
Something like,
def _update_chart(axes, waveforms):
while len(axes.lines) < len(waveforms):
axes.plot([], [])
for line, waveform in zip(axes.lines, waveforms):
x = [waveform.relative_initial_x + i * waveform.x_increment for i in range(len(waveform.samples))]
line.set_data(x, waveform.samples)
for line in axes.lines[len(waveforms):]:
line.set_data([], [])
axes.relim()
axes.autoscale_view()and updating the parameters accordingly in other instances.
There was a problem hiding this comment.
Updated the code to display list of channels
| parser.add_argument('-t', '--fir-taps', default=25, type=int, help='Defines the number of taps (coefficients) for an FIR filter.') | ||
| parser.add_argument('-fw', '--fir-window', default='HANNING', choices=niscope.FIRFilterWindow.__members__.keys(), type=str.upper, help='Specifies the FIR window type.') | ||
| parser.add_argument('-o', '--iir-order', default=2, type=int, help='Specifies the order of an IIR filter.') | ||
| parser.add_argument('-fft', '--fft-function', default='DB', choices=FFT_FUNCTIONS.keys(), type=str.upper, help='FFT amplitude spectrum function') |
There was a problem hiding this comment.
For this example, is this the same function? Can we remove this from the CLI parameters as we did in the other PRs?
There was a problem hiding this comment.
nit: Even if we have this as a CLI parameter, let's move it to the top to follow the similar order in the LabVIEW example for better readability.
There was a problem hiding this comment.
I have rearranged the parameters to match the LabVIEW example.
- [ ] I've added tests applicable for this pull requestWhat does this Pull Request accomplish?
This PR adds
niscope_digital_filtering.py, a Python adaptation of the LabVIEWniScope EX Digital Filtering.vialong with the pipeline changes required for usingmatplotlibpackage.What testing has been done?
Manually tested the example using simulated hardware and the real hardware