Closed deviousasti closed 4 years ago
Would you mind sharing the use case? Why do you need access to private methods/variables and for what? Maybe they should not private for a good reason.
I’m writing a few modules deriving from Tasks. I need to iterate though the modules to init them, send signals, etc, and having access to the linked list pointers would avoid having to reimplement it, just because of accessibility modifiers.
Sent from Mail for Windows 10
From: Anatoli Arkhipenko Sent: Monday, August 5, 2019 10:46 PM To: arkhipenko/TaskScheduler Cc: Asti; Author Subject: Re: [arkhipenko/TaskScheduler] Add guard to _TASK_SCOPE, so it can beconfigured externally (#76)
Would you mind sharing the use case? Why do you need access to private methods/variables and for what? Maybe they should not private for a good reason. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or mute the thread.
I’m writing a few modules deriving from Tasks. I need to iterate though the modules to init them, send signals, etc, and having access to the linked list pointers would avoid having to reimplement it, just because of accessibility modifiers.
Is it not possible to init using OnEnable and let the scheduler deal with the sequence?
I need to call some methods on all registered modules, so I literally need the sequence.
Sent from Mail for Windows 10
From: Anatoli Arkhipenko Sent: Monday, August 5, 2019 11:57 PM To: arkhipenko/TaskScheduler Cc: Asti; Author Subject: Re: [arkhipenko/TaskScheduler] Add guard to _TASK_SCOPE, so it can beconfigured externally (#76)
I’m writing a few modules deriving from Tasks. I need to iterate though the modules to init them, send signals, etc, and having access to the linked list pointers would avoid having to reimplement it, just because of accessibility modifiers. Is it not possible to init using OnEnable and let the scheduler deal with the sequence? — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or mute the thread.
I'm having the same use case. I'm using the TaskScheduler to manage the derived tasks and would need to access the list.
A cleaner change would be to add functions to get the next/previous task instead of misusing _TASK_SCOPE and _TASK_DEBUG. This would open a read-only interface to the queue.
I've use the protected visibility to essentially implement those getters. The base class should implement them, but it was a much bigger change.
@arkhipenko Can you tell us of a way to take this forward?
@deviousasti Interesting that you mention protected visibility. The _TASK_SCOPE
can only be set to public
or private
in src/TaskSchedulerDeclarations.h
:
#ifdef _TASK_DEBUG
#define _TASK_SCOPE public
#else
#define _TASK_SCOPE private
#endif
@moritz89 Hence the pull request. :)
I guess a simple getter
method for previous and next tasks will be ok?
Let me get to that sometime soon.
Hi @arkhipenko: I think the getter would be perfect, if there also is a getter on the scheduler to be able to get the first task (otherwise you still can't iterate over all of them). Would imo only be insufficient on multi-task-systems (since task order might be changed while iterating). Thanks for the response btw!
Should be able to push something into testing this weekend
Please test v3.1.4 from the testing branch.
#define _TASK_EXPOSE_CHAIN
Task::Task& getPreviousTask()
Task::Task& getNextTask()
and
Scheduler::Task& getFirstTask()
Scheduler::Task& getLastTask()
Will check.
Why is it Task&
instead of Task*
?
If there is no next Task, you'll be de-referencing a null pointer.
Good point. Didn't think about that. Just followed a pattern of Scheduler's currentTask
method, which, come to think now should be a pointer as well.
Will fix.
Please re-test v3.1.4 from the testing branch. Just updated it
#define _TASK_EXPOSE_CHAIN
Task::Task* getPreviousTask()
Task::Task* getNextTask()
and
Scheduler::Task* getFirstTask()
Scheduler::Task* getLastTask()
Scheduler::Task& currentTask() // <== DEPRICATED (still supported) use
Scheduler::Task* getCurrentTask() // instead
Will check.
This looks fine.
I my version, I had getFirstTask
as begin
and getLastTask
as end
.
This was following the iterator pattern (std::begin
).
Do you feel like it's more meaningful to keep getFirstTask
and getLastTask
?
Unfortunately, I was also using iPreviousMillis
to calculate how long it's been since the task last ran, which I could still access using the protected
scope.
Hmm. I have a method that allows you to know time until next run already. Never thought it would be interesting to know how long since last run. I can add that as well.
This looks fine. I my version, I had
getFirstTask
asbegin
andgetLastTask
asend
. This was following the iterator pattern (std::begin
).Do you feel like it's more meaningful to keep
getFirstTask
andgetLastTask
?
Yes. I think begin
and end
is way too generic and usually are associated with a process (like serial comms), not an object (like the processing chain is).
Unfortunately, I was also using
iPreviousMillis
to calculate how long it's been since the task last ran, which I could still access using theprotected
scope.
iPreviousMillis
is not when the task has started. It's when it was supposed to be started. It could be quite misleading if you are in the catch-up situation actually.
Yes. I think
begin
andend
is way too generic and usually are associated with a process (like serial comms), not an object (like the processing chain is).
std::begin
is actually exactly what you're doing with first and last
https://en.cppreference.com/w/cpp/iterator/begin
I assumed that based on the comment:
volatile unsigned long iPreviousMillis; // previous invocation time (millis). Next invocation = iPreviousMillis + iInterval
Is there a correct way of calculating the time elapsed since a task last ran?
I assumed that based on the comment:
volatile unsigned long iPreviousMillis; // previous invocation time (millis). Next invocation = iPreviousMillis + iInterval
Is there a correct way of calculating the time elapsed since a task last ran?
Currently, the scheduler does not capture the actual time of a task's callback start. Each task can do that by saving millis()
value as the first line of entry. I never had a use case for this functionality, as everyone was more interested in the future time of task invocations, not the history of them.
Yes. I think
begin
andend
is way too generic and usually are associated with a process (like serial comms), not an object (like the processing chain is).
std::begin
is actually exactly what you're doing with first and last https://en.cppreference.com/w/cpp/iterator/begin
Interesting, but I am not following the std::
style anywhere else, so why do it in an isolated case?
(Not saying I should not follow it, but it just never happened).
Yes. I think
begin
andend
is way too generic and usually are associated with a process (like serial comms), not an object (like the processing chain is).
std::begin
is actually exactly what you're doing with first and last https://en.cppreference.com/w/cpp/iterator/beginInteresting, but I am not following the
std::
style anywhere else, so why do it in an isolated case? (Not saying I should not follow it, but it just never happened).
The proper way would be to create a real iterator class, then use begin
and end
and next
and previous
. I was not planning on doing it simply because of lack of a use case and library size.
If you'd like to contribute, perhaps we can have an iterator under the _TASK_EXPOSE_CHAIN
option.
I think your current implementation is good. New branch works great. Thank you for all your hard work!
Need access to internals when deriving from OO style task, so I'm defining _TASK_SCOPE as protected, but it would be redefined in TaskSchedulerDeclarations.h.
Adding a guard would be great.