make it possible to enable / disable yield()

[RobTillaart]

@dlsloan from #17

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]

On several (if not most) non RTOS platforms yield() is an empty function. Compilers see this and optimize them away, The if statement that point to only an empty statement can also be optimized away, so in the end no traces left of that call.

[RobTillaart]

@dlsloan

An idea, not exactly enable/disable yield, but make it configurable.

#ifndef CRC_YIELD_MASK
#define CRC_YIELD_MASK         0xFF
#endif
...
if ((count & CRC_YIELD_MASK) == 0)  yield();

The #ifdef makes it possible to overrule the value on commandline.

Furthermore one could change the value to e.g. 0xFFFFFFFF then 1 in 4 billion calls will do a yield(). That is effectively never.

---

maybe better (at least I think I prefer the following)

setYieldMask(uint32_t yieldMask) { _yieldMaks = yieldMask; };
getYieldMask() { return _yieldMask; };
...
if ((count & _yieldMask) == 0)  yield();

Then it can be controlled runtime.

Opinion?

---

Note: both versions allow to set the value to bit patterns that have strange effects e.g. 5 = 0b101 will call yield at every 5th and 7th iteration or so. It should be made cleat that the mask is typically best chosen as 2^n - 1

[dlsloan]

In Arduino yield is a weak function ref which can be used to allow background tasks to run durring wait and once per loop() call. In my case I'm doing some background communications which require crc calculations in the yield function which can't be allowed to yield it's self. My goto is to use templates for compiletime conditional's (so you can have multiple versions in the same project) but that's not a widely used C++ feature in arduino-land.

template<bool can_yield=true>
class crc__ {
  int add(uint8_t value) {
    if (can_yield && condition) yield();
    ...
  }
}

...
CRC32<> crc_yieldable;
CRC32<false> crc_noyield;

This results in the same code or no code for the yield in the same project but... I'm not sure if this is the way to go in a normal library since it's a rarely used feature which may confuse people. And requires moving the code into the header file because templates...

[dlsloan]

or more simply there could just be a separate function for no-yield, it'll be optimized out if not used and doesn't confuse anything

[RobTillaart]

As Arduino is as much for beginners as for professionals I try to keep the default behavior relative simple with some well chosen defaults. How about the can_yield option in the constructor, default to true?

[RobTillaart]

I will create a new develop branch then you can see if it matches your needs. (its now 20:27, so give me an hour or so)

[dlsloan]

No rush at all, your responsiveness has been amazing! I was planing to maintain my little fork for my work but it looks like everything will fit my needs soon.

[RobTillaart]

develop branch is pushed, build CI is running to see it all () match etc.

Having ~150 libs to maintain is quite some work, if I do not act within days things pile up quickly. So I need to stay as responsive as possible.

[RobTillaart]

As you are doing some CRC within yield() the #define and setYieldMask() will eventually fail. so that is no acceptable solution.

In the back of my head there is also this option

// POWER USER ONLY
void enableYield() { _canYield = true; };
void disableYield() { _canYield = false; };

It gives more runtime flexibility than the constructor parameter.

footprint in RAM is equal, in PROGMEM a few bytes more (expect)

[dlsloan]

Either solution works for me, in my case a crc object will never be reused in a way that I need to change it but an extra call to turn it off isn't a big deal either.

[RobTillaart]

Just translate your requirements to a more generic API, that makes it more/ broader usable.

I think I leave out the constructor option and stick to the enable/disable interface. default value = true and reset() should reset it to startup default = true again, just like all others.

BTW, which country are you from? (me Netherlands)

[dlsloan]

That sounds good to me, I wont be trying the new changes untill after work, I'm from western Canada (13:15 here). Thanks again!

[RobTillaart]

cleaned up the constructor, so now there is only the enable / disable call. todo

• document it

Need to think about the static functions as these still call yield()

• leave as is (null scenario)
• parameter is no option as there are already enough parameters.
• function to set some global flag is crappy
• #define could but is not flex

I'll leave it as is for now (gives most time to think)

[RobTillaart]

@dlsloan updated the documentation, please verify the develop branch if you have time. If there are no problems I will merge it tomorrow.

[dlsloan]

The develop branch it working well for me!