Skip to content

Add input path for sysfs GPIOs#1935

Open
ozan956 wants to merge 4 commits into
labgrid-project:masterfrom
ozan956:add-gpio-input-support
Open

Add input path for sysfs GPIOs#1935
ozan956 wants to merge 4 commits into
labgrid-project:masterfrom
ozan956:add-gpio-input-support

Conversation

@ozan956

@ozan956 ozan956 commented Jul 13, 2026

Copy link
Copy Markdown

This is a follow-up to the GPIO input work discussed in #1458. Instead of adding a separate input-only sysfs agent, this PR keeps the existing sysfsgpio agent and extends it so a GPIO line can be configured either as output or input by the active driver.

The change adds:

  • DigitalInputProtocol, for drivers which can read a digital signal without
    necessarily supporting writes.
  • GpioDigitalInputDriver, binding to SysfsGPIO, MatchedSysfsGPIO, and
    NetworkSysfsGPIO.
  • labgrid-client io get <name> support for explicitly configured
    DigitalInputProtocol drivers.
  • Sysfs GPIO agent support for input reads, while preserving output as the
    default behavior for existing users.

DigitalOutputProtocol now derives from DigitalInputProtocol, matching the existing behavior of digital output drivers which already implement get(). This keeps existing output users compatible while allowing input-only drivers to use the same CLI command for reads.

The client-side behavior is intentionally conservative: io get first looks for a configured DigitalInputProtocol driver. If no such driver exists, the existing resource-only fallback still creates a GpioDigitalOutputDriver for NetworkSysfsGPIO, so existing configurations continue to work as before.

The sysfs agent caches GPIO line objects, so this PR also makes cached lines reconfigure their direction before reuse. This avoids stale direction state if the same GPIO line is accessed through different configured drivers during a test setup.

The implementation still uses the existing sysfs GPIO backend. Moving this to the newer gpiochip/libgpiod interface would be useful, since sysfs GPIO is deprecated, but that is intentionally left as future work to keep this PR small and focused.

Checklist

  • Documentation for the feature
  • 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
  • PR has been tested

cc @Emantor @flxzt

@Emantor

Emantor commented Jul 13, 2026

Copy link
Copy Markdown
Member

Why are you taking authorship for most commits instead of working on top of @flxzt commits?

@codecov

codecov Bot commented Jul 13, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 96.82540% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 61.3%. Comparing base (64723a5) to head (3f87f22).
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
labgrid/driver/gpiodriver.py 95.4% 1 Missing ⚠️
labgrid/remote/client.py 66.6% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff            @@
##           master   #1935     +/-   ##
========================================
+ Coverage    61.0%   61.3%   +0.3%     
========================================
  Files         182     183      +1     
  Lines       14881   14919     +38     
========================================
+ Hits         9083    9155     +72     
+ Misses       5798    5764     -34     
Flag Coverage Δ
3.10 61.3% <96.8%> (+0.3%) ⬆️
3.11 61.3% <96.8%> (+0.3%) ⬆️
3.12 61.3% <96.8%> (+0.3%) ⬆️
3.13 61.3% <96.8%> (+0.3%) ⬆️
3.14 61.3% <96.8%> (+0.3%) ⬆️

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

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

@ozan956

ozan956 commented Jul 13, 2026

Copy link
Copy Markdown
Author

Hi @Emantor,

There was only one original commit containing all the changes. I built on top of it, added my own changes, and reworked parts of the implementation.

I included a Reported-by tag, but I can change it to Signed-off-by or Co-developed-by, depending on which one best matches labgrid’s contribution guidelines.

I’m happy to update the commit metadata accordingly. Let me know which format you prefer.

Comment thread labgrid/remote/client.py
try:
drv = target.get_driver("DigitalOutputProtocol", name=name)
if action == "get":
drv = target.get_driver("DigitalInputProtocol", name=name)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I believe this implementation in the client has the same issue as I had when I attempted to implement this.
Have you checked whether #1458 (comment) also happens for you?

