Switch keysight polarization navigator by franz-sweepMe · Pull Request #226 · SweepMe/instrument-drivers

franz-sweepMe · 2026-01-27T14:41:28Z

add driver for polarization controll

gemini-code-assist · 2026-01-27T14:41:55Z

Summary of Changes

Hello @franz-sweepMe, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request introduces a comprehensive driver for the Keysight N778xB Polarization Synthesizer, allowing users to control the instrument's polarization settings through the SweepMe! application. The driver supports two primary modes of operation: setting individual waveplate orientations and retardations, and directly defining the State of Polarization (SOP) using Stokes parameters. Communication with the hardware is facilitated by integrating with the Keysight Polarization Navigator DLL, ensuring robust and precise control over the optical polarization.

Highlights

New Instrument Driver: Introduction of a new driver for the Keysight N778xB Polarization Synthesizer, integrating it into the SweepMe! framework.
Dual Control Modes: Implementation of two distinct control modes: "Orientation and Retardation" for direct waveplate manipulation and "SOP" (State of Polarization) for setting Stokes parameters.
DLL Integration: Utilizes the Keysight Polarization Navigator DLL for low-level communication with the instrument, enabling advanced control capabilities.
Error Handling: Includes basic error handling for DLL communication, providing feedback on command execution status.

🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	`/gemini review`	Performs a code review for the current pull request in its current state.
Pull Request Summary	`/gemini summary`	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	`/gemini help`	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

gemini-code-assist

Code Review

This pull request adds two new drivers for Keysight polarization control hardware: one for the N778xB using GPIB, and another for the Polarization Navigator software using a DLL. The review focuses on improving code quality, correctness, and maintainability.

Key feedback points include:

Critical issues: The Polarization Navigator driver contains a hardcoded DLL path and a duplicated, buggy call to the DLL function. These must be fixed.
High-severity issues: Error handling is inconsistent, with some errors being printed instead of raised as exceptions. There are also logical inconsistencies in function implementations and resource management issues in a test script.
Medium-severity issues: There are several opportunities for improvement, such as replacing print with proper logging, avoiding broad exception catches, removing hardcoded constants like pi, and cleaning up unused variables and imports.

gemini-code-assist · 2026-01-27T14:44:53Z

+
+from pysweepme.EmptyDeviceClass import EmptyDevice
+
+DLL_PATH = r"C:\Program Files\Keysight\Polarization Navigator\bin\PolNavClient.dll"


The DLL path is hardcoded. This makes the driver not portable and it will fail on any system where the software is installed in a different location. This path should be made configurable, for example by reading it from an environment variable, a configuration file, or passing it as a parameter during initialization.

gemini-code-assist · 2026-01-27T14:44:53Z

+        send_command(
+            target.encode("ascii"),
+            command.encode("ascii"),
+            response_buffer,
+            buffer_size,
+            response_len,
+        )
+
+        # Call the DLL function
+        result = self.client.PolNavC_SendCommand(
+            ctypes.c_char_p(target.encode("ascii")),
+            ctypes.c_char_p(command.encode("ascii")),
+            response_buffer,
+            ctypes.c_int(buffer_size),
+            ctypes.byref(response_len),  # why is this empty?
+        )


The PolNavC_SendCommand function is called twice in a row. The first call uses incorrect arguments (passing response_len directly instead of by reference) and its return value is ignored. The second call is also overly verbose. This appears to be a copy-paste error and can lead to unpredictable behavior. The block should be simplified to a single, correct call to the function.

Suggested change

send_command(

target.encode("ascii"),

command.encode("ascii"),

response_buffer,

buffer_size,

response_len,

)

# Call the DLL function

result = self.client.PolNavC_SendCommand(

ctypes.c_char_p(target.encode("ascii")),

ctypes.c_char_p(command.encode("ascii")),

response_buffer,

ctypes.c_int(buffer_size),

ctypes.byref(response_len), # why is this empty?

)

result = send_command(

target.encode("ascii"),

command.encode("ascii"),

response_buffer,

buffer_size,

ctypes.byref(response_len),

)

gemini-code-assist · 2026-01-27T14:44:53Z

+        """'apply' is used to set the new set value that is always available as 'self.value'."""
+        if self.mode == "SOP":
+            try:
+                x, y, z = map(float, self.value.split(";"))


The code splits the input self.value using a semicolon (;), but the error message on line 129 suggests a comma-separated format ('x,y,z'). This inconsistency will be confusing for users. Please ensure the parsing logic and the error message are aligned. Based on the error message, it seems the value should be split by a comma.

Suggested change

x, y, z = map(float, self.value.split(";"))

x, y, z = map(float, self.value.split(","))

gemini-code-assist · 2026-01-27T14:44:53Z

+        if result != 0:
+            msg = f"PolNav_SendCommand failed with error code: {result}"
+            print(msg)
+            # raise RuntimeError(msg)


Errors from the DLL are printed to the console but the RuntimeError is commented out. This means failures will be silent from the perspective of the calling code, which can lead to unexpected behavior and make debugging difficult. Errors should be raised as exceptions.

Suggested change

# raise RuntimeError(msg)

raise RuntimeError(msg)

gemini-code-assist · 2026-01-27T14:44:53Z

+    def get_sop(self) -> str:
+        """Get the current SOP from the Polarization Navigator.
+
+        Sending the 'Get CurrentSOPN' command results in an access error, which is why this function is currently not implemented.
+        """
+        self.send_command("Get CurrentSOPN")
+        return self.read()


The docstring states that this function is not implemented because the underlying command causes an error. However, the function body proceeds to call the failing command. If a function is not meant to be used, it should raise a NotImplementedError to clearly communicate its status to the developer.

