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

[Proposal] Provide controller provider interfaces #125

Closed
shaggygi opened this issue Dec 29, 2018 · 17 comments
Closed

[Proposal] Provide controller provider interfaces #125

shaggygi opened this issue Dec 29, 2018 · 17 comments
Assignees
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-System.Device.Gpio Contains types for using general-purpose I/O (GPIO) pins

Comments

@shaggygi
Copy link
Contributor

shaggygi commented Dec 29, 2018

This proposal is to create new controller interfaces for the various types of I/O within System.Device.API. This is similar (but not exactly) to Windows.Devices.Gpio.Provider.

Rationale and Usage

In most cases, the choice will be to control things with I/O available on main controller boards using the GpioController. However, device bindings could be structured in a way that allow them to provide their own controller that utilizes a particular interface (e.g. I2C/SPI) behind the scenes.

Examples

Use SPI to update related registers when performing a controller Read call.

Mcp23xxx mcp23xxx = new Mcp23S17(spiDevice);
GpioController mcp23xxxGpioController = mcp23xxx.GetDefaultGpioController();
PinValue value = mcp23xxxGpioController.Read(14);

Use the proposed GpioPort to update multiple pins at the same time on a binding.

// Using controller from above.
GpioPort port = new GpioPort(mcp23xxxGpioController, 1, 4, 7);
port.WriteByte(0x06);

Binding that implements multiple providers. This example uses I2C to perform the controller specifics.

public class MyArduinoDoohickey : IAdcControllerProvider, IGpioControllerProvider, IPwmControllerProvider

MyArduinoDoohickey doohickey = new MyArduinoDoohickey(i2cDevice);

// PWM stuff.
PwmController pwmController = doohickey.GetDefaultPwmController();
pwmController.OpenChannel(1, 4); // pwmChip, pwmChannel
pwmController.StartWriting(1, 4, 200, 0.3); // pwmChip, pwmChannel, frequencyInHertz, dutyCyclePercentage)

// ADC stuff (when AdcController is added to API).
AdcController adcController = doohickey.GetDefaultAdcController();
adcController.OpenChannel(5);
double adcValue = adcController.ReadValue(5);
            
// GPIO stuff.
GpioController gpioController = mcp23xxx.GetDefaultGpioController();
gpioController.OpenPin(1, PinMode.Input);
PinValue gpioValue = gpioController.Read(1);

Proposed Change

This proposal should include the following interfaces to get the specific type of controller.
This would allow device bindings to begin implementing with controller functionality.

GPIO

public interface IGpioControllerProvider : IDisposable
{
    GpioController GetDefaultGpioController();    // Gets first (or only one) in list.
    
    // Most cases will only have 1, but could possibly have multiples of same type.
    // Not sure if there would be a way to get a specific if needed?
    IList<GpioController> GetGpioControllers();  // Gets all GPIO controllers.
}

PWM

public interface IPwmControllerProvider : IDisposable
{
    PwmController GetDefaultPwmController();    // Gets first (or only one) in list.
    
    // Most cases will only have 1, but could possibly have multiples of same type.
    // Not sure if there would be a way to get a specific if needed?
    IList<PwmController> GetPwmControllers();  // Gets all PWM controllers.
}

ADC (when AdcController is added to API)

public interface IAdcControllerProvider : IDisposable
{
    AdcController GetDefaultAdcController();    // Gets first (or only one) in list.
    
    // Most cases will only have 1, but could possibly have multiples of same type.
    // Not sure if there would be a way to get a specific if needed?
    IList<AdcController> GetAdcControllers();  // Gets all ADC controllers.
}
@shaggygi shaggygi changed the title [Proposal] Provide interfaces for different controllers [Proposal] Provide controller provider interfaces Jan 4, 2019
@joperezr
Copy link
Member

joperezr commented Jan 4, 2019

I'm not sure I like this idea for one main reason: life cycle. What happens when you do something like this:

