dotnet / iot

This repo includes .NET Core implementations for various IoT boards, chips, displays and PCBs.
MIT License
2.17k stars 584 forks source link

[Proposal] Provide controller provider interfaces #125

Closed shaggygi closed 5 years ago

shaggygi commented 5 years ago

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.
}
joperezr commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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.

RoySalisbury commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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.

shaggygi commented 5 years ago

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

joperezr commented 5 years ago

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 commented 5 years ago

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?

joperezr commented 5 years ago

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.

shaggygi commented 5 years ago

Referencing #456

shaggygi commented 5 years ago

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.