More Set Commands and Binary Transfers

[carterturn]

This pull request attempts to resolve #35 and #44.

First, set_ins is split into different functions for different modes. Each of these functions is then split into a floating point to integer function (used by the set command) and an integer to SPI instruction function (used by the 'seti' command and binary programming).

Second, the seti (set-integer) command is added. This allows instructions to be programmed with integer values. The set command then has its separate delta and rate parameters combined into a single sweep_rate floating point parameter. This could be done in a way that allows for greater precision, but for the sort of testing set is likely to be used for, it may be fine as is.

Finally, the setb (set-binary) command is added. This allows instructions to be programmed in a large block without ascii encoding. The number of instructions is sent first, allowing the Pi Pico to check if there is enough memory available.

[carterturn]

I have tested set and seti in all modes, but setb has only been tested for single step mode. There, it allows filling Pi Pico RAM in <0.3s (which I think corresponds to a ~3x higher transmission rate than PrawnDO).

Documentation is not up to date yet. I think I should also refactor this a bit. It might make sense to move processing for the set, seti, and setb commands into their own functions and see if there is a way to reduce serial command error checking code duplication, as dds-sweeper.c is becoming a bit long.

[dihm]

Apologies for being slow @carterturn. I have finally had a moment to look at this. Overall I think this is a huge leap forward. The process is fairly easy to follow (or at least as easy as it could be, the logic for all the modes is pretty deep) and I greatly appreciate that higher level abstraction functions (like set) re-use all the code from the lower level ones, which should make debugging and testing quite a bit easier.

Mostly to ensure I am following along, I'm going to try to document the call chains:

• set: highest level of abstraction, user sets inputs with floats
  • single step mode
  • get_asf, get_ftw, get_pow: converts floats to Amp scale factor, freq. tuning word, phase offset word
  • calls lower level set_single_step_ins and set_time to move to memory
  • sweep modes
  • parse_amp_sweep_ins, parse_freq_sweep_ins, parse_phase_sweep_ins
• seti: intermediate level of abstraction, user sets tuning words, but in ascii
  • single step mode
  • set_single_step_ins, set_time: puts tuning words into instruction memory
  • sweep modes
  • set_amp_sweep_ins, set_freq_sweep_ins, set_phase_sweep_ins: puts tuning words and sweep words in memory
• setb: lowest level of abstraction, user pipes binary data directly into memory
  • Same instructions as seti, but directly copied off the buffer

I'm happy to follow your lead on refactoring, since you are the one mucking around and know what the pain points are. I agree that some refactoring is probably warranted. Personally, I'd consider moving the the set_xxxxx_ins, parse_xxxxx_ins, and set_time commands into a new file, though I understand if that is too annoying since the instruction memory pointer would need to be tracked differently. In any case, it does seem like much of the setb logic could be delegated to a similar set of functions to help shorten the code there.

Finally, I may be much mistaken, but I believe the set and seti sweep modes ignore the time parameter in all cases.

[dihm]

Also, I am pondering how to be more helpful. I suspect getting into the weeds of the firmware is not going to be all that productive, and likely result in longer delays as I try to keep up with you.

I'm thinking it might be better for me to focus on setting up our simultaneous amp/freq/phase test rig and developing out a fuller suite of much more automated tests. I'd start with the tests.ipynb in the repo and work from there. If agreeable, I'd probably only have to lean on you for initial code snippets for creating the binary commands and such.

[carterturn]

Thank you for the feedback so far.

Those are the intended workings of the call chains, glad that makes sense (I should still add a few more comments so it does not have to be re-derived). You are correct that I neglected the set_time call for the sweeps for set and seti; I will get that fixed soon (it should be trivial).

I think the largest functional refactor I would consider doing at this point would be to parameterize and then combine the serial parsing code. However, this seems more complex for no benefit besides reducing the amount of code to read through, and I am not sure how much that could happen given that human readable strings must be generated. Given that, it probably makes sense to keep the serial parsing as it is and move some of the parsing/setting functions into a different file. Perhaps it would even work well to move instructions into its own file (with the associated "parsing" and "setting" functions), then add "getting" functions to retrieve instructions? This might also improve future edit-ability by making the scope of instructions clear.

More automated testing would be very beneficial! I have a BLACS worker for stepping mode here: https://github.com/carterturn/zwierlein_labscript_user_devices/blob/ad9959-eval/AD9959EvalDDS/blacs_workers.py. I think it should be fairly similar for the sweeps (though that is actually entirely untested).

It would probably be good to also add a "get" command for testing. I'll do that too in the next few days.