arduino / arduino/ArduinoCore-avr

Wire Slave: arbitrary user code is called from ISR

Open
#342 2 comments 0 reactions 0 assignees View on GitHub
Dominant language
C
Stars
1.5k
Forks
1.1k
PR merge metrics
No merged PRs in 30d

Description

When a user is using the Wire library in slave mode, they generally need to register a function to handle requests for data from the master. They write some function, which must contain `Wire.write` and register it like this: `Wire.onRequest(someFunction);`.

The issue here is that `someFunction` gets called (via `twi_onSlaveTransmit`) right in the slave's interrupt service routine: https://github.com/arduino/ArduinoCore-avr/blob/58081c05e548560d3a60050bbf260ebec5d1e867/libraries/Wire/src/utility/twi.c#L528

First of all, the documentation for `Wire.onRequest` [is truly tragic](https://www.arduino.cc/en/Reference/WireOnRequest). It doesn't even tell the user they must make a call to `Wire.write` or else [the wire library will decide for them](https://github.com/arduino/ArduinoCore-avr/blob/master/libraries/Wire/src/utility/twi.c#L530-L533) to send `0x00` to the master, and it doesn't warn the user that they're writing code for inside an ISR.

Secondly, this architecture raises all kinds of potential usability issues. I think quite often a slave will need to do something much more complicated to answer a master's request than what should/can be done in an ISR. As I understand the code (please someone correct me if I'm wrong), it's impossible to answer the master outside of the call from the ISR. What if the slave needs the `delay()` function in forming its answer for the master? Impossible. What if the slave needs to fetch an answer for the master over SPI? *Maybe* that's possible, but it sure seems like a terrible idea to do that from inside an ISR.

I noticed a stub:

https://github.com/arduino/ArduinoCore-avr/blob/58081c05e548560d3a60050bbf260ebec5d1e867/libraries/Wire/src/Wire.cpp#L271-L274

Maybe the point of that was to help solve this problem?

Would you consider accepting a PR I might make so that a slave can form an answer to a master outside of the ISR?

Contributor guide

No contributing guide indexed for this repository

Research direction

Start with libraries/Wire/src/utility/twi.c at twi_onSlaveTransmit and libraries/Wire/src/Wire.cpp around the cited stub. Trace how Wire.onRequest invokes user code and how the fallback 0x00 response is selected. A complete change would define and implement a supported response path outside the ISR, with corresponding Wire.onRequest documentation and validation of ISR-safe behavior.

Written by the indexing model from the issue text.

Assessment

Tech stack
c, cpp
Domain
embedded-iot
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.