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

Only with interrupts? #2

Open
davidedelvento opened this issue Jan 8, 2022 · 8 comments
Open

Only with interrupts? #2

davidedelvento opened this issue Jan 8, 2022 · 8 comments
Assignees
Labels
enhancement New feature or request

Comments

@davidedelvento
Copy link

Thanks for providing this example, in fact the Raspberry Pi Pico C/C++ SDK is really incomplete in this regard!!!

Are interrupts like your library does really the only way to implement I2C slave on the Pico?

I need to do something like the following on my pico slave

while(true) {
    prepare some stuff
    write some_stuff to the i2c bus # ok to block
}

Of course I could change that code to use interrupt, but it becomes unnecessary more complicated, so I tried using i2c_write_blocking() and i2c_write_raw_blocking() but those do not seem to be working. Is it really so, or am I doing anything wrong?

@vmilea
Copy link
Owner

vmilea commented Jan 9, 2022

This sort of loop works well on the master side.

The slave on the other hand is always reacting to requests from master, so an event driven API fits better. pico_i2c_slave delivers these events via callbacks straight from the I2C interrupt handler to minimize delay. This is the common approach also used in Arduino with Wiring.

Now, instead of callbacks, you could have a loop which polls IC_RAW_INTR_STAT and writes like you are proposing. But then the bus is stalled during during "prepare some stuff" phase, plus it's unclear how the slave would recover in case of transmission error. A more solid implementation would require some buffering in the library.

tl;dr There's room for a more convenient interface, if enough people complain.

@vmilea vmilea self-assigned this Jan 9, 2022
@vmilea vmilea added the enhancement New feature or request label Jan 9, 2022
@davidedelvento
Copy link
Author

Thanks.

Perhaps it was already clear, but to make it crystal clear, the problem with the interrupt approach for my code, is that the request may arrive before the data to be sent is ready: so if there is nothing ready to send when the callback happens, then what? I don't really want to do the data preparation in the callback...

Not sure what do you mean by "the bus is stalled". Could it be used by other devices? Or are you implying it can't? But in any case, during the "prepare some stuff", there is nothing to send yet so either the master has to wait (if it does not have anything else to do) or it could do something else, if it posted a non-blocking read which timed out.

@vmilea
Copy link
Owner

vmilea commented Jan 9, 2022

I don't really want to do the data preparation in the callback...

It depends on what you are trying to accomplish. Here's a couple of options:

  • Prepare the data in the callback in small chunks. The maximum amount to copy at a time is 16 bytes (Tx FIFO capacity). If each chunk is processed quickly, this may be good enough.
  • Produce the data in your main loop, and consume it in the callback. If there's nothing ready to send yet, simply return from the callback and it will be invoked again. For producer-consumer scheme there's buffering and synchronization to deal with.

Not sure what do you mean by "the bus is stalled".

After a read request, the slave holds SCL low until there's something to transmit. It's stalling until you put something in Tx FIFO, and no other device can use the bus.

@davidedelvento
Copy link
Author

Prepare the data in the callback in small chunks. The maximum amount to copy at a time is 16 bytes (Tx FIFO capacity). If each chunk is processed quickly, this may be good enough.

That is not really a good option for this: the data is coming from the ADC (and perhaps via DMA, tbd), so the timing is out of control.

Produce the data in your main loop, and consume it in the callback. For producer-consumer scheme there's buffering and synchronization to deal with.

That's what I want to avoid.

After a read request, the slave holds SCL low until there's something to transmit. It's stalling until you put something in Tx FIFO, and no other device can use the bus.

Thanks for clarifying that... it might be that the best option is:

If there's nothing ready to send yet, simply return from the callback and it will be invoked again.

Would the bus be released in such a case? What would the master see?

Thanks a lot, the conversation in this issue is much more useful than the official documentation for this use case!!!

@vmilea
Copy link
Owner

vmilea commented Jan 9, 2022

Would the bus be released in such a case? What would the master see?

No, the slave keeps stalling and asserting RD_REQ until there's something to transmit.

the conversation in this issue is much more useful than the official documentation

The I2C chapter in RP2040 datasheet covers most of this in detail. "[4.3.10.1] Slave Mode Operation" is definitely worth reading.

@davidedelvento
Copy link
Author

The I2C chapter in RP2040 datasheet covers most of this in detail. "[4.3.10.1] Slave Mode Operation" is definitely worth reading.

Indeed. I hoped to be spared from that by using the SDK instead (so I could concentrate on the problem at hand, rather than solving low level communication issues).

Anyway, I reported the issue at raspberrypi/pico-sdk#708 and mentioned there your very useful repo. If you have time, you should make a PR with this repo as a subdirectory of https://github.com/raspberrypi/pico-examples/tree/master/i2c -- I'm sure that will save poor people like myself lots of headaches for banging the head too much on the keyboard....

Thanks again, and you may close this issue, as far as I am concerned.

@vmilea
Copy link
Owner

vmilea commented Jan 19, 2022

After giving it more thought, here is a minimal example of slave-transmitter in a loop.

See i2c_slave_transmit_blocking() which waits for RD_REQ before doing the blocking write. This is easier to use than the callback approach, with the caveat of stalling the I2C bus until the slave is ready to transmit. Hopefully it clears up some confusion around i2c_write_raw_blocking() as well.

Does it solve your issues?

@davidedelvento
Copy link
Author

Does it solve your issues?

That works great, and it is very clear (I know all the stuff about critical sections and the likes from other contexts -- to possible other readers of this issue: do some research about critical sections, unspecific to the pico).

Thanks a lot! You are the best!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants