-
Notifications
You must be signed in to change notification settings - Fork 13
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
[rfc] - Adding OSA driver to config file #37
Comments
You mean something like the syntax used in the artiq device_db (module + class + kwargs)? That's something I'd been thinking about after your last PR. I could see a PR along those lines getting merged. The one warning I have is that some parts of the server code have grown "organically", with the aims/scope of the project changing around bits of old code. As a result some parts have gotten to be a bit of a mess and I've mentally flagged them as needing restructuring before any other changes are made to them (otherwise one finds that new code becomes shitty to work around shitty old code and the maintenance burden of finally fixing the issue grows over time). A PR to fix this can probably avoid hitting those parts, but I'd have to see it to know. tl;dr be prepared that this is something that may not be mergeable without some restructuring work on the code first which I won't have time for a while... |
(the simple you make it the more chance of it getting merged quickly is of course!) |
Another related question is whether we really want to keep the OSAs as an option or just depreciate them and use the WLM interferometer to make life easier (and cheaper!). We're now using the interferometer patterns in our recent experiments and they do the job fine with much less fuss |
Finally, I'm not totally against merging your driver in addition to this codebase, but I'd need to understand more about your setup (AFAICT it was completely undocumented in your last PR which was never going to fly), why you chose to do this rather than the NI DAQ setup or the WLM interferometer, whether this is something that other groups will actually use, etc I am a little hesitant about merging new hardware configurations since I can't test them. There are already a number of different configurations in the wild -- between each HF WLM having its own "personality" and switch/etc configurations -- so I usually find that any change to the code breaks someone's HW configuration in a way I can't test and only find out about months later once I've already forgotten what changes I made. But, I am open to being convinced to add to them if there is a good reason to outweigh this. |
This applies to the switches as well as the OSAs. I can't edit the title to reflect that though.
I guess that's the point. If we could choose any OSA and Switch driver from the config file (instead of just the hardcoded options), then we wouldn't have to merge our nasty untestable drivers and you'd never have to worry about them. And on our side we wouldn't have to maintain a WAND fork to support them. |
In an ideal world, I think we would have a well-thought through approach for all of this. e.g. if stacked switches are useful we could implement them in a nice way. However, if no one has time for that then I'm fine with allowing arbitrary drivers to be specified in the config. Copy the code from the artiq device db since that's pretty nice. |
I think this switch configuration could be broadly useful, even for people who don't want the OSAs. This is because it allows you to go single mode all the way to the wavemeter with many different wavelengths. Toptica will sell you a PCF switch but it's expensive and only has 8 channels (which isn't enough for us). |
We were actually going to use a Stabilizer instead of an NI card but the firmware isn't ready so we just built something out of those Kiethley and Rigol instruments. That Kiethley DMM is actually pretty nice - a 1MS/s 16-bit digitizer with an ethernet interface for ~$800. Avoids NI dependencies too.
Well, the OSAs give a much nicer signal (especially when you are coupling into them from SM fiber) and there is no cost saving for us as we had already bought them by the time you added this feature to WAND. We may also think about using them do do a transfer lock on the 397 if the wlm lock isn't good enough for us. |
How about we start there and then we can push the stacked switch in the future driver if we can make it sufficiently general and non-hacky. |
The Thorlabs amplifier and the photodiodes the OSAs ship with are also pretty far from optimal. |
After building a driver to run the setup we are using here at the Oregon we though it might be useful to add the ability to just specify the OSA driver in the server configuration instead of having to hard code it in. This would allow for any OSA setup to be easily supported.
The text was updated successfully, but these errors were encountered: