adamboardman / pico-mcp23017

Raspberry Pi Pico MCP23017 16-Bit I/O Expander with I2C Serial Interface
Apache License 2.0
17 stars 8 forks source link

Read input data without interrupts? #1

Closed MathiasYde closed 2 years ago

MathiasYde commented 2 years ago

How can you read input data from the port expander without using interrupts as I want to read data continuously. This is an overview of the code I currently have.

const uint8_t MCP_ADDRESS= 0x20;

const uint8_t PIN_I2C_SCK = 20;
const uint8_t PIN_I2C_SDA = 21;

Mcp23017 mcp(i2c0, MCP_ADDRESS);

int main() {
    i2c_init(i2c0, 400000);
    gpio_set_function(PIN_I2C_SCK, GPIO_FUNC_I2C);
    gpio_set_function(PIN_I2C_SDA, GPIO_FUNC_I2C);
    gpio_pull_up(PIN_I2C_SCK);
    gpio_pull_up(PIN_I2C_SDA);

    mcp.setup(true, false);
    mcp..set_io_direction(0xffff);

    while (true) {
        uint16_t value;
        for (int i = 0; i < 16; i++) {
            value = value << mcp.get_input_pin_value(i);
        }
        printf("%d\n", value);
        sleep_ms(10);
    }

    return 0;
}
adamboardman commented 2 years ago

The best way to read continuously is with using interrupts, if you do as your doing, generally called polling its both energy inefficient (your doing work when nothing has happened) and you can also miss pulses shorter than your polling frequency. It does require a different way of thinking about your problem resulting in a different architecture though I cannot think of a use case where polling would be better than interrupts when they are available.

If you want to describe your project use case in more detail and request advice I'd suggest posting something on the forum as a better option than raising a bug against a library. When I was active on this project I used the RPiPico SDK forum, though its been a while since I last visited it.

MathiasYde commented 2 years ago

Thank you for your fast reply. About your concern about energy efficiency, I activate an IC before the poll, meaning I do need to capture the state of the bits everytime (as I can risk no change of state that an interrupt wouldn't catch).

I would assume I need to flush the inputs into the internal register of the port expander before retrieving the bits, but I don't see how I could achieve this.

And my apologies, I created an issue here as I only have a problem using your library in this small situation and I didn't think it would be much concern to the general forums.

adamboardman commented 2 years ago

Its been a while since I worked on this, but so far as I remember reading values from inputs is far more reliable when using interrupts over polling. Polling is saying 'what's your value' at a time interval (sleep_ms), if the value changes twice eg on and off within that time interval you'll miss the change. But if you use interrupts you'll at least get one of the changes flagged and if coded with a very small interrupt handler with your main loop waiting on an interrupt change (wfe) you'll get all of them.

If you don't believe me read up on the topics (Interrupt vs Polling IO). Just remember if your using printf's for debugging output that will effect the timing and reliability of capture.

If your wanting to get the initial values on startup then you just need to call update_input_values to get all the values which can then be queried per io line with get_input_pin_value which just does a bit check against the values read by the update call which gives all the values anyway. You mention flushing, which is just for output values.

As I say without a properly described use case its impossible to advise you on what is best to do.

MathiasYde commented 2 years ago

My use case for the MCP23017 is connecting them to a 6502 CPU, same as the one in the Apple II, NES and Commodore 64. I use two of these port expanders, one for the 16 bit address bus, and another for data and flag values. Right after the rising edge of the clock (pulse created from the Pico), the CPU will perform an instruction. From that, I'll need to read the current state of the address register. Due to this, I need to capture the values as of now, rather than when they will change.

adamboardman commented 2 years ago

Interesting use case, no idea why you'd be wanting to do that. You could still make use of interrupts to update the cached value of the inputs and then just read that with your clock frequency if you so desired. Either way that's not the reason why your code is failing.

I would encourage you to use an IDE like CLion where you can jump to definition/implementation with a modifier key and a mouse click. Which would easily take you to either the source or the headers.

For example see the header definitions:

/**
 * Stores the last input state in the class for later interrogation with get_input_pin_value
 * @return PICO_ERROR_NONE or PICO_ERROR_GENERIC
 */
int update_input_values();

/**
 * Checks the pin state from the last update_input_values
 * @param pin the pin to query
 * @return the pin's state
 */
bool get_input_pin_value(int pin) const;

As you can see in your code your repeatedly calling get_input_pin_value which is only reading a bit value from a cached value that your never updating.

Given your actual use case you might want to add another method to return the relevant 8 bits of the last_input in a single call rather than having to read it bit by bit and reassemble. Or a live reading call that directly reads the GPIOA or GPIOB values from the MCP23017 rather than caching it.

MathiasYde commented 2 years ago

I've managed to make it work. I am able to retrieve the value on the address bus from the 6502 CPU and it's counting in binary which is the expected output, although my wiring is a bit weird so I have to split the value in two and swap the order, and OR them together after. Though I am sure this way is less reliable and therefore slower, it works as of now. My motivation for this project is to be able to run machine code on it, and hopefully be able to emulate systems, such as the NES, where the CPU instructions are performed by the actual, physical chip.