Hey, I think that’s a really great idea. One thing I would add is that we definitely should honor timeouts in any of the retries. Also, a way to disable the retries should be a good idea. Probably making it a syscfg which when set to 0 does not do any retries.
I am assuming the errors codes will be HAL_I2C_ERR_XXXX error codes, so all the mcus would have to return the same error codes on errors. Personally, I like calling them retries than tries since if they are set to 0 the operation still happens once, but that’s just me. Regards, Vipul Rahane > On Aug 30, 2018, at 9:48 AM, will sanfilippo <wi...@runtime.io> wrote: > > 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 >
