Skip to content

Conversation

@redzynix
Copy link
Contributor

Description
By default ykushcmd command supports option to [on, off, cycle] whole ykush yepkit.

ykushcmd -u a # 'a' stands for all ports
ykushcmd -d a # 'a' stands for all ports

In current build only int is supported for port number, changing it to string allows to control power on all ports with one command instead of 3 if there are 3 ports.
Tested locally.

Checklist

  • [ - ] Documentation for the feature
  • [ x ] Tests for the feature
  • [ - ] The arguments and description in doc/configuration.rst have been updated
  • [ - ] Add a section on how to use the feature to doc/usage.rst
  • [ - ] Add a section on how to use the feature to doc/development.rst
  • [ x ] PR has been tested
  • [ - ] Man pages have been regenerated

@redzynix redzynix force-pushed the redzynix-add-string-support-to-ykushpowerport branch from de62b3a to af7f668 Compare September 25, 2023 09:24
@Bastian-Krause
Copy link
Member

I don't see how this change would be useful in the context of labgrid. As far as I understand, this would allow exporting a YEPKIT Switchable USB Hub as a single resource. I cannot imagine a case where that would be useful. Could you please elaborate why it is?

@Bastian-Krause Bastian-Krause added the needs author info Requires more information from the PR/Issue author label Oct 19, 2023
@Bastian-Krause
Copy link
Member

Are you still interested in this PR?

@cgturner1
Copy link

Are you still interested in this PR?

We are still interested in this functionality mainly our scenario focus on controlling the power to individual all ports on a ykush machine to power on/off a single device connected to a ykush multiple times over having to power on/off all devices at once. This change would preserve the individual functionality as it works now but allowing for a string would enable the 'a' command to power cycle all ports on the device.

@Bastian-Krause
Copy link
Member

@cgturner1 Can you explain your board setup a bit more? I can't really see how this would be useful.

@redzynix
Copy link
Contributor Author

redzynix commented May 27, 2024

@cgturner1 Can you explain your board setup a bit more? I can't really see how this would be useful.

@Bastian-Krause sorry for keeping you waiting that long, so basically changing "int" to "str" allows ykushcmd to affect the whole board instead of just one power port when needed.
With the current implementation, it's impossible to power cycle the whole yepkit board with one place(this can be achieved by defining each of the ports separately and running power cycle 3 times), not to mention that it's supported by ykushcmd by default.

image

https://www.learn.yepkit.com/reference/ykushcmd-reference-ykush/1/2

@redzynix redzynix removed their assignment Jun 4, 2024
@KamilxPaszkiet
Copy link

KamilxPaszkiet commented Jul 18, 2024

@cgturner1 Can you explain your board setup a bit more? I can't really see how this would be useful.

@Bastian-Krause @jluebbe @Emantor
This functionality would be very useful. Right now I have a case that would save my setup. I need to turn off both USB ports simultaneously, because the devices "power" each other. For now, I have to turn them off individually, which takes more time and unnecessary additional commands.

I don't really understand why this change wasn't implemented from the beginning. It's one of the basic functionalities of ykush.
For me it's a high priority pr

@Bastian-Krause
Copy link
Member

Bastian-Krause commented Jul 18, 2024

@cgturner1 Can you explain your board setup a bit more? I can't really see how this would be useful.

@Bastian-Krause @jluebbe @Emantor This functionality would be very useful. Right now I have a case that would save my setup. I need to turn off both USB ports simultaneously, because the devices "power" each other. For now, I have to turn them off individually, which takes more time and unnecessary additional commands.

I don't really understand why this change wasn't implemented from the beginning. It's one of the basic functionalities of ykush. For me it's a high priority pr

I think it was not implemented because it does not really match the concept of a YKUSHPowerPort when you actually work on multiple ports.

What happens when querying the power state for "a"?

@jluebbe What do you think about this?

@KamilxPaszkiet
Copy link

KamilxPaszkiet commented Jul 18, 2024

@cgturner1 Can you explain your board setup a bit more? I can't really see how this would be useful.

@Bastian-Krause @jluebbe @Emantor This functionality would be very useful. Right now I have a case that would save my setup. I need to turn off both USB ports simultaneously, because the devices "power" each other. For now, I have to turn them off individually, which takes more time and unnecessary additional commands.
I don't really understand why this change wasn't implemented from the beginning. It's one of the basic functionalities of ykush. For me it's a high priority pr

I think it was not implemented because it does not really match the concept of a YKUSHPowerPort when you actually work on multiple ports.

What happens when querying the power state for "a"?

@jluebbe What do you think about this?

error, because "a" it's not a int.

