Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: implement trigger timeout on aperiodic tasks #13

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

doudou
Copy link
Member

@doudou doudou commented Nov 4, 2024

A.k.a. "port driven with timeout"

Already present in all other OSes. Unfortunately, not great on
gnulinux as the clock used for timing out is the realtime clock.
This allows to specify a timeout on aperiodic activities, which
is how long the activity will wait for a trigger before being called
anyways. This is meant to implement timeout mechanisms in case
no trigger ever comes (e.g. on port-driven tasks)
@pierrewillenbrockdfki
Copy link

This basically allows to run setTimeout_us on a FileDescriptorActivity, explicitly only allows a 0 setting for PeriodicActivity, SequentialActivity and SlaveActivity(and does nothing else), and for the other Activities(and FileDescriptorActivity), when the associated thread is not active, running or periodic, allows for aperiodic wakeups calling the Activities loop() or step() implementation.

This all makes sense, even though the code base is confusingly using a variable named task for what is a Thread or, at best an Activity, not related to the TaskCore and its similar named methods.

The sem_timedwait being tied to CLOCK_REALTIME is just sad, but it is better than nothing.

Thanks to this, i learned that i was rather wrong about PeriodicActivity accepting ::trigger from watched ports...

clock_gettime(CLOCK_REALTIME, &ts);
long long delay_s = delay / 1000000000;

ts.tv_nsec += delay - delay_s * 1000000000;;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
ts.tv_nsec += delay - delay_s * 1000000000;;
ts.tv_nsec += delay - delay_s * 1000000000;

ts.tv_nsec += delay - delay_s * 1000000000;;
long sec = ts.tv_nsec / 1000000000;
ts.tv_sec += delay_s + sec;
ts.tv_nsec -= sec * 1000000000;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did not get this ts.tv_nsec -= sec * 1000000000;

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is essentially ts.tv_nsec = ts.tv_nsec % 1000000000;. I'm not sure why I wrote it this way except to be confusing.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sometimes, this is needed to keep values positive? Iirc, the / and % pair "rounds" towards 0.

@doudou
Copy link
Member Author

doudou commented Nov 12, 2024

This basically allows to run setTimeout_us on a FileDescriptorActivity, explicitly only allows a 0 setting for PeriodicActivity, SequentialActivity and SlaveActivity(and does nothing else), and for the other Activities(and FileDescriptorActivity), when the associated thread is not active, running or periodic, allows for aperiodic wakeups calling the Activities loop() or step() implementation.

On FileDescriptorActivity, it makes a common API for an API that already existed.

For Activity when it is not periodic (i.e. port-driven), it allows to get a time-based wakeup.

For the others, it keeps their properties, which is unthreaded for SequentialActivity and SlaveActivity (which means we can't wake them up), and purely periodic for PeriodicActivity and Activity with a period.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants