PeriodicTaskNotifier does not support different intervals per task

[StefanoSperetta]

It would be great to support different intervals per task but this would require defining a granularity for SysTick and finding a way to let the system sleep and still handle the ticks.

Not a very high priority right now but it might be important for the OBC.

[CasperBroekhuizen]

Updated in commit https://github.com/DelfiSpace/DelfiPQcore/commit/df130fbe5e857016c63e545fa1528712ceabf655 on a seperate branch.

Task notifier takes in a list of PeriodicTasks that have a defined 'taskCount'. The taskPeriod is this taskCount multiplied by 0.1s. The taskNotifier uses sysTick (related to https://github.com/DelfiSpace/EPS_FlightSoftware/issues/9). Following is an example of two tasks running at a different period and printing:

Another possible feature of this object, could be that tasks can use it to notify themselves in a more one-time scenario. e.g. a task enabled a certain action and asks the notifier to wake it up in 10seconds. @StefanoSperetta, what is your opinion on this?

[StefanoSperetta]

I forgot to comment in GitHub but I fully agree ;-)

One of the things I noticed is that there is a #define TASKNOTIFIER_PERIOD that hardcore the clock frequency to 48MHz. This is also defined somewhere else (not sure right now). We might want to keep the definition of the clock frequency in one place only or recall that define here.

Is there any chance we might want to have a customizable count resolution (0.1s)? Not sure this is really worth the effort...

I see the PeriodicTaskNotifier is already in use, after discussing these details we might close the issue.

[CasperBroekhuizen]

I see your point.

The TASKNOTIFIER_PERIOD is now TASKNOTIFIER_PERIOD_MS, which uses FCLOCK to get the count. They have to be defines in order to static_assert a 24bit period value.

Using this define, the periodic tasks are now set in ms and the period can be adjusted if necessary. https://github.com/DelfiSpace/DelfiPQcore/commit/99a5b8cf90783967029756b8617da6b7f77e701c

[StefanoSperetta]

How do we deal with rounding? if the period is 99ms, the division will give 0... I know I am annoying about this ;-)

[CasperBroekhuizen]

Correct, the 'count' will always be rounded as it is an integer, having a period smaller then the resolution will round to 0, meaning the task will be notified every notification cycle.

Meaning that the smallest possible period is constraint by the resolution.

[StefanoSperetta]

I see you just committed a change (523e4ec): what would be the difference between setting the period to 99 and 199 (basically between 0 and 199)? I guess none, right? It should always take 1 iteration...

I am not sure if it would be best to use a round() to limit the error... but it could also give slightly non-intuitive results...

[CasperBroekhuizen]

Correct, as the division will always round down and a period of 0 being impossible. meaning that both 99ms (rounded to 0) and 199ms (rounded to 1) will have a period of (100ms) 1 count.

We can also implement a better 'rounding' by looking at the remainder of the period using the modulus operand. However this will still cause that any period of 0-149ms will be rounded to 100ms. As sub-resolution notification is not possible in this setup.

Is this necessary however? Are the task periods going to be that precise? if so we can also increase the resolution.

[StefanoSperetta]

I have been thinking about it quite a lot and I started to like the "usual" implementation of the delay function in a regular OS that guarantees the delay never be shorter than the specified interval. This is mainly to avoid surprises.

In our case I would suggest having the following cases: request -> delay 0 -> 100ms 100 -> 100ms 101 -> 200ms 200-> 200ms

This could be done if we use the ceil() function or we simply do count = (a / b) + ((a % b) != 0);

What do you think?

As usual, my greatest worry is that somebody with limited experience screws up the code, so I am maybe overcautious ;-)

[CasperBroekhuizen]

Great! implemented in https://github.com/DelfiSpace/DelfiPQcore/commit/39a53a4c2f6772f1c90e869b3a48845774226590