I think my only comment is tries vs retries. You always want to make at least one try so you would have to set that to 1 every time right? I like retries personally but that is just me. Mainly because you would never allow zero for tries.
> On Aug 30, 2018, at 9:21 AM, Christopher Collins <ch...@runtime.io> wrote: > > On Wed, Aug 29, 2018 at 10:06:37PM -0500, Greg Stein wrote: >> On Wed, Aug 29, 2018 at 8:59 PM Christopher Collins <ch...@runtime.io> >> wrote: >> >>> Hello all, >>> >>> I noticed the HAL master I2C API does not include any retry logic. As >>> you probably know, in I2C, the default state of the SCL line is NACK. >>> >> >> Euh... ACK/NACK is on SDA while the master cycles SCL. > > Yes, you are right. Thanks for the correction. > > [...] > >> Or that something monkeyed the bus, so the peripheral decided to stop >> receiving/processing. ... Your basic point holds: a peripheral might just >> go away for any number of reasons. I wouldn't even bother with the on_dnack >> flag. Seems better to consider retry at the whole-packet level. It all gets >> sent, or it did not and should be retried. > > Thank you for the insight. I am starting to think you are right - > retries should be done by the low-level HAL implementation. So, my > current thinking is that we should make the following changes: > > 1. Define a common set of error code for the I2C HAL [1]. > 2. Add a single member to the `hal_i2c_master_data` struct: `tries` [2] > 3. Modify the HAL I2C implementations so that they retry up to (tries-1) > times when an unexpected NACK is recieved. > > [1] We should do the same for the other HAL APIs (i.e., non-I2C), but > that can come later. > > [2] I prefer specifying number of tries rather than number of retries. > I think it helps avoid some ambiguity. > > All comments welcome. > > Thanks, > Chris