Introduce a new attribute for IntermediateDevice defining the minimum clock high time

[philipstarkey]

Introduce a new attribute for IntermediateDevices that allows them to specify a minimum clock high time in order to allow asymmetric clock ticks when combined with gated clocks on a pseudoclock. minimum_clock_high_time defaults to half of the minimum time between IntermediateDevice instructions. It be backwards compatible with previous versions of labscript devices.

Fixes #51.
Fixes #85.

I also discovered a bug where the check against the next all change time did not capture the last change time on the clock line because the stop time had not yet been added. This is now fixed.

There were also some issues with various error messages. I've fixed those too and moved to use f strings so they're more readable.

The whole Pseudoclock.collect_change_times method has also been reformatted in line with how black would format it (since I was working on it anyway). Some commented out code was also removed here too.

It might also solves #53 but I would appreciate @dihm to verify that as it seemed like it might have been slightly different.

I don't have an apparatus to test on so would appreciate wide testing if possible. Should be tested in conjunction with a forthcoming patch to labscript_devices.

[dihm]

It will be a bit before I can actually test this on hardware. In the meantime, I want to make sure I know how to actually build a test that fails under current code but succeeds here. I don't think any of our current experiments are butting into this problem anymore by design so I'll need to craft something from scratch.

Is the basic test to use a PB to clock a Novatech and a DAQ, command a novatech update at time t, the command an update do a DAQ output at time t+tau, with tau varying from say 120us down to 1us? Are there any special edge cases that I should be looking out for? I've looked through the linked issues, but I suspect reproducing any of those specific situations may get rather tricky since most don't actually include scripts or shot files that produce the error.

[philipstarkey]

Yep, that's all that you should need to do. Should currently fail when tau is less than 100us, but not with these PRs. There really shouldn't be any bugs, I've only changed error messages and where the stop time is inserted into the change time array. But it's possible there is a bug in how I calculate the default min high time for devices other than the NovaTech or something else like that 🤷

#53 was your issue report originally so if you don't have something current to reproduce that I think I'd like to just say it was a duplicate of #51 (maybe it was a shutter offset or something that made it seem like it was different).

[dihm]

This appears to be working for the standard case. Here is my test script:

  start()
  t = 0

  ao0.constant(t,0)

  dds.setfreq(t, 10*MHz)
  dds.setamp(t, 0.1)
  dds.setphase(t, 0)

  t += start_delay
  pb_2.go_high(t)

  dds.setamp(t, 0.3)

  t += tau

  ao0.constant(t, 0.5)

  t += hold_time
  
  pb_2.go_low(t)

  stop(t+1e-3)

I'm checking outputs on a scope, so I'm changing the DDS amplitude as it is easier to see. I then set a constant AnalogOut (on another clockline from the same pulseblaster pseudoclock) to change a time tau after the DDS update. With these changes, tau can be set as low as 2 us. Anything less throws the error. I'm assuming that is the correct logic for a minimum_high_time=1e-6 for the novatech (so the code prevents updates for twice the set minimum high time).

I haven't tried to recreate any of the specific scenarios mentioned above. If I can find time I'll try, but I think this can be merged now (assuming logic is right) and fixed later if I find anything in further testing.

[dihm]

Forgot to upload the scope trace for the above testing.

Red is pb_2, blue is the novatech output, and green is the DAQ output. tau = 2*us

[dihm]

@philipstarkey Browsing old PRs and found this. Are you OK to merge?

[philipstarkey]

I think so 🤷

[dihm]

Cool. Let's do it!