feat(awg): add modulation to AWG

[LizzyC-115]

Summary

Adding modulation feature to AWG drivers.

Type of change

• Bug fix (fix)
• New feature (feat)
• Breaking change (feat! / fix!)
• Refactor (refactor)
• Documentation (docs)
• Chore / tooling (chore)

Verification

Connection to Tektronix Scope:

Hardware Tests:

Software Tests:

Tests

• Unit tests added or updated
• Existing tests cover this change
• No tests — explain why:

Checklist

• PR title follows Conventional Commits (e.g. feat(driver): add support for Keysight E36300)
• I have read CONTRIBUTING.md
• Documentation updated if user-facing behavior changed
• Code follows the style/conventions of the surrounding code

Notes for Reviewers

For this implementation, we are decoupling setting modulation features (mod_type, shape, and magnitude) from enabling modulation on a specific channel. This is to avoid having the user reset the channel's modulation in procedural tasks. Therefore, instead of solely modulate(), we use enable_modulation(), set_modulation(), and disable_modulation().

This pattern-matches well with Keysight 33500B Waveform Generator as well. For the Keysight, you can enable/disable modulation with the command [mod_type]:STAT:ON or [mod_type]:STAT:OFF.

[mintlify]

Preview deployment for your docs. Learn more about Mintlify Previews.

Project	Status	Preview	Updated (UTC)
instro	🟢 Ready	View Preview	Jul 31, 2026, 9:00 PM

💡 Tip: Enable Workflows to automatically generate PRs for you.

[mintlify]

Preview deployment for your docs. Learn more about Mintlify Previews.

Project	Status	Preview	Updated (UTC)
instro	🟡 Building	–	Jul 31, 2026, 9:00 PM

💡 Tip: Enable Workflows to automatically generate PRs for you.

[greptile-apps]

PR author is not in the allowed authors list.

[claude]

Claude finished @LizzyC-115's task in 2m 40s —— View job

---

Code review

• Gather context (PR diff, changed files, AGENTS.md conventions)
• Review driver conventions (composition, registration, tests)
• Review base-class/API surface changes (awg.py, types.py)
• Check for premature abstraction
• Check docs sync
• Post findings

Overview: Clean, convention-following addition. modulate() is added as an optional method on AWGDriverBase (raises NotImplementedError by default), RigolDG1022Z composes VisaDriver as before, and _MOD_INTERNAL_FUNCTIONS mirrors the existing type-keyed dict pattern (_PUBLISHED_NAMES, _CREST_FACTORS) rather than introducing a new abstraction. Docs (awg.mdx) and HAL-level tests were updated in the same PR. One ergonomic note (non-blocking): magnitude: float is overloaded to mean depth-%, Hz-deviation, degree-deviation, amplitude-V, or Hz-rate depending on mod_type — worth a short doc/docstring clarification of units per modulation type.

Findings:

1. tests/unstable/awg/rigol/test_rigol_dg1022z_software.py:379-400 — mocked-transport coverage only exercises AM and FSK. ASK (a distinct SCPI branch in rigol_dg1022z.py:207-211) and the _mod_carrier unsupported-shape ValueError are only covered by @pytest.mark.hardware tests, which don't run in CI. Per AGENTS.md, new driver logic needs mocked wire-level assertions — please add software tests for the ASK branch and the carrier-rejection path.

2. packages/instro-unstable/instro/unstable/awg/drivers/rigol_dg1022z.py:193-194 — RigolDG1022Z.modulate re-validates isinstance(mod_type, ModulationType), duplicating the check InstroAWG.modulate already does (awg.py:363-364) before calling the driver. Existing driver methods (e.g. set_amplitude) don't re-validate enum args — HAL is the single validation layer. Minor, but worth aligning unless the standalone-driver-usage case (as used directly in the hardware tests) is the intentional reason to keep it.

[nhschwab]

My only major concern is that the modulation command as currently implemented is not stateful. In other words, a user can send a command to modulate a waveform, but they can't turn it off or turn if back on again after the fact. I think the correct shape here is to have an internally tracked struct for each channel that records the modulation type and its characteristics and to have methods for updating the modulation signal and for turning on/off modulation for a given channel

[LizzyC-115]

Added ModulationState as a class to the Rigol driver to allow enable and disable functions that the user can call. Each channel has a dedicated instance. The class keeps track of type, shape, magnitude, and whether or not the channel has been enabled. We clear these states to None when modulation is disabled to avoid leaks.

[LizzyC-115]

Decoupled modulate() into enable_modulation() and set_modulation()

[LizzyC-115]

@claude review

[claude]

Claude finished @LizzyC-115's task in 2m 43s —— View job

---

Code review

• Gather context (diff vs origin/main, prior review threads, AGENTS.md conventions)
• Review driver conventions, base-class scope, docs sync
• Post findings

Overview: Good iteration since the last pass — the previously-flagged mocked-transport gap (ASK, carrier-rejection path) is now fully covered in test_rigol_dg1022z_software.py, and the enable/set/disable split with per-channel ModulationState cleanly resolves @nhschwab's statefulness concern without adding a shared base-class abstraction (state lives on the driver, per AGENTS.md). _validate_carrier/_validate_modulator naming and the awg.mdx magnitude/shape table address the carrier-vs-modulator and magnitude-semantics review comments well.

Findings:

1. packages/instro-unstable/instro/unstable/awg/awg.py:383-387 — set_modulation's docstring is two paragraphs (summary + a magnitude by mod_type line). AGENTS.md requires one short line max (INSTRO-248 cleanup); the per-mod_type unit table already lives in awg.mdx, so this could just link there or drop to one line.

2. packages/instro-unstable/instro/unstable/awg/drivers/rigol_dg1022z.py:219-225 — set_modulation's state.enabled check and _validate_carrier(mod_type, self.get_waveform(channel)) run before self._visa.lock() is acquired, while enable_modulation/disable_modulation do their state checks inside the lock. Since self._visa.lock() is an RLock and get_waveform already acquires it internally, this looks like it can just move inside the with self._visa.lock(): block to close the gap and match the atomic-sequence pattern AGENTS.md calls out (BK9140-style). Non-blocking for single-threaded HAL use (the whole call is already serialized by InstroAWG._resource_lock), but matters for direct driver use with a shared VISA resource.

3. tests/unstable/awg/test_awg.py — set_modulation/disable_modulation each get a *_delegates_to_driver + *_returns_command_with_correct_descriptor pair, but enable_modulation only gets the generic NotImplementedError/channel-validation coverage — no dedicated HAL-level delegate/descriptor test. Minor asymmetry worth closing for consistency with its two siblings.

👍'd the still-relevant duplicate-isinstance(mod_type, ...) validation point from the earlier review (driver re-validates what InstroAWG.set_modulation already checks) rather than repeating it — the ASK/carrier-rejection test gap from that same comment is now resolved.