Skip to content
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

Update artiq.coredevice.urukul and artiq.coredevice.ad9910 for new Urukul capabilities #2657

Open
wants to merge 20 commits into
base: master
Choose a base branch
from

Conversation

newell
Copy link
Contributor

@newell newell commented Jan 22, 2025

ARTIQ Pull Request

Description of Changes

This PR adds coredevice driver capabilities and tests for new Urukul CPLD functionaly (see related issue).

such as:

  1. Decoupling the individual DDS channels.
  2. Individual DDS Channel:
    • PROFILE
    • IO_UPDATE
    • OSK
    • DRCTRL
    • DRHOLD
    • ATT_EN
  3. Digital Ramp Generator (DRG)
  4. Backward compatible Urukul coredevice driver that can work between protocol revision 8 and new editions.

Related Issue

quartiq/urukul#7

quartiq/urukul#12

Type of Changes

Type
✨ New feature

All Pull Requests

  • Use correct spelling and grammar.
  • Update RELEASE_NOTES.rst if there are noteworthy changes, especially if there are changes to existing APIs.

Code Changes

  • Run flake8 to check code style (follow PEP-8 style). flake8 has issues with parsing Migen/gateware code, ignore as necessary.
  • Test your changes or have someone test them. Mention what was tested and how.
  • Add and check docstrings and comments
  • Check, test, and update the unittests in /artiq/test/ or gateware simulations in /artiq/gateware/test

Licensing

See copyright & licensing for more info.
ARTIQ files that do not contain a license header are copyrighted by M-Labs Limited and are licensed under LGPLv3+.

@newell newell changed the title Urukul: Update artiq.coredevice.urukul and artiq.coredevice.ad9910 for new Urukul capabilities Update artiq.coredevice.urukul and artiq.coredevice.ad9910 for new Urukul capabilities Jan 22, 2025
@sbourdeauducq
Copy link
Member

CPLD: quartiq/urukul#12

@Spaqin
Copy link
Collaborator

Spaqin commented Jan 23, 2025

Has this been tested with real hardware?

@newell
Copy link
Contributor Author

newell commented Jan 23, 2025

Has this been tested with real hardware?

Yes. If you would like to run some tests yourself you can flash the CPLD and then run test_ad9910_waveform.py to see some of the new functionality in action on your oscilloscope.

I’ve uploaded a video to Google Drive demonstrating the new capabilities (no guarantee for future readers that this link will remain active):

https://drive.google.com/file/d/18Y5HMqlYAk4xTw7fwv_1mqRqKBfi1A85/view?usp=sharing

The tests executed during the video were:

$ python3 -m unittest test_ad9910_waveform.py
Running test: test_att_single_tone
.sRunning test: test_drg_nodwell_amplitude
.sRunning test: test_drg_nodwell_frequency
.sRunning test: test_drg_nodwell_phase
.sRunning test: test_drg_normal_amplitude
.sRunning test: test_drg_normal_frequency
.sRunning test: test_drg_normal_phase
.sRunning test: test_drg_normal_with_hold_amplitude
.sRunning test: test_drg_normal_with_hold_frequency
.sRunning test: test_drg_normal_with_hold_phase
.s..Running test: test_osk_auto
.sRunning test: test_osk_manual
.sRunning test: test_single_tone
.sRunning test: test_toggle_profiles
.s
----------------------------------------------------------------------
Ran 30 tests in 189.314s

OK (skipped=14)

@Spaqin
Copy link
Collaborator

Spaqin commented Jan 23, 2025

Would this be a part of ARTIQ-8? How about the ICE version later, considering CPLD is EOL and ARTIQ-9 is a long way ahead still? There's some changes breaking backwards compatibility in the driver (set_profile now requires a channel parameter).

artiq/coredevice/urukul.py Outdated Show resolved Hide resolved
@sbourdeauducq
Copy link
Member

Would this be a part of ARTIQ-8?

No, we would just release 9 accordingly.

@newell newell marked this pull request as draft January 23, 2025 17:20
@newell newell marked this pull request as ready for review January 24, 2025 23:42
@newell
Copy link
Contributor Author

newell commented Jan 30, 2025

I believe I have addressed all the review comments so far. Is there anything else needed to get this merged?

artiq/coredevice/urukul.py Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants