-
Notifications
You must be signed in to change notification settings - Fork 125
Triggering at sig. gen. frequency #367
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Would an emulator (like https://developer.android.com/studio/run/emulator or https://www.ldplayer.net) be good enough/better for testing during development? |
|
Thanks for the tip – I'll give that a go. |
|
I haven't studied the code on the triggering functionality and so I have difficulty understanding the use of triggering at sig. gen. frequency. What added benefits does this PR brings? Where do we apply it? May be a simple example will make things much clearer |
|
Generally, triggering on something besides a plotted 'scope channel is not unheard of, https://blog.teledynelecroy.com/2022/05/oscilloscope-basics-external-line-and.html . Triggering at the sig. gen. frequency in particular eliminates the need to constantly adjust the trigger level as you're going through a lab activity. Say you're probing a low-pass filter by connecting 'scope sig. gen. OUT to the filter IN and connecting the filter OUT to 'scope ch. 1 IN. If you adjust the frequency, the amplitude of the measured waveform will change, and if rising/falling edge triggering is being used, the previous trigger level may no longer produce a good lock. Tying the triggering instead to the sig. gen. frequency makes the plotted waveforms automatically steady. The android build works, but for reasons unrelated to the specific app, I've not yet been able to get an emulator to work. |
|
Indeed that is a good addition to the triggering sources and I see its benefit when stabilizing the jitter on specific frequency sources (e.g. 7.9 Khz). |
|
I also see you made rearrangements to the genericusbdriver.cpp file. Is there an added benefit besides the reshuffling? If there isn't, I prefer to keep it as is. |
|
As for the rest, I recommend @EspoTek to look further at this PR since he is the original author on triggering functionality |
Originally, I thought it was easier to have functiongencontrol.cpp communicate the sig. gen. period and clock settings to the isodriver object, but now I see that it's probably just as easy to have them sent by genericusbdriver.cpp to isodriver. I'll try to minimize the number of changes to functiongen... and genericusb... relative to master. |
|
OK – I restored functiongencontrol.cpp and genericusbdriver.cpp to more or less their master branch versions. |
Desktop_Interface/isobuffer_file.cpp
Outdated
| { | ||
| //ignore singleBit for now | ||
| double timeBetweenSamples = (double) sampleWindow * samplesPerSecond / (double) numSamples; | ||
| double timeBetweenSamples = (double) sampleWindow * samplesPerSecond / ((double) numSamples - 1); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this change related to the PR?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, just something I noticed when working on the PR. I made the same change on line 162 of isobuffer.cpp. I'm fairly confident that dividing by (numSamples - 1) is correct. Next time, I'll be sure to create a separate PR even for minor things like this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know the function behind and thus I can't say the changes are correct or incorrect. I just noticed that it doesn't look to be part of the PR and the reason for my question. Nevertheless, I still recommend @EspoTek to look further at this PR.
@mi-hol Finally was able to get an emulator to work. Specifically used a virtual Nexus 10 with x86 architecture running Android SDK level 28. I wasn't able to get the emulator to run any device with armeabi-7va architecture, which at first seemed to be the most promising based on the Labrador build script, so tried x86 as a last-ditch effort and was very happy to see the device launch and the Labrador UI pop up. |
|
@EspoTek would you perhaps have time to review this PR in the coming 2 weeks? |
@brentfpage is above still the current state of testing? |
@mi-hol Just tried it on Android and everything went smoothly. |
|
@EspoTek I would be happy to make the PR a bit more conservative and just retain the core functionality. |
Yeah, I'm looking at it and it seems like the surgery is a little bit more extensive than necessary. That said, if it hasn't broken any of the old triggering code I think it might be worth merging. |
|
@EspoTek One thing that I wasn't able to figure out in the trigger code is why |
|
@EspoTek @mmehari @brentfpage it was a bit silent, hence I wonder if the previous comments have been resolved? |
… core sig gen triggering functionality is retained
|
Continued at #390 . The new PR has a cleaner file history and also essentially only makes changes/additions that are required for the sig. gen. trigger feature. |
Added new trigger type options, Sig. Gen. Ch1 and Ch2 , that set the trigger frequency equal to the respective sig. gen. frequencies. In these modes, there's no need to adjust the trigger level to get the waveform steady on the screen. While trying to debug the issue mentioned below, I changed the trigger position identifier from a list of trigger points to a boolean array with the same length as the buffer and in which True denotes a trigger point. This seems like the simplest way to ensure that every valid trigger point is kept around while old trigger points are cleared out.
I haven't run any tests on android – still need to get a device for that. I tried using existing code as a template for ensuring good functionality on android, but can't be sure it works.
The sig. gen. trigger feature gets finicky above ~25 khz or so, especially when 'scope channel 2 is enabled. I've dug around trying to find a cause, but haven't had any luck yet. The issue only appears in a limited operating range (>~ 25 khz), is not all that common even in that range, and can usually be resolved by disabling/enabling 'scope channel 2, so imo isn't a huge demerit for the PR.