Extend IRunnable::run with a deadline

[pavel-kirienko]

See #363 (comment)

This should be a deadline rather than a timeout. The clock used for the deadline tracking is described in https://github.com/OpenCyphal-Garage/libcyphal/blob/main/docs/design/readme.md#time-primitives

[serges147]

What if introduce ICancellable (or IInterruptable TBD) with bool isCancelled() method, pass this interface (optionally?) to whatever run as MaybeError run(const TimePoint now, ICancellable* cancellation_token), and whatever loop we have in libcyphal currently is supposed to periodically check (on each iteration, or maybe even in between of some breakable sub-tasks of a single iteration, or even pass it to a nested internal runXxx methods in necessary) ? User implementation of such ICancellable might implement whatever logic she wants, f.e. keep track of currently allowed time slot, and when it's gone start returning true - and whole run should quickly unwind. With this approach we don't need to bring the clock entity into lybcyphal, and so don't need compare time by ourselves. Also, it allows user to interrupt a run safely and early by maybe some other than time condition...

[pavel-kirienko]

Makes sense. But are we sure we need an interface for this instead of a simple cetl::function<bool() noexcept, 4*sizeof(void)>?

[serges147]

IMHO both will work, I just think that interface pointer might be more light way... In both cases we still need to first check for "emptiness" (either != nullptr or bool operator of function) before calling isCancelled, like:

while (/*some currently existing loop condition*/)
{
    if ((cancellation_token != nullptr) && cancellation_token->isCancelled())
    {
        return cetl::nullopt; // or `break;` if it suits better for some cases
    }
    ...
    runXxx(cancellation_token); // b/c might have nested "loop" inside
    ...
}

UNLESS!!! We will require non-empty token (either by passing ICancellable&, or require function to be pre-assigned with some functor like dummy []() { return false; }). Then the above code will be a bit simpler and with less branches to cover.

@pavel-kirienko What benefits do you think will be in using function here? Thnx

[pavel-kirienko]

What benefits do you think will be in using function here?

[thirtytwobits]

A cancellable assumes multiple threads of execution. We should avoid requiring this.

[serges147]

A cancellable assumes multiple threads of execution.

This is not what I meant. Here is an example of possible user-provided implementation of ICancellable:

bool TimeslotCancellable::isCancelled()
{
    return ::user_system_clock() > deadline_;
}

And somewhere else user calls our run like this in her super-loop:

TimeslotCancellable timeslot;
while (true)
{
    const TimePoint now = ::user_system_clock();
    timeslot.setDeadline(now + 20ms);
    
    auto maybe_error = libcyphal.run(now, &timeslot);

    // do some other user-specific stuff
    ...
} 

OR like this if we go with function:

...
    const TimePoint deadline = user_system_clock() + 20ms;
    auto maybe_error = libcyphal.run(now, [deadline]() { return ::user_system_clock() > deadline; });
...

[pavel-kirienko]

@thirtytwobits Are you saying we need a different name?

[thirtytwobits]

@thirtytwobits Are you saying we need a different name?

nope. I like what Serge is proposing.

[serges147]

@pavel-kirienko I believe this issue is not applicable anymore.