this is small change for You, but big change for us.

@Bastian-Krause
Copy link
Member

@cgturner1 Can you explain your board setup a bit more? I can't really see how this would be useful.

@Bastian-Krause @jluebbe @Emantor This functionality would be very useful. Right now I have a case that would save my setup. I need to turn off both USB ports simultaneously, because the devices "power" each other. For now, I have to turn them off individually, which takes more time and unnecessary additional commands.
I don't really understand why this change wasn't implemented from the beginning. It's one of the basic functionalities of ykush. For me it's a high priority pr

I think it was not implemented because it does not really match the concept of a YKUSHPowerPort when you actually work on multiple ports.
What happens when querying the power state for "a"?
@jluebbe What do you think about this?

error, because "a" it's not a int.

No, I meant with this PR applied (e.g. labgrid-client power get).

@redzynix
Copy link
Contributor Author

@cgturner1 Can you explain your board setup a bit more? I can't really see how this would be useful.

@Bastian-Krause @jluebbe @Emantor This functionality would be very useful. Right now I have a case that would save my setup. I need to turn off both USB ports simultaneously, because the devices "power" each other. For now, I have to turn them off individually, which takes more time and unnecessary additional commands.
I don't really understand why this change wasn't implemented from the beginning. It's one of the basic functionalities of ykush. For me it's a high priority pr

I think it was not implemented because it does not really match the concept of a YKUSHPowerPort when you actually work on multiple ports.
What happens when querying the power state for "a"?
@jluebbe What do you think about this?

error, because "a" it's not a int.

No, I meant with this PR applied (e.g. labgrid-client power get).

By doing a static analysis of this function it will fail due to different port numbers. 'a' != 15
https://github.com/labgrid-project/labgrid/blob/master/labgrid/driver/powerdriver.py#L314-L332

By running ykushcmd -g a ykushcmd returns the port number of the USB that supplies the board with power, at least in my configuration.

$ ykushcmd -g a
Downstream port 15 is ON

By default ykushcmd command supports option to [on, off, cycle] whole
ykush yepkit.
> ykushcmd -u a  # 'a' stands for all ports
> ykushcmd -d a  # 'a' stands for all ports
In current build only int is supported for port number, changing it
to string allows to control power on all ports with one command instead
of 3 if there are 3 ports.

Signed-off-by: Redzynia, MateuszX <mateuszx.redzynia@intel.com>
[bst: adjusted docs accordingly]
Signed-off-by: Bastian Krause <bst@pengutronix.de>
Copilot AI review requested due to automatic review settings February 2, 2026 16:35
@Bastian-Krause Bastian-Krause force-pushed the redzynix-add-string-support-to-ykushpowerport branch from af7f668 to bafb51a Compare February 2, 2026 16:35
@Bastian-Krause Bastian-Krause added enhancement and removed needs author info Requires more information from the PR/Issue author labels Feb 2, 2026
Copy link
Member

@Bastian-Krause Bastian-Krause left a comment

Choose a reason for hiding this comment

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

Okay, since multiple people seem to be interested in this, I've added documentation for the use case and I think we can merge this.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR extends the YKUSHPowerPort resource to support string values for the port index parameter, enabling users to specify 'a' to control all ports on a YKUSH USB hub simultaneously instead of controlling each port individually.

Changes:

  • Modified YKUSHPowerPort and NetworkYKUSHPowerPort to accept string index values instead of integers
  • Updated documentation to reflect the string type and document the 'a' option for controlling all ports

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
labgrid/resource/ykushpowerport.py Changed index parameter from int to str type with string validator and converter for both YKUSHPowerPort and NetworkYKUSHPowerPort classes
doc/configuration.rst Updated YKUSHPowerPort documentation example to use quoted string for index and added documentation for using 'a' to control all ports

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@codecov
Copy link

codecov bot commented Feb 2, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 45.1%. Comparing base (d02396e) to head (bafb51a).
⚠️ Report is 2 commits behind head on master.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@          Coverage Diff           @@
##           master   #1272   +/-   ##
======================================
  Coverage    45.1%   45.1%           
======================================
  Files         173     173           
  Lines       13712   13712           
======================================
  Hits         6188    6188           
  Misses       7524    7524           
Flag Coverage Δ
3.10 45.1% <100.0%> (ø)
3.11 45.1% <100.0%> (?)
3.12 45.1% <100.0%> (ø)
3.13 45.1% <100.0%> (ø)
3.14 45.1% <100.0%> (ø)
3.9 45.1% <100.0%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@Bastian-Krause Bastian-Krause merged commit dbd4b11 into labgrid-project:master Feb 2, 2026
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants