-
Notifications
You must be signed in to change notification settings - Fork 189
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.
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
Add support for GPIO Control using libgpiod (as sysfsgpio is deprecated) #1569
Conversation
db29152
to
d114043
Compare
Rebased so that the commits are no longer behind the main branch :) |
e6486a4
to
4003d51
Compare
A button can be used to simulate pressing a physical button on a device. For example a DigitalOutputButtonDriver can be used when a reset button is connected via a relay to a DigitalOutput. In addition there are the ManualButtonDriver and ExternalButtonDriver drivers analogous to the ManualPowerDriver and ExternalPowerDriver drivers. Operations on a button include "press", "release", "press_for", and "get". Signed-off-by: Perry Melange <[email protected]>
…tput The "invert" method should invert the value of a digital output. Signed-off-by: Perry Melange <[email protected]>
Add the LibGPIO, NetworkLibGPIO and MatchedLibGPIO resources. These resouces bind to the /dev/gpiochipN and it's associated gpio line. In addition there is an optional active_low attribute which can be used to invert the logical value used on the gpio line. The resources SysfsGPIO, NetworkSysfsGPIO and MatchedSysfsGPIO resources have been modified to have an additional optional active_low attribute which can be used to invert the logical value used on the gpio line. The SysfsGPIO, NetworkSysfsGPIO and MatchedSysfsGPIO resouces are now marked as deprecated. See https://www.kernel.org/doc/Documentation/gpio/sysfs.txt Signed-off-by: Perry Melange <[email protected]>
Add the LibGPIODigitalOutputDriver which implements the DigitalOutputProtocol, ResetProtocol, PowerProtocol, and ButtonProtocol. Update the GPIODigitalOutputDriver to also implement the ResetProtocol, PowerProtocol, and ButtonProtocol. Update the GpioDigitalOutput agent to use the new active_low attribute Mark GPIODigitalOutputDriver and GpioDigitalOutput as deprecated. See https://www.kernel.org/doc/Documentation/gpio/sysfs.txt Signed-off-by: Perry Melange <[email protected]>
Add the LibGPIOExport class to the exporter. Update the GPIOSysFSExport class to use the new active_low attribute Signed-off-by: Perry Melange <[email protected]>
4003d51
to
987021c
Compare
…otocol Add NetworkLibGPIO to the client, including commands for io, power, and button protocols. Update NetworkLibGPIO to support the commands for power and button protocols. Signed-off-by: Perry Melange <[email protected]>
…Drivers Add documentation for LibGPIO, MatchedLibGPIO, ManualButtonDriver, ExternalButtonDriver, DigitalOutputButtonDriver, and LibGPIODigitalOutputDriver. Update the SysfsGPIO, MatchedSysfsGPIO, and GpioDigitalOutputDriver to add the new active_low attribute and the new protocols supported by the GpioDigitalOutputDriver. In addition, add documentation that the SysfsGpio classes are deprecated. Signed-off-by: Perry Melange <[email protected]>
Signed-off-by: Perry Melange <[email protected]>
Signed-off-by: Perry Melange <[email protected]>
Signed-off-by: Perry Melange <[email protected]>
987021c
to
2ede768
Compare
@@ -906,6 +938,8 @@ def digital_io(self): | |||
drv = self._get_driver_or_new(target, "HttpDigitalOutputDriver", name=name) | |||
elif isinstance(resource, NetworkDeditecRelais8): | |||
drv = self._get_driver_or_new(target, "DeditecRelaisDriver", name=name) | |||
elif isinstance(resource, NetworkLibGPIO): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With NetworkSysfsGPIO
, lines are requested by the kernel and GPIOs remain driven even after labgrid-client
terminates. With LibGPIODigitalOutputDriver
, it seems to me that this won't be the case.
To quote gpioset(1)
:
The line output state is maintained until the process exits, but after that is not guaranteed.
In practice, some kernel drivers keep driving the last output and others don't. Does your PR address this issue?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With all the tests I have done, the line state stays the same. Also, I am not calling command gpioset
After completion of a command via labgrid-client, such as power on
, the line stays on. Afterwards, I can get with power get
and call power cycle
and the GPIO lines act as expected. Do you have a specific test scenario that I could use to test with?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With all the tests I have done, the line state stays the same.
I haven't tested on a Raspberry Pi, but you can try:
- boot kernel with
pinctrl_bcm2835.persist_gpio_outputs=n
- remove optocoupler/latches and work on the raw GPIO pin directly
- attach weak pull-down
- set GPIO high
- terminate GPIO setting program
- measure GPIO line
Raspberry Pi users have a way out using above module parameter, but in general, it's up to the driver what happens when a line is released.
If this happens to work, one would need to check what is preventing the kernel from calling bcm2835_pmx_gpio_disable_free()
, which would mux the pin as input.
Also, I am not calling command gpioset
This behavior is inherent to the GPIOD API, gpioset
is only one example of a tool affected by it.
After completion of a command via labgrid-client, such as power on, the line stays on. Afterwards, I can get with power get and call power cycle and the GPIO lines act as expected. Do you have a specific test scenario that I could use to test with?
Undefined can mean that it works for some, because they have the lucky combination of hardware, kernel module parameter, other GPIOs requested on the same controller, ... etc, but it's not universal.
The GPIOD DBus API is one solution for this problem. I don't know if it's a suitable solution for Labgrid, but replacing SysFS usage with libgpiod without ensuring the lines remain requested won't cut it, I think.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just did a bit more research. https://pip.raspberrypi.com/categories/685-whitepapers-app-notes-compliance-guides/documents/RP-006553-WP/A-history-of-GPIO-usage-on-Raspberry-Pi-devices-and-current-best-practices.pdf on page 9 had some information which I didn't know before.
I have modified the config parameters on the RaspberryPi4 which I am using to include dtparam=strict_gpiod
. In this mode, I see the instability of the gpio line which you have described. But only on gpiochip0. On the HATs which I am using I do not see the instability.
Is this the reason that an agent was used for the GpioDigitalOutput class?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
An agent is just a way to execute some Python code on the exporter. The main issue with the gpiochip/libgpiod API in some cases state is lost as soon as labgrid-client io high
exits (which also terminates any agents it started on the exporter, closing the gpiochip FD).
I'm not sure if that has been solved with the daemon and D-Bus API. If so, using that instead of the sysfs could be an option.
There have been past discussion about this fyi, e.g. #468 This shouldn't mean that libgpiod is not wanted (I don't make maintenance decisions anyway), but just so you are aware of past arguments for and against. |
I have created a new PR ( #1583) which does not include the LibGPIO stuff. The new PR adds the new Button Protocol, the "invert" command to the IO calls, and the new active_low attribute. Working with dbus and libgpio will have to wait until I have more time to work on it. Using dbus requires using the "testing" repos from Debian and I figured that it might just be a bit too early to integrate it into labgrid. |
Regarding the LibGPIO discussion, this week I tried to use SysfsGPIO with an exporter v24.0.2 on a Pi 3+ with the current bookworm release, which was unsuccessful. I discovered that Bookworm dropped the sysfsgpio support altogether. So I think in the Future it will become unavoidable to switch from the deprecated sysfs to a supported alternative for some plattforms. |
That seems quite strange to me. I don't have a pi 3, but it's using bookworm with sysfsgpio. Could you please post your export.yaml file? The problem with getting libgpio up and running with dbus is that it's really quite new and the librarie versions that libgpiod are based upon are in debian/testing. I don't think that having debian/testing as minimal requirements for labgrid will be accepted. |
OK, after your comment, i went back and set up different Bookworm versions for testing. I tried the sysfs manually on fresh Boards with no additional software installed, and it seems that sysfs support is only dropped in the latest release, in older Pi OS Versions with Bookworm, everything works fine. So it's no problem for me to stay with an older Version at the moment, but I think it's good to know that Labgrid SysfsGPIO driver will not work with the current Pi OS release and assumable newer versions from now on. |
Description
As far as new features go...
ButtonProtocol is to be used in my test environment to control pressing the reset button of DUTs. For many of the DUTs, pressing the reset button when powering on provides (via uboot) a TFTP client or server or simply access to a web socket to be able to upload a new firmware image. A logical button is better for the test environment I am creating since it encapsulates the high/low logic levels of IO ports into something logical.
LibGPIO classes are added to get away from the deprecated sysfsgpio subsystem (see https://www.kernel.org/doc/Documentation/gpio/sysfs.txt).
Extending the GPIO based drivers (both libgpio and sysfs) to the Reset, Power, and Button protocols was very straight forward and I did wonder why the Reset and Power protocols had not jet been implemented. They seem like logical extensions to general purpose IO lines.
Adding the active_low attribute was necessary because the IO lines used in the testbed are connected to relays which are active_low.
All development and testing was done on a raspberry pi 4 with opto-coupler relays connected to the GPIO lines to control power and reset buttons.
Checklist