Added AnalogQuantity.square_wave().

[zakv]

This PR adds support for a square_wave() waveform for analog outputs. It is of course already possible to achieve a square wave output using AnalogQuantity.customramp(), but getting it right takes a bit of thought, particularly accounting for rounding errors in the timings. Therefore it's useful to have a built-in method for them.

The arguments that set the output values are called level_0 and level_1 which isn't the prettiest. I went back and forth over a few different ideas on how users should set the output levels but decided that was the most clear and simple way. I think amplitude and offset was somewhat ambiguous (is offset the midpoint or low level? Is amplitude peak-peak or is it half that like a sine wave?). I also think high_level and low_level isn't ideal because then you'd probably want another option like start_high=False, which can be misleading if the initial phase isn't zero, and what should be done if the user sets high_level to something smaller than low_level? Also, initial_level and final_level aren't great since the actual final output value may be equal to initial_level if there is a non-integer number of cycles. I'm all ears though if someone has a better suggestion for those arguments.

[philipstarkey]

Seems like a good idea!

~What about parameterising it with an amplitude and offset? That might be more consistent with the other functions and solve the kwarg naming concerns?~

[philipstarkey]

...just realised you covered that in your message. Oops sorry. Ignore me!

[zakv]

No worries, amplitude and offset might still be a better way to go despite the slight ambiguity given that the docstring will explain what they mean. In that case it might make sense to have the phase go from 0 to 2 pi then for better analogy with sine.

[dihm]

So, I think this is probably pretty easy to get merged and would love to do so. @zakv, did you come down on a choice for how to paramterize?

My 2 cents (feel free to ignore), is to use amplitude and offset. That is most familiar to me as it closely approximates the standard behavior of every function generator I've ever used. As you say, a docstring should be more than enough to stamp out any potential confusion.

Another option (that hopefully doesn't require too much work) is to just have two versions: one for each parameterization. If manageable, I suppose it could be one and you just do the conversion between them internally. I'd still personally just do the former and be done with it, but if you can't decide, that's probably a good sign that both are needed. Besides, many function generators let you parameterize both ways anyhow.

[zakv]

Sounds like that's two votes for amplitude/offset rather than level_0/level_1! I think the two-method option seems reasonable given that function generators have that behavior and a similar thing was done with exp_ramp() and exp_ramp_t(). Also I've already set it up with level_0/level_1 so leaving that in isn't much work.

I don't have a function generator handy so I think it'd be good to double check the expected behavior. Suppose the amplitude is 0.5 and the offset is 1. In that case, what would the high/low levels be? And should it start low or start high? And if the duty cycle is 0.1 it should be in the low state 90% of the time and in the high state 10% of the time (assuming amplitude is set to a positive value) right? And how should negative amplitudes be handled?

[dihm]

Here is an example screen from a Tektronix AFG that I clipped from the manual.

It defines the amplitude as the peak-to-peak value, the offset as the DC component. So 0.5 and 1 would give a high/low of 1.25/0.75.

As for start, I'd say start high since you typically code the start for when something actually happens. That can be tuned by adjusting the phase parameter (default being zero, starting high at time zero). So if you wanted it to start low, you could just set phase to pi. I think it is convention to define duty cycle as percentage on, so 10% duty cycle is 10% on, 90% off.

Personally, I think it should enforce positive amplitude and leave setting negative values to appropriately setting the offset.

Finally, this may be a bit too much, but you could probably add a debug kwarg that prints the equivalent high/low values. That should help users ensure what they have programmed is what they actually want without having to fire up runviewer.

[dihm]

Actually, looking at that screen clip and thinking a moment, maybe a delay setting instead of a phase would be easier to parse for a square wave?

[zakv]

Thanks for the suggestions! I've updated the code to have to parameterizations: square_wave() which uses amplitude/offset and square_wave_levels() which uses level_0/level_1. I agree with your point that square_wave() should start HIGH when the initial phase is set to zero, so it does so.

For now no error is raised if amplitude is set to a negative value since that is probably an easy/convenient hack for starting LOW rather than HIGH (although you can do the same thing by adjusting the phase, you have to think for ~30 seconds about whether you need to set phase = duty_cycle or phase = 1 - duty_cycle).

Currently the methods are setup with a phase argument rather than a delay argument, mainly just because that's how I had already set it up. I think specifying the phase is maybe marginally more convenient. For example if you use the delay argument to start LOW instead of HIGH, then you have to calculate the required delay from the frequency. It's not hard to convert between phase and delay though so I think either would be reasonable.