search

gutierrezps / ESP32_I2C_Slave

I2C slave library for ESP32
GNU Lesser General Public License v2.1
81 stars 20 forks source link

Ensured non-blocking update() loop + changed onRequest calling policy #9

Closed FStefanni closed 3 years ago

FStefanni commented 3 years ago

Hi,

I am using this library for a project, which has this kind of behavior:

  1. The master performs a write, inserting into the packet a unique ID
  2. The master waits few time
  3. The master performs a read: if everything is fine, inside the read packet it should receive the last sent ID. This is just a sort of ACK to ensure the sent packet has been actually received by the slave

During testing, I have found these unexpected behaviors (from my humble point of view as user of this library).

1. Blocking behavior

When this library, inside the update() method, calls i2c_slave_read_buffer(..., 0), it sometimes blocks. I have not understood when this exactly happens, but I am sure that blocking is not fine: we are inside the Arduino loop() method, so we want to be asynchronous to improve performances.

Therefore I have changed the call into i2c_slave_read_buffer(..., 1) to ensure the call never blocks. The official ESP IDF API documentation is not clear what happens in case of zero (no wait? wait until there is at least something?), so this change seems safer and actually my code stopped to have a blocking behavior (so this change works).

2 Callback behavior

The user_onRequest method is always called, which is somehow unexpected. Actually, the official Wire lib documentation is not completely clear on the behavior, but it seems to me reasonable to have a strict separation between the two callbacks: user_onReceive called only for master writes, user_onRequest only for master reads. So I have slightly changed the if condition, and successfully obtained the desired behavior.

All these changes are provided into the pull request, along with the addition of a small sanity check. I provide this pull request in the hope you will find these changes useful (if not, feel free to discard this pr).

Regards.

gutierrezps commented 3 years ago

  1. I haven't faced this issue while using the library. But I think that setting ticks_to_wait to one doesn't change the behaviour, it's OK. ✔️

  2. The callback behaviour was somewhat expected and stated in README:

The only difference is that onRequest() is called whenever the master sends data, since there's no way to know when the master will request or is requesting data.

But you figured out a way to mitigate it: an empty packet can, in fact, be interpreted as the master requesting data. 🥇 ✔️

FStefanni commented 3 years ago

Hi,

thank you for accepting the pr.

Regards.