Skip to content
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 Radxa's Rock Pi 4(C+) Single Board Computers #902

Merged
merged 6 commits into from
Feb 14, 2023

Conversation

wgroeneveld
Copy link
Contributor

Hi,

I've been fiddling with a Raspberry Pi clone for a while and managed to get CircuitPython running on it. I've contributed the correct pinout here this week: adafruit/Adafruit_Blinka#651
But since I wanted to do something in Go with I2C, I stumbled upon Gobot, which doesn't have the correct mapping either. Here's my attempt to add compatibility in Gobot.

Please note that, although I've read the CONTRIBUTING guide, I am unable to run unit tests thanks to a nasty dependency: https://github.com/warthog618/gpiod which literally states: "For other platforms, where you intend to cross-compile for Linux, don't attempt to compile the package when it is installed". On my Mac, I get a lot of errors like "../../../../go/pkg/mod/github.com/warthog618/[email protected]/gpiod.go:326:35: undefined: uapi.LineInfo". On the RockPi itself, the Go version isn't recent enough.
But since the rockpi_adaptor.go is based on the Pi's, and it has reduced functionality, and the most important aspect is the mapping, I figured this is as good as it gets.

Note that Radxa's boards aren't great when it comes to support/compatibility with existing software. The character device gpiod driver doesn't seem to work, and PWM only works through the mraa lib, which could be implemented using cgo, but I didn't yet. I think that's for another contributor!

Thanks for any feedback! Cheers

@gen2thomas
Copy link
Collaborator

Hi @wgroeneveld , thanks for sharing your nice work. I will have a deeper look in the next days. What I can say immediately:

  • please can you add some of your description of the PR to the README.md, e.g. in a section I2C, PWM, GPIO etc.
  • if the gpiod chracter device is known not to work, just remove the "system.WithDigitalPinGpiodAccess()" in the constructor (line 52)

@gen2thomas gen2thomas self-requested a review February 10, 2023 13:44
@wgroeneveld
Copy link
Contributor Author

Great, thanks, I did that, and I managed to add tests by locally removing the problematic dependency!

platforms/rockpi/README.md Show resolved Hide resolved
platforms/rockpi/rockpi_adaptor.go Show resolved Hide resolved
platforms/rockpi/rockpi_adaptor.go Outdated Show resolved Hide resolved
platforms/rockpi/rockpi_adaptor_test.go Outdated Show resolved Hide resolved
platforms/rockpi/rockpi_adaptor_test.go Outdated Show resolved Hide resolved
platforms/rockpi/rockpi_adaptor_test.go Outdated Show resolved Hide resolved
platforms/rockpi/rockpi_adaptor_test.go Outdated Show resolved Hide resolved
platforms/rockpi/README.md Outdated Show resolved Hide resolved
@gen2thomas
Copy link
Collaborator

Hi @wgroeneveld I have finished the PR review for now. For your remark

On my Mac, I get a lot of errors like "../../../../go/pkg/mod/github.com/warthog618/[email protected]/gpiod.go:326:35: undefined: uapi.LineInfo". On the RockPi itself, the Go version isn't recent enough.

We use v0.8.0 in gobot and I had not tested the v0.8.1 yet on my Linux developer machine. If your problem persists, please file an issue at gpiod, because it is most likely not caused by gobot.

@wgroeneveld
Copy link
Contributor Author

Thank you for your review @gen2thomas , I've changed the files as requested.
I know the gpiod issue isn't caused by gobot, see my previous comments.

@wgroeneveld
Copy link
Contributor Author

Okay I solved the two open review issues, I think everything looks good now!

@gen2thomas
Copy link
Collaborator

Yes, thanks for your rework. I will going to merge now.

@gen2thomas gen2thomas merged commit 64b7c9c into hybridgroup:dev Feb 14, 2023
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.

2 participants