GpioController myController;
using (Mcp23xxx mcp23xxx = new Mcp23S17(spiDevice))
{
  myController = mcp23xxx.GetDefaultGpioController();
  // ... some other code
}
// The Mcp device is disposed here which means the controller should be disposed as well, but we still keep a reference to it.
PinValue value = mcp23xxxGpioController.Read(14);

We specifically decided to allow having multiple GpioController objects on the same thread (by not making it a singleton) so that you could workaround this by having one controller on the device and one other controller on your app.

Also I'm not sure how useful this could be, since the idea would be that if the pin 14 had any useful data in your above example, then mcp23xxx would have a method that gets this data out instead of getting the underlying controller and trying to read the data directly on the consumer.

@shaggygi
Copy link
Contributor Author

shaggygi commented Jan 5, 2019

You make a good point. The only other thing I can think of right now is possibly allowing bindings to inherit from GpioControllers if needed, but looks like you can't since it is sealed.

I agree most peeps will interact with the bindings using the basics of reading/writing registers to perform the common features (write memory, read temp, etc.) they offer. I will probably do it that way myself most of the time. I also sort of disagree what interacting directly with the bindings sometimes. While working on the Mcp23xxx, I constantly found myself searching through the datasheet trying to find the specific register(s) and understand what would happen if I wrote a 1 to a specific bit. Having the option of controllers can improve this story.

There are cases where you could have more complex hardware and wish to program against the goodness of a controller instead of calling multiple things to setup similar results. I'll use a couple of my proposal examples to try and explain...

Setup up certain hard config.

The Mcp23S18 has open-drain outputs and also has pull-up resistor registers. While you could write to the registers to setup them up using the bindings Write method... A user might prefer to call something like the following knowing the low-level logic (performed by I2C/SPI) would determine the respective registers/bits to work with.

mcp23xxxController.OpenPin(1, PinMode.OpenDrainPullUp);
mcp23xxxController.Write(1, PinValue.Low);

As a GpioPort

There are expanders with 60 I/O and you could have a need to create a GpioPort that combines pins that are not controlled by the same port or register. You could use the following to group those into a single port and let the low-level logic update. Image having a bi-directional port and having to call the various Read/Write calls to config between input and outputs.

GpioPort port = new GpioPort(bigHonkingGpioController, 1, 57, 42, 23, 24);
port.WriteByte(0x06);
// Do other logic like latching output data and now read data back from external device below.
var busData = port.ReadByte(0x06);

I admit I know this hasn't been completely thought through, but let's keep pondering it. I believe there is something useful with this ability.

@shaggygi
Copy link
Contributor Author

shaggygi commented Jan 5, 2019

Another thought... I had started working on an Mcp23xxxGpioDriver, which implemented GpioDriver and is created by passing in an Mcp23xxx object (that is already setup with I2C/SPI).

Mcp23xxxGpioDriver : GpioDriver

Could you do something like this...

var i2cConnectionSettings = new I2cConnectionSettings(1, 32);
var i2cDevice = new UnixI2cDevice(i2cConnectionSettings);
var mcp23017 = new Mcp23017(i2cDevice);
using (var mcp23xxxGpioController = new Mcp23xxxGpioController(mcp23017))
{
    mcp23xxxGpioController.SetPinMode(3, PinMode.InputPullUp);
    PinValue pinValue = mcp23xxxGpioController.Read(3);
}

At this point, it wouldn't need to implement an IGpioControllerProvider interface.

@joperezr
Copy link
Member

joperezr commented Jan 9, 2019

The problem with allowing them to extend GpioController is that what happens if the device supports multiple protocols (which I believe is the general case)? For example, what if the binding allows Gpio and Spi or I2c communication? I think that because of this, it is not ideal to have a device to extend a controller.

For the other cases that you mention above of really just wanting to interact with the device's registers directly, I'd still argue that this means that the device-binding is incomplete so it would be better (for the community and yourself) to expose that extra functionality on the device binding itself. In my mind, the device binding should really be a sort of direct translation of the device's datasheet for all of the useful information that it exposes.

