> On Aug 30, 2018, at 7:21 PM, 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.
Sounds good. Extending the struct seems like the natural place to do it.
I don’t think we should worry about binary compatibility.
> [1] We should do the same for the other HAL APIs (i.e., non-I2C), but
> that can come later.
Not sure this makes sense for other ones, as i2c is the only one with
ACK.
>
> [2] I prefer specifying number of tries rather than number of retries.
> I think it helps avoid some ambiguity.
Seems fine to me.
>
> All comments welcome.
>
> Thanks,
> Chris
