-
Notifications
You must be signed in to change notification settings - Fork 12
Open
Description
Originally posted by @NicolasFussberger in #28 (comment)
Some follow up points that we should work on (but not necessarily in this PR):
- We have to implement the LifeCycleManager to report the state to the LaunchManager
- The naming is now confusing, the newly added interface is in a library called lifecycle_client_lib. However, there is already a library called lifecycle_client - which contains the launch_manager specific interface (which is currently called LifecycleClient). Probably we should change either of the names in the future.
- We have some sample apps in the repo using the LaunchManager specific interface directly. We might adapt those to use the newly introduced interface, if we want to keep this demo running.
- assume we will need rust bindings for the newly introduced interface? Or rust applications have to use the lauch_manager specific interface directly, for which a rust binding exists here.
Comments on #28 from @pawelrutkaq :
- src/lifecycle_client_lib/src/lifecyclemanager.cpp -- line 85: at_exit shall land in ScopeGuard to ensure proper cleanup automatically.
- src/lifecycle_client_lib/src/lifecyclemanager.cpp line 97: should all logs be with library specific context for easy tracking ?
- src/lifecycle_client_lib/src/lifecyclemanager.cpp line 133: Spawning a thread in each app to wait for the signal handler does not sound like the best design. Why don't we dont reuse the main or try to use the native signal handler to dispatch m_stop_source
- src/lifecycle_client_lib/src/lifecyclemanager.cpp line 125: mixing inline logs with logs from a variable is really confusing and since this log is not repeated N time, there is no point in doing it ?
- src/lifecycle_client_lib/src/lifecyclemanager.cpp line 162 This is ot obvious exit path from the process flow, it would be better when this is called from main, as above comment
- src/lifecycle_client_lib/src/aasapplicationcontainer.cpp line 27: Since this is ptr and context_ takes an array, there is a missing nullptr check somewhere or this method shall take &
- src/lifecycle_client_lib/src/aasapplicationcontainer.cpp line 105: I would say main shall be for control flow and not for app run (see my comment about signal handling). Then all the logic below would be executed post signal handling.
Metadata
Metadata
Assignees
Labels
No labels
Type
Projects
Status
Backlog
Status
Ready