MCP23008 driver is limited to one device

[rwaldron]

The MCP23008 supports up to 8 devices on the same bus, by configuring hardware address pins, A0, A1 and A2:

  7-bit Slave Addr ->|
 B7 B6 B5 B4 B3 B2 B1|B0
  X  X  X  X A2 A1 A0|RW
  0  1  0  0  0  0  0| 0  = 0x20 (Default)
  0  1  0  0  0  0  1| 0  = 0x21
  0  1  0  0  0  1  0| 0  = 0x22
  0  1  0  0  0  1  1| 0  = 0x23
  0  1  0  0  1  0  0| 0  = 0x24
  0  1  0  0  1  0  1| 0  = 0x25
  0  1  0  0  1  1  0| 0  = 0x26
  0  1  0  0  1  1  1| 0  = 0x27

The present driver design only supports using 1 MCP23008 device at the default address.

Proposed Solution:

• Redesign the driver to be a class that produces instances (vs. the present class that just has static methods)
• Allow an explicitly provided address
• Fallback to default when no explicit address is provided

If there is interest in pursuing this solution, I would like to try my hand at implementing the upgrade.

[phoddie]

The current implementation of MCP23008 intentionally mimics the API of pins/digital. That is clearly too limiting for general use. Your proposed solution is perfect.

A good model for the MCP23008 is one of the Sharp I2C sensor drivers, like GP2AP01VT00F and qm1h0p0073, or the lis3dh accelerometer. All are subclasses of I2C, which provides a consistent way to set the I2C address (and other I2C parameters) using the dictionary passed to the constructor.

[rwaldron]

Thanks, I will take a look at those drivers first.

Another thing I spotted: there is presently no way to control the pullup state, which I can include in my solution here.

[phoddie]

Yup - configuring the individual pins (pull up, etc) is needed as well. Interested to see how you do that.

[rwaldron]

@phoddie design decisions time!

When driver objects are instantiated...

A. Should the chip's GPIO, GPPU and IODIR registers be reset to their "POR/RST" state?

• 👍 An always normalized state on initialization
• 👎 Not very useful if hardware application needs the state persisted (eg. relays)

B. Should the chip's GPIO, GPPU and IODIR registers be read into the memory for use by the instance?

• 👎 Not useful if user code wants to reset the hardware on program startup
• 👍 Useful if hardware application needs the state persisted (eg. relays)

C. Should the constructor accept an optional property in the dictionary arg that lets use code decide which of the above it prefers?

• 👍 Can satisfy either A or B, depending on user application/hardware needs.

My vote is for C.

[phoddie]

Interesting questions. We've tried to implement the modules for sensors so that simple uses stay simple (e.g. after the constructor runs, the sensors are ready to be used for common situations without additional configuration). A GPIO expander requires some configuration, since there's no obvious correct initial state. That means that the caller of the code needs to configure GPIO, GPPU, and IODIR on each pin it will use (unless it can safely make assumptions about their state based on the particular application running). Therefore, there's no need to reset the expander in the constructor.

I would try to avoid caching state about the expander in the instance to avoid synchronization problems should there be multiple instances. (Reset is also problematic if there are multiple instances)

Speaking of design, if you haven't already considered it, bulk read and write commands would be interesting they can be considerably faster.

[rwaldron]

We've tried to implement the modules for sensors so that simple uses stay simple (e.g. after the constructor runs, the sensors are ready to be used for common situations without additional configuration).

I plan to preserve that—no explicit address or reset needed for a "Just Works" case.

bulk read and write commands would be interesting they can be considerably faster.

Do you mean, for example: read or write the whole GPIO or GPPU at once? That's the way MCP23008 works by default

[phoddie]

Do you mean, for example: read or write the whole GPIO or GPPU at once? That's the way MCP23008 works by default

Yes, exactly. The current implementation only provides access to one pin at a time. That's simple for simple cases, but eliminates some uses of the expander.

[rwaldron]

While preparing to push a changeset that I've been working on, I pulled and rebased with the latest master, but now i2c is broken in some way that I don't understand.

/Users/rwaldron/clonez/moddable/modules/pins/i2c/i2c.c (156) # Break: (host): write failed!

The next call in the "Calls" pane shows this.write(register, value & 255); inside of SMBus.prototype.writeByte, which is being called from my own code.

Scanning the I2C bus confirms that the MCP23017 that I'm trying to write to is there:

I2C scanner. Scanning ...
Found address: 32 (0x20)
Done.
Found 1 device(s).

Also, this is printing out in my terminal, where it previously did not:

<?xs.87654321?>
<?xs.87654321?>
<?xs.87654321?>
<?xs.87654321?>
<?xs.87654321?>
<?xs.87654321?>
<?xs.87654321?>
<?xs.87654321?>
<?xs.87654321?>

Like I said, all of this only started when I rebased my branch with master

[phoddie]

The error "Break: (host): write failed!" means that the I2C protocol write failed. It is usually caused by a mismatch between the physical wiring and the sda/scl pins specified by the application. Since the scan works, it means the pins required in the scanner are correct.

The processing instructions are unrelated. They are part of an extension to the serial debugging protocol we're working on. We will clean those up.

[rwaldron]

lol @ me: I figured it out. My code was still using "clock" 🤦‍♀️ (yes, as in the same patch I wrote myself)

[phoddie]

Ha.

[phoddie]

The driver rewrite by @rwaldron committed allows the I2C address to be set by a script, resolving this issue.