feat(awg): add keysight 33500b awg driver - #359
Conversation
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Workflows to automatically generate PRs for you. |
e8b7795 to
0dad42b
Compare
|
PR author is not in the allowed authors list. |
|
Claude finished @LizzyC-115's task in 2m 46s —— View job Review completeTasks
Overview
One inline nit posted:
No other defects, convention divergences, or missing coverage found. Nice work matching the existing driver pattern closely. |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
LizzyC-115
changed the title
| self._visa.write(f"{prefix}:PHAS {magnitude}") | ||
| else: | ||
| raise AssertionError(f"unhandled ModulationType {mod_type}") | ||
| self._visa.write(f"{prefix}:STAT ON") |
There was a problem hiding this comment.
Keysight 33500B require it's SCPI commands to use the modulation type to enable modulation. However, there is no "query for modulation type" SCPI command. The most we can do is check if a modulation type if ON. So, to avoid having software states, choosing to enable modulation in the set_modulation function where the modulation type is unambiguous and force modulation_enable() to disable only.
| """Disables modulation""" | ||
| _check_channel(channel) | ||
| if enable: | ||
| raise ValueError( | ||
| "the Keysight 33500B enables modulation as part of set_modulation;" | ||
| " modulation_enable only supports disabling (enable=False)" | ||
| ) | ||
| with self._visa.lock(): | ||
| for prefix in _MOD_SCPI_PREFIX.values(): | ||
| self._visa.write(f"{prefix}:STAT OFF") | ||
| self.check_errors() |
There was a problem hiding this comment.
Echoing from above function: Keysight 33500B require it's SCPI commands to use the modulation type to enable modulation. However, there is no "query for modulation type" SCPI command. The most we can do is check if a modulation type if ON. So, to avoid having software states, choosing to enable modulation in the set_modulation function where the modulation type is unambiguous and force modulation_enable() to disable only.
|
Claude finished @LizzyC-115's task in 3m 20s —— View job Reviewing PR #359
OverviewRe-reviewed after the One design note worth surfacing (not a defect): Checked: driver registration ( |
| @abc.abstractmethod | ||
| def check_errors(self) -> None: | ||
| """Check the instrument error queue.""" | ||
|
|
There was a problem hiding this comment.
This file only contains removals of check errors. The reasoning for this change was detailed in this issue here: #369
1 participant
Summary
Adding support for a new AWG device. This PR builds on the AWGBaseDriver and InstroAWG framework already in main and implements those functions for the Keysight 33500B series.
Type of change
fix)feat)feat!/fix!)refactor)docs)chore)Verification
Tests
Checklist
feat(driver): add support for Keysight E36300)