This was referenced Jan 30, 2019
@RoySalisbury
Copy link

I could have sworn that I posted this exact same topic back in December, but I cant find it. So I will comment on this one.

I have done something similar going all the way back to .NET MF. I had to do it in Win10 IoT with the UWP model, and even did it for the .nF (nano Framework) just a few months ago.

Providing interfaces for IGpioController and IGpioPin is the way to go. Same goes for I2C and SPI.

It makes creating device libraries SO much easier when you don't have to (basically) embed the controller into the driver. If the driver (so for an LCD display) required 8 GPIO pins, then I can get these pins from ANYWHERE (DefaultController, MCP23xxxx chip, or another provider). I could even mix and match them. I use GPIO pins from the Pi for the interrupt enable pins on the MCP23008. I could even pass in interrupt enabled pins from the PI AND read/write gpio's from the MCP chip to my LCD display. I should never be passing in a controller itself.

Here is my repository of nanoFramework drives that I started creating. Take a look at the MCP23008 and MCP230017 chips.

https://github.com/RoySalisbury/nF.Devices

Roy

@joperezr
Copy link
Member

joperezr commented Feb 1, 2019

Yeah now that we have been adding more devices I see more clearly the need for having something like this. At least the fact of having an IGpioController makes a lot of sense because as you mention there are some other devices (such as the GPIO expansion headers) that provide pins just like controllers, so it will be handy to simply enable this such that existing devices have a constructor where they optionally take in a IGpioController where they will get the pins from.

@RoySalisbury
Copy link

RoySalisbury commented Feb 1, 2019

Devices.zip

I have attached the version I did for the UWP model. Look in the GPIO directory and you will see how to implement the necessary interfaces for creating a ControllerProvider. UWP made it a bit harder than it should have been, but it made it easier to work with for drivers.

Roy

@RoySalisbury
Copy link

Yeah now that we have been adding more devices I see more clearly the need for having something like this. At least the fact of having an IGpioController makes a lot of sense because as you mention there are some other devices (such as the GPIO expansion headers) that provide pins just like controllers,

Would not be a stretch for someone to create a bit-banged I2C device driver. I have some I2C devices that have static addresses on them. With only a single i2c bus its one way to get multiple devices with the same address working. But I don't want my device driver to know anything other then it's an I2C device.

