Yield condition is not updated in loop

[dlsloan]

Hello, I've just started using your library and after looking through the code a little I noticed the yield checks seem to be slightly off (either never called at all or called every itteration)

void CRC_X::add(const uint8_t * array, uint8_t length)
{
  _count += length;
  while (length--)
  {
    // reduce yield() calls
    if ((_count & 0xFF) == 0xFF) yield();
    _update(*array++);
  }
}

Should probably be:

void CRC_X::add(const uint8_t * array, uint8_t length)
{
  while (length--)
  {
   _count++;
    // reduce yield() calls
    if ((_count & 0xFF) == 0xFF) yield();
    _update(*array++);
  }
}

I have a slightly different solution in my fork (moved the check to add(value)) but here's my patch if you're interested: https://github.com/dlsloan/CRC/commit/9338e989b7640921d899a75e4cbfbd667e78f3cf

[RobTillaart]

Thanks for reporting, I will look into this later today. At first sight you are right,

I will look into your fork too, if it is a better solution may I include it in the library?

[dlsloan]

Feel free, I'm happy to contribute!

[RobTillaart]

Indeed a bug

Your solution looks cleaner as it allows to merge _update() into add() by the compiler. I keep them separated to keep the "RTOS yield()" code from the pure CRC code.

PR coming soon.

---

Question: should the static functions in CRC.h also have a yield() call?
The length of the array can be large and therefor the time can be many microseconds.

CRC-performance sketch indicates 122 bytes on 16MHz UNO takes 0.4 - 1 millisecond. (64 bit - 5 ms)) Blocks os SD cards and especially on ethernet can be much larger.

It can be done with a similar line

if (length & 0xFF == 0) yield();  // compare with zero is slightly faster.

[RobTillaart]

PR submitted,

• minor edits.
• increase length to 16 bits for static functions
• added CRC_polynomes.h to collect "well known" CRC's

so I included some "backlog"

[dlsloan]

Yield is a nice touch for long running functions, but it's nice to have an option with no yield in-case someone want's to run a crc calc from within a yield call (when not using an rtos that does context switching). In my fork I made it optional in the class, but I'll probably be changing that to something compile time in the future.

[RobTillaart]

made a new issue for this