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

Support for GPIO Bank 1 when using Raspberry Pi Compute Module 3 #762

Open
mjjames opened this issue Oct 3, 2019 · 8 comments · May be fixed by #765

Comments

@mjjames
Copy link

commented Oct 3, 2019

Although the Raspberry PI 3 Driver works very well it is currently limited to the number of GPIO pins on the Raspberry PI 3. The Raspberry Pi Compute Module 3 has an additional bank of GPIO's so you can use 1-45.

I propose that we make the ValidatePinNumber method protected virtual so that a RaspberryPiComputeModule3 driver is introducted that inherits from the RaspberryPi3 Driver and simply extends the known GPIO's.

Everything else is pretty much the same in terms of implementation I believe.

@mjjames

This comment has been minimized.

Copy link
Author

commented Oct 3, 2019

I've created a fork and started working on this. The GpioController also needs updating as the GetBestDriverForRaspberryPiRevision method needs to consider the revision number to determine to use the CM3 driver or the current Pi 3 driver

@mjjames mjjames referenced a pull request that will close this issue Oct 3, 2019
@Frankenslag

This comment has been minimized.

Copy link
Contributor

commented Oct 3, 2019

@mjjames
I had a quick scan of the fork and note that you are detecting the cm3 by using values read from procinfo. You may have problems with this outside of raspbian please see #661

@mjjames

This comment has been minimized.

Copy link
Author

commented Oct 3, 2019

I agree, happy to discuss if working on #661 is required before completing this. It makes sense to fix an underlying issue than build on top of it

@Frankenslag

This comment has been minimized.

Copy link
Contributor

commented Oct 3, 2019

@mjjames

I have asked @joperezr if #661 should be resurrected now. It was postponed as we didn’t want to clash with the already mostly tested 3.0 release and there is a workaround by explicitly specifying the driver.

What #661 is trying to do is identify a pi whereas your proposed pr is looking for the type of pi and I guess they are separate and the connection is that they are in the same code area and need to be aware of the way that other os’s work. This is why I chose to read the device tree as it was part of the firmware package and likely to be available on all Linux based os’s.

Thoughts are that if @joperezr agrees I resubmit the or associated with #661 as it improves on something already there and you, knowing what’s in there, can continue to work on your pr.

Very happy to discuss further.

@mjjames

This comment has been minimized.

Copy link
Author

commented Oct 4, 2019

After coming back to this with fresh eyes, the ConvertPinNumberToLogicalNumberingScheme doesn't make alot of sense for the ComputeModule3. Would it be acceptable to throw a NotSupportedException here?

@krwq can you comment on this and regarding the tests earlier up the convo?

@Frankenslag

This comment has been minimized.

Copy link
Contributor

commented Oct 4, 2019

@mjjames

PR #766 created.

@joperezr

This comment has been minimized.

Copy link
Member

commented Oct 4, 2019

Is the problem here really just that when you try to use the existing RaspberryPi3 driver it is failing because of validation when trying to do an operation on those extra GPIOs? If so, I would rather provide a way to opt-out of that validation or to pass in the number of pins that should be available instead of creating a whole new driver for this.

@joperezr joperezr added this to the Future milestone Oct 4, 2019
@krwq

This comment has been minimized.

Copy link
Member

commented Oct 11, 2019

@mjjames

After coming back to this with fresh eyes, the ConvertPinNumberToLogicalNumberingScheme doesn't make alot of sense for the ComputeModule3. Would it be acceptable to throw a NotSupportedException here?

Yes, fine to throw if it doesn't make sense. Also please try to make the code as simple as possible (basically what @joperezr wrote)

@krwq can you comment on this and regarding the tests earlier up the convo?

Not sure what's the question is about the tests but adding implementation for compute module as a workaround and then improving detection logic as a next step sounds good to me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.