Improve scheduler #506
Improve scheduler #506
Conversation
This clarifies the time distribution in the stack, and simplifies testing.
This makes it easier to test in isolation, and cleans up the pnet_t struct
First step in investigating rtlabs-com#455
olbjo
left a comment
olbjo
left a comment
There was a problem hiding this comment.
I had a major comment on adding current time to the api. Think it though and comment on how you want to do it.
After that I can do some additional review of the scheduler internals
|
|
||
| /**************** Scheduler *******************************/ | ||
|
|
||
| volatile pf_scheduler_data_t scheduler_data; |
There was a problem hiding this comment.
I think the name should be "scheduler" adding a "_data" suffix does not add any value.
This is just an opinion.
| os_mutex_t * scheduler_timeout_mutex; | ||
| uint32_t scheduler_tick_interval; /* microseconds */ | ||
|
|
||
| /**************** Scheduler *******************************/ |
There was a problem hiding this comment.
I do not think adding "/***************" style comments improves readability.
Possibly to highlight a group of attributes.
(Opinion)
|
|
||
| /** Time between stack executions, in microseconds */ | ||
| uint32_t tick_interval; | ||
| } pf_scheduler_data_t; |
There was a problem hiding this comment.
Maybe pf_scheduler_t instead of pf_scheduler_data_t ?
| void pf_scheduler_init (pnet_t * net, uint32_t tick_interval); | ||
| void pf_scheduler_init ( | ||
| volatile pf_scheduler_data_t * scheduler_data, | ||
| uint32_t tick_interval); |
There was a problem hiding this comment.
I think a callback to get_current time should be registered at initialization. os_get_current_time_us() could be registered for normal execution and a mock for test.
This solution would mean less changes in the api and it reduces the complexity to the user of the API.
| scheduler_data->freelist_head = PF_MAX_TIMEOUTS; /* Nothing in queue. */ | ||
|
|
||
| if (net->scheduler_timeout_mutex == NULL) | ||
| if (scheduler_data->mutex == NULL) |
There was a problem hiding this comment.
Why this check? In the init function I expected a memset 0 of the scheduler instance and initialization and and creation of various data entries.
| scheduler_data, | ||
| ix_free, | ||
| scheduler_data->busylist_head)) | ||
|
|
| /** | ||
| * Check if timeout \a a is scheduled before timeout \a b. | ||
| * | ||
| * |
4 participants
No description provided.