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

Fix marker period start mnemonic #182

Closed
wants to merge 1 commit into from
Closed

Fix marker period start mnemonic #182

wants to merge 1 commit into from

Conversation

maij
Copy link

@maij maij commented Dec 9, 2022

While the QDAC II manual states "PST" as the abbreviated mnemonic, when using this command the QDAC raises an error with unknown mnemonic. Using the unabbreviated form works.

I haven't seen this issue for other commands, but have not checked thoroughly.

N.B. I am using firmware version 7-0.17.5

@astafan8
Copy link
Contributor

astafan8 commented Dec 9, 2022

@jpsecher perhaps this should be reviewed by you? :)

@jpsecher
Copy link
Contributor

Yes, unfortunately there is a discrepancy where some of the short form commands (in firmware up to version 0.17.5) do not follow the SCPI standard. This is fixed in more recent firmware, but this newer firmware has not been released yet. I will make sure that it will be released as soon as possible and post a link here.

In the meantime, you can use the long form of the keyword (like pstart) or use the short form with an extra vowel (like psta). Sorry for the inconvenience.

jpsecher added a commit to QDevil/Qcodes_contrib_drivers that referenced this pull request Dec 12, 2022
@jpsecher
Copy link
Contributor

The newest firmware is here:

https://qdevil-public.s3.eu-west-1.amazonaws.com/qdac2/qdac2-fw-update_11-1.14.exe
https://qdevil-public.s3.eu-west-1.amazonaws.com/qdac2/qdac2-fw-update-macos_11-1.14
https://qdevil-public.s3.eu-west-1.amazonaws.com/qdac2/qdac2-fw-update-linux_11-1.14

but, out of the box, it will not be usable with QCoDeS QDac2 drivers prior to version 1.1.2 (which is not released yet), see #180

@jenshnielsen
Copy link
Collaborator

@jpsecher Just to be sure the long form like in this pr will continue to be supported so its safe to merge this?

@jenshnielsen
Copy link
Collaborator

@maij It looks like some of the tests are written to assume the short from and would need updating

@jpsecher
Copy link
Contributor

The test have been updated together with the fix, so I don't know why the old tests are run. Can I retrigger the tests?

@jenshnielsen
Copy link
Collaborator

@jpsecher I guess the tests was updated on #180 ? If that one is ready for review it perhaps makes more sense to merge that and do a new release and close this one ?

@jpsecher
Copy link
Contributor

jpsecher commented Dec 13, 2022 via email

@jpsecher
Copy link
Contributor

jpsecher commented Dec 13, 2022 via email

@maij
Copy link
Author

maij commented Dec 13, 2022

Thanks for following up -- happy to close this once the other changes are merged.

@astafan8 astafan8 closed this in 43b37eb Dec 14, 2022
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