Suggested change

def get_sop(self) -> str:

"""Get the current SOP from the Polarization Navigator.

Sending the 'Get CurrentSOPN' command results in an access error, which is why this function is currently not implemented.

"""

self.send_command("Get CurrentSOPN")

return self.read()

def get_sop(self) -> str:

"""Get the current SOP from the Polarization Navigator.

Sending the 'Get CurrentSOPN' command results in an access error, which is why this function is currently not implemented.

"""

raise NotImplementedError("Reading SOP is not supported due to a command error.")

gemini-code-assist · 2026-01-27T14:44:53Z

+        """Set the orientation and retardation for the 5 waveplates."""
+        try:
+            orientations_and_retardations = [float(x.strip()) for x in orientations_and_retardations.split(",")]
+        except Exception as e:


Catching a broad Exception can hide bugs and make debugging more difficult. It's better to catch more specific exceptions. In this case, float(x.strip()) is most likely to raise a ValueError, so it would be better to catch that specifically.

Suggested change

except Exception as e:

except ValueError as e:

gemini-code-assist · 2026-01-27T14:44:53Z

+        for n in range(5):
+            orientation = orientations_and_retardations[n * 2]
+            retardation = orientations_and_retardations[n * 2 + 1]
+            if not (0 <= orientation <= 2 * 3.14159):


Using a hardcoded value for π is less precise and readable than using the constant from the math module. Please import the math module at the top of the file and use math.pi.

import math # ... later in the code if not (0 <= orientation <= 2 * math.pi):

Suggested change

if not (0 <= orientation <= 2 * 3.14159):

if not (0 <= orientation <= 2 * 3.141592653589793):

gemini-code-assist · 2026-01-27T14:44:54Z

+
+            configuration += f"{orientation},{retardation},"
+
+        error = self.port.query(f"POLCON:WAVEPL {configuration}")


The return value of self.port.query(...) is assigned to the error variable, but it's never used. If the device returns an error status, it is being ignored, which could lead to silent failures. This value should be checked to ensure the command was successful.

gemini-code-assist · 2026-01-27T14:44:54Z

+        """Initialize the driver class and the instrument parameters."""
+        super().__init__()
+
+        self.shortname = "N778xB"  # short name will be shown in the sequencer


The shortname is set to "N778xB", but this driver is for the "Keysight Polarization Navigator" software, as indicated by the file path and class description. This could be confusing. Consider using a more descriptive shortname like "PolNav" to better reflect the instrument being controlled.

Suggested change

self.shortname = "N778xB" # short name will be shown in the sequencer

self.shortname = "PolNav" # short name will be shown in the sequencer

gemini-code-assist · 2026-01-27T14:44:54Z

@@ -0,0 +1,22 @@
+# -*- coding: utf-8 -*-
+
+import time


The time module is imported but never used in this file. Unused imports should be removed to keep the code clean.

franz-sweepMe added 11 commits January 27, 2026 15:30

Keysight N778xB -> initial driver version

c501091

Polarization Navigator -> dll testing CAN BE OMITTED

093266a

Pol Navigator -> Working dll integration in sandbox

9927fcd

N778xB -> dll testing

38022d3

PolNav -> add driver with dll

16a227c

PolNav -> Fix error in self.value handling

932ffba

PolNav -> remove SOP readout

da60735

N778xB -> add SOP control via GPIB

0377ec2

PolNav -> Add tests

c5ccf3e

N778x -> update return variables

e35fc83

PolNav -> add COM example code CAN BE OMITTED

457013f

gemini-code-assist Bot reviewed Jan 27, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Switch keysight polarization navigator#226

Switch keysight polarization navigator#226
franz-sweepMe wants to merge 11 commits into
mainfrom
Switch-Keysight_PolarizationNavigator

franz-sweepMe commented Jan 27, 2026

Uh oh!

gemini-code-assist Bot commented Jan 27, 2026

Uh oh!

gemini-code-assist Bot left a comment

Uh oh!

gemini-code-assist Bot Jan 27, 2026

Uh oh!

gemini-code-assist Bot Jan 27, 2026

Uh oh!

gemini-code-assist Bot Jan 27, 2026

Uh oh!

gemini-code-assist Bot Jan 27, 2026

Uh oh!

gemini-code-assist Bot Jan 27, 2026

Uh oh!

gemini-code-assist Bot Jan 27, 2026

Uh oh!

gemini-code-assist Bot Jan 27, 2026

Uh oh!

gemini-code-assist Bot Jan 27, 2026

Uh oh!

gemini-code-assist Bot Jan 27, 2026

Uh oh!

gemini-code-assist Bot Jan 27, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant


		from pysweepme.EmptyDeviceClass import EmptyDevice

		DLL_PATH = r"C:\Program Files\Keysight\Polarization Navigator\bin\PolNavClient.dll"

	x, y, z = map(float, self.value.split(";"))
	x, y, z = map(float, self.value.split(","))

	if not (0 <= orientation <= 2 * 3.14159):
	if not (0 <= orientation <= 2 * 3.141592653589793):


		configuration += f"{orientation},{retardation},"

		error = self.port.query(f"POLCON:WAVEPL {configuration}")

	self.shortname = "N778xB" # short name will be shown in the sequencer
	self.shortname = "PolNav" # short name will be shown in the sequencer

Uh oh!

Conversation

franz-sweepMe commented Jan 27, 2026

Uh oh!

gemini-code-assist Bot commented Jan 27, 2026

Summary of Changes

Highlights

Footnotes

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist Bot Jan 27, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Jan 27, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Jan 27, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Jan 27, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Jan 27, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Jan 27, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Jan 27, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Jan 27, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Jan 27, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Jan 27, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant