Task IF refactoring #636

muellerr · 2022-05-30T10:48:18+02:00

muellerr commented

2022-05-30 10:48:18 +02:00

These changes were tested on all OSALs. The FreeRTOS and RTEMS updates wered tested in the FSFW example for the STM32H7. The Linux changes were tested in the EIVE fork.
The hosted changes were tested in the hosted FSFW example.

Also, some other general improvements/notes:

There seems to be a concensus that virtual functions should not have default arguments. See https://google.github.io/styleguide/cppguide.html#Default_Arguments

The defaults for arguments in a virtual function call are determined by the static type of the target object, and there's no guarantee that all overrides of a given function declare the same defaults.

and this stack overflow post https://www.google.com/search?channel=fs&client=ubuntu&q=default+arguments+on+virtual+or+override+methods+are+prohibited .

Also, clang-tidy recommends adding [[nodiscard]] to all constant getters.
For me, this seems to be a reasonable action. It can prevent missed returnvalue checks and useless getter function calls. If someone decides to actively ignore a returnvalue, they can explicitely cast the function call to void to avoid the warning.

Also, it might be a good idea to compile the FreeRTOS and RTEMS OSALs in the CI/CD as well. It should be possible to create a docker container which has all necessary tools pre-installed.

Refactoring general task code

There was a lot of duplicate/boilerplate code inside the individual task IF OSAL implementations. Remove it by introducing base classes PeriodicTaskBase and FixedTimeslotTaskBase.
A lot of clang-tidy improvements

Refactor PeriodicTaskIF

Convert virtual ReturnValue_t addComponent(object_id_t object) to virtual ReturnValue_t addComponent(object_id_t object, uint8_t opCode = 0), allowing to pass the operation code passed to performOperation. Update API taking an ExecutableObjectIF accordingly

Refactor FixedTimeslotTaskIF

Add additional addSlot function which takes an ExecutableObjectIF pointer and its Object ID

Refactor FixedSequenceSlot

Introduce typedef CustomCheckFunc for ReturnValue_t (*customCheckFunction)(const SlotList&).
Convert ReturnValue_t (*customCheckFunction)(const SlotList&) to ReturnValue_t (*customCheckFunction)(const SlotList&, void*), allowing arbitrary user arguments
for the custom checker

Linux OSAL

Use composition instead of inheritance for the PeriodicPosixTask and make the PosixTask a member of the class

These changes were tested on all OSALs. The FreeRTOS and RTEMS updates wered tested in the FSFW example for the STM32H7. The Linux changes were tested in the EIVE fork. The hosted changes were tested in the hosted FSFW example. Also, some other general improvements/notes: There seems to be a concensus that virtual functions should not have default arguments. See https://google.github.io/styleguide/cppguide.html#Default_Arguments ``` The defaults for arguments in a virtual function call are determined by the static type of the target object, and there's no guarantee that all overrides of a given function declare the same defaults. ``` and this stack overflow post https://www.google.com/search?channel=fs&client=ubuntu&q=default+arguments+on+virtual+or+override+methods+are+prohibited . Also, `clang-tidy` recommends adding `[[nodiscard]]` to all constant getters. For me, this seems to be a reasonable action. It can prevent missed returnvalue checks and useless getter function calls. If someone decides to actively ignore a returnvalue, they can explicitely cast the function call to void to avoid the warning. Also, it might be a good idea to compile the FreeRTOS and RTEMS OSALs in the CI/CD as well. It should be possible to create a docker container which has all necessary tools pre-installed. ## Refactoring general task code - There was a lot of duplicate/boilerplate code inside the individual task IF OSAL implementations. Remove it by introducing base classes `PeriodicTaskBase` and `FixedTimeslotTaskBase`. - A lot of `clang-tidy` improvements ## Refactor PeriodicTaskIF - Convert `virtual ReturnValue_t addComponent(object_id_t object)` to `virtual ReturnValue_t addComponent(object_id_t object, uint8_t opCode = 0)`, allowing to pass the operation code passed to `performOperation`. Update API taking an `ExecutableObjectIF` accordingly ## Refactor FixedTimeslotTaskIF - Add additional `addSlot` function which takes an `ExecutableObjectIF` pointer and its Object ID ## Refactor FixedSequenceSlot - Introduce typedef `CustomCheckFunc` for `ReturnValue_t (*customCheckFunction)(const SlotList&)`. - Convert `ReturnValue_t (*customCheckFunction)(const SlotList&)` to `ReturnValue_t (*customCheckFunction)(const SlotList&, void*)`, allowing arbitrary user arguments for the custom checker ## Linux OSAL - Use composition instead of inheritance for the `PeriodicPosixTask` and make the `PosixTask` a member of the class

muellerr added 10 commits 2022-05-30 10:48:19 +02:00

7df1922633

refactor task IF

18b342e94b vector as core container is ok