-
Notifications
You must be signed in to change notification settings - Fork 17
Added gpiod shell driver, added SPIBus class, added support for MCP3004/MCP3008 #7
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
Conversation
…fic methods. refactored getFunction to use gpiod
…n-interface Feature/replace deprecated pin interface
…n-interface refactor: remove unnecessary code
… function to the Commandable interface since it's a hard requirement
…s because the underlying method was silent
… it was set causes gpiod to wipe the state
… and sometimes present as the wrong state being shown in the command outpu
…return self to make chaining the disableChip() method possible
Feature/mcp library
|
Wow, thanks so much for your contributions here!! I'll give this a proper review over the next few days, but here are my initial thoughts: GPIOD Shell DriverThanks for doing this. The use of caching in there is really clever. I previously explored using the shell to control the pins (https://github.com/danjohnson95/pinout/blob/main/src/Shell/RaspiGpio.php), but I found that it was just a bit too slow, especially noticeable when interacting with an LCD display. But it looks like yours is much more efficient than mine, so I'll give it a go. Which version of Raspian are you running and which model of Pi? SPIBusAmazing! I've got some SPI displays I've been meaning to write a driver for, so this will come in super handy ⭐ MCP300XVery handy, and will open up the path to us being able to support lots more components. I think I've got some of those chips lying around at home so I'll give this a go. |
|
Hi there, The caching is actually necessary as it turns out. The gpiod shell commands automatically cast the pin function based on what you do. EG: if you set a pin to output high, then subsequently read the state, it goes low because it gets cast as an input pin so the caching layer is there to allow you to query the state of output pins without messing up their active config. I hope it's fast enough, I'd definitely be interested to see how it works with your LCD. I'm open to other ideas, I just needed something that worked since the syslog gpio system is totally gone on my Pi. I'll get back to you on the Raspbian version but it's Raspbian Lite and I only installed it a week or so ago. I'm running it on my 3B. Incidentally, the other thing I had issues with was with my model 1B, there's no PHP 8.x builds available for ARMV6 so you have to build from source to run Laravel on the 1B or the Zero. Just a quick note that the SPI Bus implementation isn't complete atm. It handles mode 0 (the one the MCP300Xs use) well enough but 1,2 and 3 are untested since I haven't got any of those devices. I've tried to build it with the other modes in mind but it's not supported atm. I might rig it up with an arduino as a serial monitor so I can test the different modes and pull together a full implementation. I'm also gonna go back through and add some test coverage where I can think how to reasonably test it. Cheers |
|
Hi, Grabbed some system info off the pi for you Cheers |
…inIsInput() and assertPinIsOutput() methods
…led in set clock ( I may refactor this but for now it's okay ).
Testing and misc changes to aid testing or fix minor bugs
|
Hi there, Apologies for adding yet more code haha. I've gone through the SPIBus and MCP300X and added what are think are some good baseline tests? I'm open to ideas on the testing though as I'm not entirely sure how to test the actual SPI driving bit at the moment. I'm intending on working on the SPI driver soon-ish to get modes 1,2 and 3 working since I only targetted mode 0 (though I did attempt to build in a way that should make adding the other modes easier). However if it may end up the case that I need to load a per-mode subdriver into the SPI class. Hope that's all okay 🙌 |
danjohnson95
left a comment
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.
Thanks again for your hard work here.
I've tried out your branch on a Raspberry Pi 3 Model A+ using Linux 12 (Bookworm), and you're right that the SysFile approach doesn't work there, but your libgpiod implementation works really nicely. As expected it is slower - here's a video of how it works with the LCD driver:
IMG_3424.mov
I think we're only going to see better performance with a PHP extension that directly interacts with the GPIO pins through C.
That being said, I definitely think we should merge this PR in as a simple way to get going with GPIO pins using newer versions of Linux.
I've just got one suggestion around the naming of the shell class, and then I think we're good to go 🚀
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.
Since this class is using the libgpiod CLI tool and not the sys files, I reckon we should rename this class LibGPIOD. Thoughts?
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.
Ahhh yeah, that's a great suggestion. The significance of the SysFile name hadn't clicked until you pointed this out.
Sounds good to me.
Yeah I noticed it's not the speediest and I had considered looking into building a PHP extension to do it but I'm a bit busy to actually take that on a the moment. Is a bit of a shame that the SysFile approach is deprecated on account of the benefits it offers here.
I think if you have existing projects or a specific implementation where you need the older one you can use it, it's still the default in the config AFAIK. Maybe that needs noting somewhere, you need to change the config to use the new class, I just left it default as the old one to avoid a breaking change for existing users.
Fixes issues in LibGPIOD class (from PR danjohnson95#7): - Add -c flag to all gpioinfo, gpioget, and gpioset commands (required syntax) - Fix gpioget output parsing to handle "26"=active/inactive format - Add background process management for gpioset to hold pins HIGH/LOW - Improve error handling with proper command execution Also includes improvements to PinService and SetCommand: - Fix PinService::setLevel() to refresh pin state after setting - Fix SetCommand to display both level and function in output - Move publishes() to boot() method for proper Laravel convention The original SysFile class remains unchanged for backwards compatibility. Users can opt-in to the improved LibGPIOD version by setting: 'sys_file' => \DanJohnson95\Pinout\Shell\LibGPIOD::class These fixes resolve issues where: - gpioset/gpioget commands failed due to missing -c flag - Pins didn't stay HIGH/LOW (gpioset released immediately) - gpioget output parsing failed with "26"=inactive format - Pin state wasn't refreshed after changes
Hi, I accept it's quite a big PR.
GPIOD Shell Driver - I had issues with the /sys/class/gpio interface not being present on my Pi, it seems to have been deprecated a fair while ago so I've just made a version that uses the new gpiod underneath and made the commandable::class bind driven by a config so you can swap it as required.
SPIBus - The MCP300X chips use an SPI interface so I made a generic SPIBus class that bit-bangs the SPI Interface. Idea being it can be used in other libs if required.
MCP300X support - added a driver for the MCP3008 (MCP3004 is thrown in for free because it's just a smaller 4 pin version that ignores a bit in the same payload). Tried to make the dev experience of using this as nice as possible so all the low-level SPI stuff is handled for you.
Cheers