My second reason for the interfaces, and its mainly the "framework designer" in me. Stop coming out with a new model once everyone adopts the prior one. :) (I'm being bit sarcastic here).

I was used to working with the .NET MF model, and then along comes UWP. I just get used to that, and alongs comes this one. Each time having to redesign a new driver model around it. The nanoFramework project adopted the UWP model, and once I convinced them to let me add the interfaces, pretty much all my drivers "just worked" (with the exception of needing different assembly references). If the same model is followed here, then all the drivers that people created for UWP will transition over with very little effort.

@shaggygi
Copy link
Contributor Author

shaggygi commented Feb 2, 2019

Would not be a stretch for someone to create a bit-banged I2C device driver.

See #122 as this is similar, but for bit-banged SPI driver.

@shaggygi
Copy link
Contributor Author

shaggygi commented Feb 2, 2019

One thing that bothers me is the approach on how the GpioController is initialized and sets up the pins in all the bindings. Take the Mcp23xxx constructor for example.

public Mcp23xxx(int deviceAddress, int? reset = null, int? interruptA = null, int? interruptB = null)

When instantiating the object, we provide pin numbers (if the user wants to use). This will, internally, instantiate the GpioController with a GpioDriver (getting the best for platform). At this point, we can't provide a driver/pins if we wanted to use another.

I guess we could overload to do this and pass in the GpioDriver.

public Mcp23xxx(int deviceAddress, GpioDriver driver, int? reset = null, int? interruptA = null, int? interruptB = null)

// And then the controller gets created with selected driver.
_controller = new GpioController(PinNumberingScheme.Logical, driver);

// And then setup pins to inputs/outputs, default values, etc.

One problem with this is it forces you to use pins from only one controller/driver. As @RoySalisbury mentioned, you may want to mix and match pins from different types of controllers (dev board, binding, etc.). The only way I can see how to get around this is to use an abstract/interface (GpioPin/IGpioPin). It does appear that was the approach with Windows.Devices.Gpio.

// Can remove the GpioDriver in constructor as each pins knows what controller/driver it uses.
public Mcp23xxx(int deviceAddress, IGpioPin? reset = null, IGpioPin? interruptA = null, IGpioPin? interruptB = null)

This would also fix a limitation from the GpioPort proposal (#124) as it currently can only use one controller and pins because of similar issues.

// Current constructor proposal.
// public class GpioPort(GpioController controller, params int[] bitIndexes)
GpioPort port = new GpioPort(controller, 1, 2, 3, 4);
// Proposal option using IGpioPin.
// public class GpioPort(params IGpioPin[] bitIndexes)
GpioPort port = new GpioPort(IGpioPin one, IGpioPin two, IGpioPin three, IGpioPin four);

I know this completely changes the path we were headed. In addition, I'm sure the early design reviews discussed this and the pros/cons. Just throwing around ideas based on some of the issues we crossed while developing bindings. 🤔

@joperezr
Copy link
Member

joperezr commented Feb 8, 2019

I don't think we can consider this one done, in fact @JeremyKuhne just send out the PR to add IGpioController into the main library. In any case, I think we still want to keep this open as we also will eventually need a ISpiController, IPwmController and a II2cController for the same reasoning.

@joperezr joperezr reopened this Feb 8, 2019
@joperezr joperezr added enhancement New feature or request area-System.Device.Gpio Contains types for using general-purpose I/O (GPIO) pins labels Feb 8, 2019
@shaggygi
Copy link
Contributor Author

shaggygi commented Feb 9, 2019

What are some of the thoughts being pondered for II2cController and ISpiController?

@joperezr
Copy link
Member

Apparently just as there are devices such as the MCP Gpio extenders, there are also I2c bus extenders. According to @JeremyKuhne they work basically like a switch that you communicate to through I2c, which is connected to 3 virtual I2c buses. With these type of devices, you can have multiple devices of the same type (which would have the same I2c Address) on the same I2c bus since you have this extra layer of the expander in which you can switch and pick to which one you want to talk to. There seems to be similar types of devices for SPI as well.

@krwq
Copy link
Member

krwq commented Feb 22, 2019

This thread is very long and I'm not sure what the work is (currently bulk changing the milestone) - @JeremyKuhne is this something we plan to do for 3.0?

@joshfree joshfree added this to the Future milestone Feb 23, 2019
@joshfree joshfree added the api-suggestion Early API idea and discussion, it is NOT ready for implementation label Feb 23, 2019
@joperezr
Copy link
Member

The idea that this issue is tracking, is that just as @JeremyKuhne added the IGpioController interface, we want similar type of interfaces for our other protocols (I2c, Spi, and Pwm). I don't think that it is a feature that should block release, but it is definitely a nice-to-have, specially now that we are getting a lot of bindings that support those protocols added.

@joperezr joperezr removed the enhancement New feature or request label Apr 17, 2019
@shaggygi
Copy link
Contributor Author

shaggygi commented Jun 4, 2019

Referencing #456

@shaggygi
Copy link
Contributor Author

We now have IGpioController and IPwmController in the APIs. I'm going to go ahead and close this since we have other issues opened for other types. See the following for details.

@ghost ghost locked as resolved and limited conversation to collaborators Oct 15, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-System.Device.Gpio Contains types for using general-purpose I/O (GPIO) pins
Projects
None yet
Development

No branches or pull requests

6 participants