Skip to content

driver: usbsdmuxdriver: add GPIO get and set support #1891

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.

Sign up for GitHub

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

Open
hamztouiz wants to merge 1 commit into
base: master
Choose a base branch
from
+19 −0
Open

driver: usbsdmuxdriver: add GPIO get and set support #1891

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions labgrid/driver/usbsdmuxdriver.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,3 +45,22 @@ def get_mode(self):
cmd = self.mux.command_prefix + [self.tool, self.mux.control_path, "get"]
proc = subprocess.run(cmd, stdout=subprocess.PIPE, check=True)
return proc.stdout.strip().decode()

@Driver.check_active
@step(title="sdmux_gpio_get", args=["gpio"])
def get_gpio(self, gpio):

@Emantor Emantor Jun 9, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not using the DigitalOutputProtocol here means that we can't make use of the IOs in the client and discoverability is difficult (since the function is named differently)

But the DigitalOutputProtocol requires that the index of the GPIO is set beforehand.

@Bastian-Krause @jluebbe do you have a better idea here?

@Bastian-Krause Bastian-Krause Jun 30, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Emantor @jluebbe @SmithChart Maybe this should be a dedicated resource/driver then? Is it safe to run the usbsdmux tool in parallel?

@SmithChart SmithChart Jun 30, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The tool is not safe for parallel use:

We do some read-modify-write over the I2C-bus on the device (e.g. here ). With two instances in parallel one could loose and may get it's changes overwritten by the other instance.
The tool would exit without error - but the state of the USB-SD-Mux would not have changed as expected.

But isn't labgrid strictly serial in it's execution?

@hamztouiz hamztouiz Jun 30, 2026 •
edited
Loading

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After thinking about it more, I think having separate resources/drivers per IO could actually introduce a problem if two clients each acquire a different IO simultaneously, since they’d still be hitting the same USB device underneath. And honestly I’m not sure there’s a real use case for it beyond being able to implement the protocol, which we could achieve differently anyway.

Would it be possible at the driver level to have two internal indexes and implement DigitalOutputProtocol for each of them within the same driver? That way we keep a single point of access to the USB device and still get the discoverability benefit.

@Bastian-Krause Bastian-Krause Jun 30, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After thinking about it more, I think having separate resources/drivers per IO could actually introduce a problem if two clients each acquire a different IO simultaneously, since they’d still be hitting the same USB device underneath.

That's exactly what I meant when I asked about the parallel access.

And honestly I’m not sure there’s a real use case for it beyond being able to implement the protocol, which we could achieve differently anyway.

The common interface is the benefit. Every GPIO can be used the same way. And labgrid client io won't work otherwise.

Would it be possible at the driver level to have two internal indexes and implement DigitalOutputProtocol for each of them within the same driver? That way we keep a single point of access to the USB device and still get the discoverability benefit.

I don't think that's possible. If you could live with using only one the GPIOs, we could either hard-code it in the driver or configure it in the USBSDMuxDevice/NetworkUSBSDMuxDevice resource. Either that or add a new resource + driver and some locking or exclusive access + wait/timeout in the usbsdmux tool. But both options are not really ideal.

@hamztouiz hamztouiz Jul 2, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the clarification.

You mentioned that adding locking or exclusive access in the usbsdmux tool is not really ideal, could you elaborate on why?

@SmithChart SmithChart Jul 2, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hamztouiz There was no need for locking in the tool until now. Usage of the tool was always strictly linear, not parallel.

I do not see why we should not add a lock to the tool if it turns out to be useful.

But I do not understand why access can happen in parallel. Labgrid will lock the resource, so only a single user will access the resource. And isn't labgrid strictly linear in executing by design? How could a parallel access ever happen?

@Bastian-Krause Bastian-Krause Jul 2, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@SmithChart If we would turn the usbsdmux GPIOs into dedicated resources and add a driver for them implementing the DigitalOutputProtocol, these new GPIO and USBSDMuxDevice/NetworkUSBSDMuxDevice resources could be accessed during the same time from different places.

@hamztouiz With "not ideal" I tried to hint at the rather high effort needed for such a simple operation as toggling a GPIO (add locking to usbsdmux, add a new resource, add a new driver).

@hamztouiz hamztouiz Jul 3, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll let you decide on the best approach here, I don't have enough context on the project conventions to make the call.

I'm also fine with not implementing the protocol and skipping the dedicated resource and driver for now.

if gpio not in [0, 1]:
raise ExecutionError(f"GPIO '{gpio}' not supported by USBSDMuxDriver")
cmd = self.mux.command_prefix + [self.tool, self.mux.control_path, "gpio", str(gpio), "get"]
proc = subprocess.run(cmd, stdout=subprocess.PIPE, check=True)
return proc.stdout.strip().decode()

@Driver.check_active
@step(title="sdmux_gpio_set", args=["gpio", "value"])
def set_gpio(self, gpio, value):
if gpio not in [0, 1]:
raise ExecutionError(f"GPIO '{gpio}' not supported by USBSDMuxDriver")
if value.lower() not in ["high", "1", "low", "0"]:
raise ExecutionError(f"GPIO value '{value}' not supported by USBSDMuxDriver")
cmd = self.mux.command_prefix + [self.tool, self.mux.control_path, "gpio", str(gpio), value.lower()]
processwrapper.check_output(cmd)