Skip to content
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

ps3404D AWG Signal period wrong #186

Open
MitchiLaser opened this issue Jun 1, 2023 · 5 comments
Open

ps3404D AWG Signal period wrong #186

MitchiLaser opened this issue Jun 1, 2023 · 5 comments

Comments

@MitchiLaser
Copy link

MitchiLaser commented Jun 1, 2023

Hello,

although the ps3404D (not MSO) is not listed as a supported device I tried using its waveform generator. It works perfectly fine except for the fact that I have to divide time for the signal duration by the constant 5. My Code works completely fine on some ps2000a devices and a ps3204A.

I just wanted to notice this behavior, maybe it can be taken into account when it comes to the implementation for support for the ps3404D.

@hmaarrfk
Copy link
Collaborator

hmaarrfk commented Jun 4, 2023

our the device driver are hand written and manually vetted.

PRs welcome to address any issues with specific models.

Their capabilities vary quite a bit.

@prestegaard
Copy link

prestegaard commented Sep 8, 2023

Seeing similar issues with PS6424E.
The signal period is depending on number of samples in the buffer, and when using a small number of samples the period is off by around a factor 5.

Update:
After some finding out turned the driver from PicoScope does not the playback frequency per sample, but the period for the whole buffer. The PS6242E has a maximum playback frequency of 200 MHz, meaning 5 ns per sample point. If you try to play back a pattern faster than this it is truncated and the total playback time remains as specified.

@jcafhe
Copy link
Contributor

jcafhe commented Oct 31, 2023

Hi,

Currently I'm trying to implement the AWG capability for the ps4000a series, and I noticed an issue with the deltaphase calculation, which seems to lead to improper signal duration.

deltaPhase = int(samplingFrequency / self.AWGDACFrequency *
2 ** (self.AWGPhaseAccumulatorSize -
self.AWGBufferAddressWidth))

After digging in programmer's guides of the 3000a, 4000a, 6000 and 6000a series, it appears that the waveform length (number of samples) parameter is missing in the formula. Depending on the guide, the deltaPhase formula is rearranged differently, but all the variations are equivalent :

ps6000a guide (p.110):

deltaPhase = outputFrequency/dacFrequency × arbitraryWaveformSize × 2**(phaseAccumulatorSize - bufferAddressWidth)

With phaseAccumulatorSize and bufferAddressWidth as a number of bits.

or in ps3000a, ps4000a, ps6000 guides, respectively on pages 98, 101, 82:

deltaPhase = outputFrequency/dacFrequency × arbitraryWaveformSize × phaseAccumulatorSize / awgBufferSize

With phaseAccumulatorSize as a maximum value (e.g. 2**32), and awgBufferSize equivalent to 2**bufferAddressWidth.

From these two definitions, we can see that the arbitraryWaveformSize parameter is missing, i.e. the "length in samples of the user-defined waveform".

Since this calculation is done by the _PicoscopeBase, I imagine this bug affects all implementations of the AWG.

I can make a PR to fix this bug if needed, but it means modifying the signature of the "public" methods getAWGDeltaPhase() and getAWGTimeIncrement() by adding the waveform length as a parameter to the methods. This will affects all the implementation modules that enable AWG capability. Do you think it is acceptable or maybe there's an alternative ? @hmaarrfk @bmoneke @prestegaard @MitchiLaser

@jcafhe
Copy link
Contributor

jcafhe commented Oct 31, 2023

I guess we can just create two new methods with new names which compute deltaPhase and timeIncrement with the right formulas. The setAWGSimple and setAWGSimpleDeltaPhase methods will now use these new created methods.

For the legacy methods, we just throw a warning or a DeprecationWarning when one calls getAWGDeltaPhase or getAWGTimeIncrement directly, explaining that the returned values might not be valid, so we don't break anything but the user is notified.

@jcafhe
Copy link
Contributor

jcafhe commented Nov 3, 2023

My bad, after further investigations, it seems I was mistaken about the waveform size. In fact, it is already included in the sampling_interval parameter of the getAWGDeltaPhase method, which equal to outputFrequency × arbitraryWaveformSize.

I guess I was misleading because, as the op, I noticed some weird behavior when working with a waveform with a small number of samples at a low frequency, where the deltaPhase value tends to 0, i.e. where the resolution error becomes significant.

Although, the pico-python slightly differs from calling directly the clib function psXXXXSigGenFrequencyToPhase, where the deltaPhase value seems to be smartly rounded instead of using a floor function.

Round instead of floor would extends the limits of the generator with low frequency and small number of samples, at the expense of a increased error, but it would be aligned with the clib function (will try include this in a PR).

As a rule of thumb, it is preferable to work with a waveform discretized by the maximum number of sample allowed by the device, i.e. 2**psAWGBufferAddressWidth.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants