ModuleBase add common base and cleanup

[dagar]

This PR updates ModuleBase<T> (px4_module.h) to have a non-template base class ModuleBaseInterface. Then instead of each module having a static _object pointer there's a single LinkedList.

Then methods like ModuleBase<T>::stop_command() have been refactored so that the bulk of the code is common, with each module frontend finding its object pointer by name from the LinkedList, rather than statically. Getting the bulk of this code out of the template is where flash is saved.

This is an incremental step towards expanding ModuleBase to optionally handle multiple instances of a module.

[dagar]

@bkueng and @julianoes could I get some initial feedback? I originally did this last year, so I need to review it myself...

[julianoes]

This is a bit confusing because there are checks for object != nullptr in several places.
Could you invert the logic to:

object = get_module_instance(name);
if (object == nullptr) {
    break;
    ...

And with a scoped lock you could even get rid of the do while and just return directly.

[dagar]

The locking and structure is a bit weird because it's expecting the task to kill itself cleanly, so it's sleeping, checking, etc in between.

I'll review it again.

[julianoes]

I still think you could turn that around for better readability.

[bkueng]

The design looks ok - I initially went for a stronger separation between tasks, but since we're generally moving away from that, this works too.
I haven't checked the details or for corner cases.

[dagar]

@bkueng @julianoes can we regroup and start going through this again? It saves a relatively large amount of flash (> 5 kB on fmu-v2) and sets us up for taking the next step in running multiple instances of certain modules (eg multi-EKF).

Notes:

• I wouldn't really worry about the modules command yet, it's mostly for visibility and to think about doing a full clean shutdown in the future. Currently modules stop-all only does a request_stop() for each, which isn't quite right.
• As a followup I'm considering creating specialized ModuleBases for WQs and Tasks now that the px4 work queue patterns are becoming clear (including some shutdown holes). I think we'll be able to do a cleaner/better job here for each separate case. With that in mind we need to make sure the core of this change works properly and is safe, but it doesn't need to be perfect (yet).

[dagar]

[bkueng]

I will need to go through that in detail. Meanwhile can you check again, I think some of my previous remarks are not resolved yet.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. Thank you for your contributions.

[LorenzMeier]

Closing as stale.