host
host copied to clipboard
gpioioctl:Implement ioctl access to Linux GPIO chips/lines.
Fixes host issue #39
First, let me start by saying I tend to work in lots of languages, and my fluency in golang is not really high. If you see non-idiomatic things you can't live with, just let me know. Similarly, I know you're the maintainer, and it's your call in implementations.
This is a pretty big add. If you feel like a call would be beneficial, I can do a voice call.
Terminology
I'm using the term "Line", and not "Pin". Line does implement conn/v3/gpio.PinIn, PinOut, and PinIO. This is kind of a difference. Line is an attribute of the chip, while pin is an attribute of the board. It seems to me that we want the code to work across boards, regardless of how a board physically presents a GPIO chipset's line. So a pin is a physical element that the chip's line is connected to. I can see potentially a GPIO line that's not exposed as a physical pin on a board. As an aside, the ioctl()s use the Line terminology. However, if you think that's unnecessarily confusing, I can change it.
Tests
Refer to Tests.md
Interfaces
consumer
In the ioctl(), there's a "consumer" field. The kernel docs say:
a functional name for the consumer of this GPIO line as set by whatever is using it,
will be empty if there is no current user but may also be empty if the consumer doesn’t
set this up
I'm writing the program_name@pid into this field. I can see other programs use it too. For example, I see PPS bound to a GPIO Chip on my Pi. You might consider adding
func Consumer() string
to the gpio.Pin interface for the next revision.
PinFunc
I really don't understand the PinFunc interface, so I didn't implement it. I looked it over, and from what I could tell, I didn't like the idea, but perhaps I didn't understand it. If you can give me a better idea of the intent, I'll give it a go.
PWM
PWM isn't really part of the GPIO ioctl User Space API. The kernel docs say:
Do NOT abuse userspace APIs to control hardware that has proper kernel drivers.
There may already be a driver for your use case, and an existing kernel driver
is sure to provide a superior solution to bitbashing from userspace.
I've stubbed in a PWM method to satisfy PinOut, but it should probably be deprecated from the PinOut interface. Here's the what the doc says:
pwm-gpio: drivers/pwm/pwm-gpio.c is used to toggle a GPIO with a high resolution
timer producing a PWM waveform on the GPIO line, as well as Linux high resolution
timers can do.
MultiPin IO
Multi-Pin IO is handled by the new construct LineSet. The ioctl() interface makes this pretty straight forward. Single line input/output are identical to multi-line. Other than configuration, no clever tricks are required. You just call Read() and it returns the pin values as a uint64 bitmap. Essentially, a LineSet provides methods for reading or writing all lines in one go. LineSetLine provides convenience methods to allow writing individual pins and implements PinIn, PinOut, and PinIO. Additionally, functions of pins can be different within the set. Some can be input, and others output.
A common use case for a LineSet might be doing a HD44780 LCD display which has 4 or 8 lines + a strobe line, and a backlight line.
I haven't looked at the code yet but answering a few questions for sake of speed:
- PinFunc is mostly relevant for MCU provided hardware pins, so it's okay to ignore for now.
- PWM: sounds good. It's okay to just return an error.
- Line vs Pin and multi-pins: it's something I wanted to create a conn/v4 for but lost steam sadly as the refactor became big; I want to make all stateful functions like PWM() to accept a context.Context.
:warning: Please install the to ensure uploads and comments are reliably processed by Codecov.
Codecov Report
Attention: Patch coverage is 12.87879% with 460 lines in your changes missing coverage. Please review.
Project coverage is 26.7%. Comparing base (
50d4ed0) to head (9204ccc).
| Files with missing lines | Patch % | Lines |
|---|---|---|
| gpioioctl/gpio.go | 21.8% | 238 Missing and 2 partials :warning: |
| gpioioctl/lineset.go | 0.0% | 173 Missing :warning: |
| gpioioctl/ioctl.go | 0.0% | 47 Missing :warning: |
Additional details and impacted files
@@ Coverage Diff @@
## main #59 +/- ##
=======================================
- Coverage 27.4% 26.7% -0.7%
=======================================
Files 89 92 +3
Lines 11011 11538 +527
=======================================
+ Hits 3014 3081 +67
- Misses 7864 8322 +458
- Partials 133 135 +2
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
I'll merge once the checks are green.
@maruel So i'm pretty sure the reason that netlink was not loading in host init() is because the pipeline blocked infinitely trying receive the message from the netlink socket. I got creative and added a 2s timeout to the onewire.Init(). That seems to stop the pipeline from timing out. It now passes the Linux pipeline build. I tested it on a pi with a couple of DS18B20 units attached and it worked as expected.
I've fixed all of the lint issues in gpioioctl, sysfs, and netlink, which is code that I've touched.
The rest is all gosec G115 possible overflow errors. I don't think they're false positives. I'd recommend disabling that check in the pipeline.
@maruel Were you waiting on me for something more? I think I've got everything you asked for completed.
Yes sorry I just needed to silence gosec, done in https://github.com/periph/host/commit/1e513a0cdf6a479e05b38984a467787868c7d006.
Thanks a lot! It's really appreciated.