-
Notifications
You must be signed in to change notification settings - Fork 4
Add Tektronix_DPO4xxx Setup Functionality #35
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
Add Tektronix_DPO4xxx Setup Functionality #35
Conversation
Add load_setup_file, and save_setup_file to load and save scope setups from a file Add store_setup, and recall_setup to store and save scope setups to on board memory
So, I'm going to look into the functionally of your changes a little bit later, but after a quick glance I noticed something that has me questioning something. What do you think about moving the min supported version of python to at least 3.10? This would simplify a lot of the type hinting. For example the union type you use in this update, Once I thought of it I also realized that our current min version, 3.9, also supports using generic collections for typing, so you don't need to do I think for any future updates to files we should clean this up, but for new updates/additions just use the built-ins |
complete. Defaults to 5s. | ||
""" | ||
prev_timeout = self.timeout | ||
self.timeout = timeout_seconds * 1000 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't notice that the timeout setter in VisaResource uses milliseconds. That seems inconsistent since the init method uses seconds to define it.
I'll update the baseclass timeout setter, this line, and the one in recall_setup in a subsequent commit.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok makes sense to me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, I see no issue merging this in.
The only issue I see with going to 3.10 is what Ted mentioned in his draft PR (#30) which is that Windows 7 can only use up to 3.8. However, currently we don't have 3.8 compatible type hints either. I would personally lean towards only supporting from 3.10 up and proceeding with the latest supported type hints. These are the most intuitive and straightforward to use. If we wanted to have a separate branch/repo for 3.8 support I'm sure there is some automated program that can quickly convert the newer type hints to the older ones (or remove them all together). |
Yea, I'm inclined agree. At this point Windows 7 is pretty dated anyhow. So, I'll look into upgrading the lib then, I'll see if 3.10 is the next logical jump or if it should be 3.11/12/13. I need to look back at the change log. Is your group running anything specific? Here we are mostly 3.11+ |
This update adds the following functionality to the Tektronix_DPO4xxx class: