search

Exopy / exopy_hqc_legacy

Transition package to smooth transition from HQCMeas to Exopy.
BSD 3-Clause "New" or "Revised" License
0 stars 9 forks source link

AWG events #56

Closed rassouly closed 3 years ago

rassouly commented 5 years ago

The code is a slightly modified version of the code written by Raphaël and Zaki. It changes the way the AWG is operated: from on/off to events. The sequences are usually built such that receiving an event starts playing the sequences in order and receiving a second event stops and rearms the AWG.

The code is not fully tested yet but you can start reviewing it.

rassouly commented 4 years ago

This should be cleaned up. Some of the changes don't really make sense but they will in later PRs.

MatthieuDartiailh commented 4 years ago

Which PR related to the AWG should I review first @rassouly ?

rassouly commented 4 years ago

Which PR related to the AWG should I review first @rassouly ?

You should review this one first

rassouly commented 4 years ago

I finally have access to an AWG so I've restarted working on cleaning up the AWG changes. I still don't know whether I should add the new tasks in exopy_hqc_legacy (like we have currenly on our branch) or in exopy_pulses.

MatthieuDartiailh commented 4 years ago

Only generic tasks should go in pulses. I can have another look if you want. What task should I look at ?

rassouly commented 4 years ago

What task should I look at ?

I was talking about the TransferAWGFileTask (PR #57). The PR needs some work before you can review it though (it needs to be rebased on this patch and properly tested).

Since it's only applicable to the Tektro, I guess it can stay here in exopy_hqc_legacy.

rassouly commented 4 years ago

This has now been tested on real hardware.

MatthieuDartiailh commented 4 years ago

In that case yes let's keep it here. Should I review now ?

rassouly commented 4 years ago

Yes, you can review this PR now.

rassouly commented 4 years ago

I hadn't realize that the ask_for_values function was already deprecated. There are more than 200 uses of this function in this plugin alone. Should we replace them all now? Regarding Unicode -> Str, would a search and replace be enough to update the entire repo?

MatthieuDartiailh commented 4 years ago

Unicode to Str is trivial yes. The ask_for_values is tricker we should at least stop using it in new code. And if you can clean up the instrument you have access to that would be great.

rassouly commented 4 years ago

This is ready to be merged I think.

rassouly commented 3 years ago

This has been rebased on main, @MatthieuDartiailh, could you take a look at it now?

MatthieuDartiailh commented 3 years ago

Will do my best to review before next Wednesday.

rassouly commented 3 years ago

Could we get this merged now and ask for Zaki's review later? The Tektro is almost unusable on master so I doubt any tektro user is using master. They are all using code from other branches that has been cleaned up and is included in this PR.

MatthieuDartiailh commented 3 years ago

Let me ping people once just in case. I will merge by the end of the week in the absence of any comment.

@leghtas @MVillius @rlescanne please review if you can since this affects you.

MVillius commented 3 years ago

I have never used the AWG so as far as I am concerned I am fine with all these changes. Raph and Alice&Bob people are mostly using the QM I think, which leaves only Clarke with the AWG, but we will be fine with all this

MatthieuDartiailh commented 3 years ago

Thanks @MVillius