Fix for DAQmx input tasks #185
Conversation
Signed-off-by: Shalini-Subramanian <[email protected]>
Shalini-Subramanian
requested review from
lornazh and
Mattjet27
as code owners
Shalini-Subramanian
requested a deployment
to
gh-action-testing
GitHub Actions
Waiting
|
@lornazh For other drivers, this is already implemented. |
| @@ -22,6 +22,7 @@ public static PinSiteData<double[]> ReadAnalogMultiSample(this DAQmxTasksBundle | |||
| return tasksBundle.DoAndReturnPerSitePerPinResults(taskInfo => | |||
| { | |||
| taskInfo.VerifyTaskType(DAQmxTaskType.AnalogInput); | |||
| taskInfo.Task.Stream.ChannelsToRead = taskInfo.ChannelList; | |||
There was a problem hiding this comment.
I think we need to restore original channels after the read operation, otherwise subsequent reads using direct driver calls will be affected.
There was a problem hiding this comment.
@lornazh Updated as such, I have introduced new variable to store the All channels information.
As strings are immutable I've taken this approach. Since the type Tasks are of type mutable, it is taking shallow copy of the value that is taken.
Please let me know if there are any concerns.
There was a problem hiding this comment.
I'm a little confused, if you directly write the following, are you seeing value of availableChannels changing after the second line of code is executed?
string availableChannels = taskInfo.Task.Stream.ChannelsToRead;
taskInfo.Task.Stream.ChannelsToRead = taskInfo.ChannelList;
There was a problem hiding this comment.
Yes that' right. That's why I have taken a deep copy and updated accordingly.
SemiconductorTestLibrary.Extensions/source/InstrumentAbstraction/DAQmx/AnalogOutput.cs
Outdated
Show resolved
Hide resolved
|
Please add unit tests in Analog/DigitalInputTests to verify behavior. |
Signed-off-by: Shalini-Subramanian <[email protected]>
Shalini-Subramanian
requested a deployment
to
gh-action-testing
GitHub Actions
Waiting
Shalini-Subramanian
changed the title
@lornazh I have updated the unit tests based on comparing the simulated values. Hope it is fine. |
| { | ||
| var sessionManager = Initialize("DAQmxMultiChannelTests.pinmap"); | ||
| var tasksBundle = sessionManager.DAQmx(pinName); | ||
| var filteredResult = tasksBundle.FilterBySite(new int[] { 1, 3 }).ReadAnalogMultiSample(100); |
There was a problem hiding this comment.
Why do we read 100 samples?
There was a problem hiding this comment.
To visualize better using WDV, I have the samples as 100. I'll update it based on your comments on this.
| @@ -56,6 +57,21 @@ public void ReadFiveAnalogSamplesFromTwoChannels_ResultsContainExpectedData() | |||
| Assert.Equal(5, results.ExtractSite(1)["VCC2"].Length); | |||
| } | |||
|
|
|||
| [Theory] | |||
| [InlineData("AIPin")] | |||
| public void ReadTwoAnalogSamples_ChannelForMultipleSitesPerPinInstrument_ResultsContainExpectedData(string pinName) | |||
There was a problem hiding this comment.
Method name does not match method logic.
Suggest to rename to "ReadAnalogSamplesFromTwoFilteredChannels_ResultsContainExpectedData".
Please update all tests accordingly.
| Assert.Equal(sites.Length, results.SiteNumbers.Length); | ||
| } | ||
|
|
||
| private void Initialize(string pinMapFileName = "DAQmxSingleChannelTests.pinmap") |
There was a problem hiding this comment.
To be consistent, please change it to public and move it to the beginning of the test class.
|
I think we need to add tests to confirm channels to read is not affected after executing these read extension methods. |
What does this Pull Request accomplish?
This PR contains the implementation for updating the channels to be read with the filtered channel list, where the task contains channels for multiple sites for one pin and on applying FilterBySite while we get the measurements.
Why should this Pull Request be merged?
Previously, while getting the measurements for the Daqmx tasks, the channels to be read was not updated with the filtered channel list.
We are updating this logic to read/write channels for the filtered list alone.
What testing has been done?
Read measurements for all sites
Read measurements for filtered sites