Sorry, just read Will’s comment. Did not want to repeat it.
- Vipul > On Aug 30, 2018, at 11:35 AM, Vipul Rahane <vi...@runtime.io> wrote: > > 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 >> >
