orocos-toolchain / ocl

Orocos Component Library
Other
16 stars 33 forks source link

deployment: stop, cleanup and unload components in the order they were loaded #22

Closed meyerj closed 9 years ago

meyerj commented 9 years ago

This pull request contains two patches for the OCL DeploymentComponent and a minor update for the TaskBrowser. They can be applied independently from each other.

  1. https://github.com/meyerj/ocl/commit/71425aeca7b82ff702c3681800ea57a4feb3f18e fixes a compiler warning in the TaskBrowser constructor.
  2. https://github.com/meyerj/ocl/commit/f5a0a7343bf45ab565e4194dad1d0ab20fcc18bb changes the order in which components within a group are stopped, cleaned up and unloaded in stopComponents(), cleanupComponents(), unloadComponents() and kickOutAll(). Groups are unloaded in reverse order, but all components within one group have been unloaded in alphabetical order, which does not make sense in my opinion (e.g. when components depend on some of their peers during stopping or cleaning up). All components loaded with loadComponent(...) are assigned to the default group 0 and hence have been unloaded in alphabetical order. With this patch stopping, cleaning up and unloading always happens in reversed load order.
  3. https://github.com/meyerj/ocl/commit/2ecd14e867a0e5e814e48c1a2628b3c9a52212b5 fixes a bug(?) in group counting when components have been loaded from a configuration file into different groups. configureComponents() and startComponents() configure and start the component groups in forward order now instead of reversed (was this a bug or intended?). Components in group 0 were never configured and started by the deployer, even if AutoConf/AutoStart was set to true.

    This patch partially reverts 31b6e28594ed4156d328f168a6fcbd609d4e8ee6. nextGroup should be set to min(nextGroup, group + 1) if components are loaded into a group explicitly with loadComponentsInGroup() to make sure that they are properly stopped and cleaned up.

Another possibility that would probably fix 2. would be to increment the nextGroup counter if a component is loaded with loadComponent(name, type) instead of implicitly assigning the default group 0 in order to have every loaded component in its own group. An alternative solution for 3. is to assign the group 1 to the first group of components loaded from a configuration or kickStart file and reserve 0 for loadComponent().

Furthermore, configureComponents() and startComponents() only configure and start components that have been loaded from a configuration file and have the AutoConf and AutoStart property set to true, while stopComponents(), cleanupComponents() and unloadComponents() also affect components loaded with loadComponent(name, type). Is this intended? Before 31b6e28594ed4156d328f168a6fcbd609d4e8ee6 was applied, group 0 was excluded from stopping, cleaning up and unloading.

meyerj commented 9 years ago

This patch partially reverts 31b6e28. nextGroup should be set to min(nextGroup, group + 1) if components are loaded into a group explicitly with loadComponentsInGroup() to make sure that they are properly stopped and cleaned up.

This part of patch 2ecd14e broke the cleanup of components in group 0 loaded with loadComponent(). I updated the commit so that it does not change the stopComponents(), cleanupComponents(), unloadComponents() and kickOutAll() implementations anymore.