Qblox qmi driver by heevasti · Pull Request #215 · QuTech-Delft/QMI

heevasti · 2026-06-02T11:26:10Z

Add a QMI driver for Qblox Cluster series device.

The driver contains only base functions to control the cluster, and trigger channels, and receive system info and module | function reference objects.

…rted editing unit-tests.

…ests.

bvanommen

Tests look good, maybe bit of overlap between the two cluster modules.

I don't have experience with the device but looks OK. I would like to know why there are two classes. Is it to control either via SCPI commands or "native" control?

bvanommen · 2026-09-18T12:04:31Z

+EXT_TRIGGERS_IN_CLUSTER = 15
+# Module configuration constants:
+# AO_IN_MODULE:              Analog output channels in modules with value range -1V...+1V.
+# AI_IN_MODULE:              Analog output channels in modules with value range -1V...+1V.


input channels presumeably

Yes, will correct this, thanks!

bvanommen · 2026-09-18T13:32:51Z

+        # Add type handles as callables
+        for type_handle, value in self.cluster._type_handle.__dict__.items():
+            if type_handle.startswith("_is"):
+                # As 'value' gets overridden in memory for the lambda function, do like this:


overwritten

Will correct this, thanks!

bvanommen · 2026-09-18T13:34:39Z

+        idn = self.cluster._get_idn()
+        _, model, _, _ = idn.split(",")
+        self._modules = {"0": _QbloxModule(model, self.cluster_funcs)}
+        for k, v in self.cluster._mod_handles.items():


code would be a bit more readable if these keys and values have a bit more descriptive names

I guess the v's are modules?

Yes, I will change 'k' to refer to the slot numbers and 'v' to module handles.

bvanommen · 2026-09-18T13:40:43Z

+            if str(module) == module_type:
+                # Just double check module type before returning
+                if (
+                    module_type == "QCM"


if these two are on the same line it looks much better

I'm not sure if Ruff formatter will agree on that. I'll see what it thinks.

Sorry, Ruff left it that way.

bvanommen · 2026-09-18T13:42:25Z

+        self.cluster._slot_reset()
+
+    @rpc_method
+    def get_module(self, module_type: str, slot_no: int | None = None) -> NativeCluster | _QbloxModule:


I guess it might be fair enough, but if you have multiple of the same module and you specify the module name, it's not clear which one you're gonna get. Maybe ideally you warn the user (or refuse) when you know there are duplicate modules and they didn't specify the slot.

The module you will get is the first match, i.e. with smallest slot number. I could add a message for the logger here to say the the first matching module will be 'get' when no slot number was provided. To 'know' if there are >1 modules of given type, I would have to change the loop to go to the end or do some pre-loop with the module names only. I think the call description (dosctring) in the base class and a logger info should be informative enough, without making extra checks here.

Sure I guess that's fine!

bvanommen · 2026-09-18T13:47:02Z

+        for sequencer in range(SEQUENCERS_IN_MODULE[module_type]):
+            try:
+                sequencer_config = module_func_refs["_get_sequencer_config"](sequencer)
+                if channel_type in [0, 1] and AI_IN_MODULE[module_type] > 0:


magical constants, ideally we replace

Do you mean the channel_type? Those are explained in the base class docstring. But we could consider making enumerated integers class for them, which would make them more clear. Like

class ChannelTypes(Enum.Enum): ALL_CHANNELS = 0 # all channels (default) AI_CHANNELS = 1 # analog input channels (adc) AO_CHANNELS = 2 # analog output channels (dac) MRK_CHANNELS = 3 # marker channels IO_CHANNELS = 4 # IO channels (QTM only)

and replace

if channel_type in [ChannelTypes.ALL_CHANNELS, ChannelTypes.AI] ...

Or perhaps with even shorter names as we use these only internally, really:
_ChannelTypes.ALL, _ChannelTypes.AI etc.

Yeah sorry my comment was a bit short. Indeed It was not clear to me what channel type 0 and 1 would refer to. I think the second solution is good!

bvanommen · 2026-09-18T13:47:08Z

+                        for channel in sequencer_channel_map[1]:
+                            channels[f"adc{channel}_acq_Q{sequencer}"] = sequencer_config["awg"]
+
+                if channel_type in [0, 2] and AO_IN_MODULE[module_type] > 0:


idem with previous

bvanommen · 2026-09-18T13:47:18Z

+                    for channel in sequencer_channel_map[1]:
+                        channels[f"dac{channel}_Q{sequencer}"] = sequencer_config["awg"]  # TODO: Is this always valid?
+
+                if channel_type in [0, 3] and DIGITAL_MARKERS_IN_MODULE[module_type] > 0:


idem with previous.

bvanommen · 2026-09-18T13:49:00Z

+    invert: Any
+
+
+class Qblox_QcodesCluster(Qblox_ClusterBase):


what is the difference between this class and Qblox_NativeCluster?

The difference is that the 'NativeCluster' communicates directly with the cluster SCPI interface, while the 'QcodesCluster' communicates with the QCoDeS interface, which then communicates via SCPI interface (and a couple of special features). Both have their pros and cons, which I haven't managed to decide so far which one is overall better. The qblox_instruments API has changed a lot over the development of various versions, and 'QcodesCluster' option was a bit more stable towards the changes, whereas 'NativeCluster' usually needed more smaller changes. On the other hand, at times the 'QcodesCluster' option's dependency 'Cluster' class radically changed with much more complex changes in the 'QcodesCluster' as result.

The other thing is that communicating directly with the SCPI interface we actually do not need the QCoDeS dependency. This reduces also considerably dependency conflicts with other packages as we can override or ignore the QCoDeS version be something else than in the qblox_instruments requirements.

This is of course more maintenance work which is not desirable in the long term. But so far I couldn't weight one out against the other such that a decision could be made into which option we will focus on.

Right, thanks for the explanation, that is very clear. Then let's keep both for now.
Maybe you could add a sentence or two in comments somewhere that documents this difference? It would be quite informative

Henri Ervasti added 7 commits May 28, 2026 17:58

[QMI-XXX qblox-qmi-driver] Added Qblox QMI driver in instruments. Sta…

c410578

…rted editing unit-tests.

[qblox-qmi-driver] Slowly progressing on unittests.

b9fb536

[qblox-qmi-driver] Fixed more call names and further fixing of unit-t…

32833aa

…ests.

[qblox-qmi-driver] Improved coverage to be over 90% with CODEX.

d5e36a1

[qblox-qmi-driver] More unittests and some more documentation.

8d055e4

[qblox-qmi-driver] Mypy fixing.

8d21a66

[qblox-qmi-driver] trying to fix unittest issue.

d318de6

heevasti requested a review from pieterjbotma June 2, 2026 11:45

bvanommen requested changes Sep 18, 2026

View reviewed changes

Henri Ervasti added 2 commits September 28, 2026 13:07

Merge remote-tracking branch 'origin/main' into qblox-qmi-driver

1cfb324

[qblox-qmi-driver] Edits after review comments.

ee6476d

heevasti requested a review from bvanommen September 28, 2026 12:18

Conversation

heevasti commented Jun 2, 2026

Uh oh!

bvanommen left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

bvanommen left a comment •

edited

Loading