@ozan956 ozan956 Jul 14, 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.

Yes, tested on a Raspberry Pi 2 Model B running Raspbian with kernel 6.12.47+rpt-rpi-v7.

I tested whether labgrid-client io get changes the sysfs direction from out to in after setting the line high:

$labgrid-client -p rpi-gpio io high
cat /sys/class/gpio/gpio529/direction
out
cat /sys/class/gpio/gpio529/value
1

$labgrid-client -p rpi-gpio io get
digital IO for place rpi-gpio is high

cat /sys/class/gpio/gpio529/direction
out
cat /sys/class/gpio/gpio529/value
1

So I could not reproduce the behavior from the linked comment with this branch. After io get, the line remains configured as out and the value remains high.

This should be because the resource-only NetworkSysfsGPIO fallback still uses GpioDigitalOutputDriver for labgrid-client io get. The new DigitalInputProtocol path is only used when an input driver is explicitly configured.

@flxzt

flxzt commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

I would appreciate if attribution is kept in some way.

@Emantor

Emantor commented Jul 13, 2026

Copy link
Copy Markdown
Member

Hi @Emantor,

There was only one original commit containing all the changes. I built on top of it, added my own changes, and reworked parts of the implementation.

I included a Reported-by tag, but I can change it to Signed-off-by or Co-developed-by, depending on which one best matches labgrid’s contribution guidelines.

I’m happy to update the commit metadata accordingly. Let me know which format you prefer.

I usually retain the original authorship in the commit and add my Co-developed-by for commits that I changed, but ultimately this is up to @flxzt.

@flxzt

flxzt commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

Co-developed-by me would be fine for me

ozan956 added 4 commits July 14, 2026 09:27
Add a dedicated DigitalInputProtocol for drivers which can read a
digital signal without also exposing writes. Keep DigitalOutputProtocol
as a specialization of the input protocol, matching existing output
drivers which already implement get().

Co-developed-by: Felix Zwettler <Felix.Zwettler@duagon.com>
Signed-off-by: Felix Zwettler <Felix.Zwettler@duagon.com>
Signed-off-by: Ozan Durgut <ozan.durgut@analog.com>
Add GpioDigitalInputDriver for sysfs GPIO resources. Extend the
sysfsgpio agent to configure lines for input reads while keeping
output as the default path for existing users.

Reconfigure cached GPIO lines before reuse so switching between
input and output access leaves the sysfs direction in the state
requested by the active driver.

Co-developed-by: Felix Zwettler <Felix.Zwettler@duagon.com>
Signed-off-by: Felix Zwettler <Felix.Zwettler@duagon.com>
Signed-off-by: Ozan Durgut <ozan.durgut@analog.com>
Resolve labgrid-client io get via DigitalInputProtocol when a matching
driver is configured. This allows input-only drivers to use the
existing io command.

Keep the existing NetworkSysfsGPIO fallback on GpioDigitalOutputDriver
so resource-only configurations continue to behave as before.

Co-developed-by: Felix Zwettler <Felix.Zwettler@duagon.com>
Signed-off-by: Felix Zwettler <Felix.Zwettler@duagon.com>
Signed-off-by: Ozan Durgut <ozan.durgut@analog.com>
Document GpioDigitalInputDriver and add coverage for sysfsgpio input
direction handling, cached direction reconfiguration, input driver
reads, protocol compatibility, and labgrid-client io get routing.

Co-developed-by: Felix Zwettler <Felix.Zwettler@duagon.com>
Signed-off-by: Felix Zwettler <Felix.Zwettler@duagon.com>
Signed-off-by: Ozan Durgut <ozan.durgut@analog.com>
@ozan956
ozan956 force-pushed the add-gpio-input-support branch from d117758 to 3f87f22 Compare July 14, 2026 07:28
@ozan956

ozan956 commented Jul 14, 2026

Copy link
Copy Markdown
Author

Thanks @flxzt and @Emantor, I updated the commit trailers/sign-offs accordingly. 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants