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

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

Open
wants to merge 4 commits into
base: dev
Choose a base branch
from

Conversation

conejoninja
Copy link
Member

No description provided.

return i
}
}
return 99
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

99?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wasn't sure what value return ... that should be in case no port is enable. 0 is the first port and can not be used. Any value >= 16 is an invalid port and could be used.

Other idea is to return (port byte, err error) but again, a value must be used when error 🤔

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any value >= 16 is an invalid port and could be used.

In that case how about:

const InvalidPort = 0xff

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added, thanks

@soypat
Copy link
Contributor

soypat commented Jan 27, 2024

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
Copy link
Member Author

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
Copy link
Contributor

soypat commented Jan 27, 2024

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
Copy link
Member Author

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
Copy link
Contributor

soypat commented Feb 13, 2024

@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!

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.

None yet

3 participants