tinygo-org / drivers

TinyGo drivers for sensors, displays, wireless adaptors, and other devices that use I2C, SPI, GPIO, ADC, and UART interfaces.
https://tinygo.org
BSD 3-Clause "New" or "Revised" License
584 stars 180 forks source link

Initial support for PCA9548A 8-channel I2C-bus #644

Open conejoninja opened 5 months ago

soypat commented 5 months ago

I'm not quite agreeing with the abstraction. I'd suggest a I2C factory pattern for each port.

Here's what I'd consider an ideal API:

type Device struct {...}

type Port struct {
   dev *Device
   portmask uint8
}

func New(drivers.I2C, addr uint8) (*Device)

func (*Device) GetPort(selectedChannels ...uint8) (Port, error)

func (Port) Tx(addr uint16, w, r []byte) (err error)

By choosing this API you can now use each port as a drivers.I2C which simplifies things a lot. Of course you now have to do some bookeeping to make sure the Port is acquired by one device at a time and that the port is selected.

conejoninja commented 5 months ago

I'm not sure what to think about the proposed interface by @soypat . I had a few examples and didn't use GetPort() maybe that's the reason why I'm having some trouble understanding it. How its usage would looks like?

soypat commented 5 months ago

sure! Here's what I've got in mind!

onChipI2C := machine.I2C0
// pca9548.Addr would also come handy as a package level function!
mux := pca9548.New(onChipI2C, pca9548.Addr(false, true, true))

// Here's the common usage of Device.GetPort. We generate a I2C bus that 
//can be used just like the on-chip I2C given by the machine package.
// This abstraction is especially powerful because you can use most drivers in the tinygo/drivers
// package with a pca9548.Port since it implements drivers.I2C.
// The example below is how you'd use the package to have multiple MPU6050's on the same I2C bus
// even if the devices share the same address!
port4 := mux.GetPort(4)
mpu1 := mpu6050.New(port4, mpu6050.DefaultAddr)
port5 := mux.GetPort(5)
mpu2 := mpu6050.New(port5, mpu6050.DefaultAddr)

// This group will send I2C over channels 0,1,2 and 3 on the PCA9548.
// I've actually never seen this done in practice, but apparently the device allows
// for grouping any combination of the channels so that a message is sent on all of them.
portGroup := mux.GetPort(0, 1, 2, 3)
devMulti := MultiI2CDevice(portGroup, DefaultMultiAddr)
conejoninja commented 4 months ago

I've been thinking about it, the only way I managed to see it working, is that if in func (Port) Tx(addr uint16, w, r []byte) (err error) the port is set/enabled each time. That will impact negatively (more txs) if one device is used twice (or more time) in a row (some devices need to trigger a reading and read at a later time), or if one of your devices is used more than others. I'm also not sure if there's a lot of need for multiple-devices communications (this can be achieved if you set the ports to whatever you want to and send the message, for example

mux.SetPortState(whatever)
mpu6050.Configure() // this will send the configuration msg to every selected port, configuring them at the same time.

What do you think? is it worth it?

soypat commented 4 months ago

@conejoninja

the only way I managed to see it working, is that if in func (Port) Tx(addr uint16, w, r []byte) (err error) the port is set/enabled each time

Not necessarily. Here:

type Port struct {
    dev *Device
    portmask uint8 
}

func (p Port) Tx(addr uint16, w,r []byte) error {
    // Mutex protects exported functions that use the bus.
    p.dev.mutex.Lock()
    defer p.dev.mutex.Unlock()
    p.acquire()
    return p.dev.bus.Tx(addr, w, r)
}

func (p Port) acquire() {
    if  p.portmask == p.dev.portmask {
          return // Port already acquired.
    } else if p.dev.optionLazyPortSwitch && p.portmask&p.dev.portmask == p.portmask {
         // If users want to further reduce the I/O and their device permits it
         // we can provide an option to only switch port if the mask is not already enabled,
         // which can let users build interesting I2C network topologies in such a way to reduce I/O even further.
         return 
    }
    p.dev.setport(p.portmask) // Here we do the I/O to set the portmask.
}

This way you have no additional I/O over the bus and users do not have to worry about selecting the port before using it, it all happens automatically which is really convenient if you ask me. What's more, in the best case this method would do less I/O than manually selecting ports since it would only select ports when strictly necessary!